kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: clang-tidy: silence pragma-once-outside-header false positives
Date Fri, 10 Jan 2020 19:25:56 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 39df95e  clang-tidy: silence pragma-once-outside-header false positives
39df95e is described below

commit 39df95e5fa2ffec1462431e4fddc3bcdbebf2bdd
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Fri Jan 10 01:14:46 2020 -0800

    clang-tidy: silence pragma-once-outside-header false positives
    
    This turned out to be quite the rabbit hole.
    
    The false positives that have been plaguing us since the LLVM 9 upgrade
    appear to be due to the '-x c++' flag provided via compile_flags.py. That
    causes clang-tidy to treat every file as a C++ source rather than .cpp files
    as C++ sources and .h files as C++ headers.
    
    So, how do we convince clang-tidy to properly handle our headers? Removing
    '-x c++' and letting Clang's auto-detection go to town doesn't work, as
    Clang expects C++ headers to be suffixed with .H, .hh, or .hpp; .h files are
    interpreted as C headers, causing issues with C++ includes like <cstdint>.
    Passing '-x c++-header' for headers doesn't work as it causes some kind of
    issue when Clang loads a compilation database that borks clang-tidy.
    
    Speaking of compilation databases, on a lark I tried using -p (-path in
    clang-tidy-diff) to provide a path to the actual compilation database, and
    that seems to have done the trick. To get it to work properly I had to
    replace the use of -- with -extra-arg in order to pass -DCLANG_TIDY; not
    sure why clang-tidy cares about this.
    
    Using the compilation database allows us to fix another long-standing wart:
    the static compile_flags.py which is stale with respect to Kudu's actual
    preprocessor definitions. The only remaining usage was YouCompleteMe, which
    used the static flags if the compilation database didn't exist, but that
    seems like a corner case not worth supporting.
    
    Change-Id: If316344dc3c120ff278c9835b8a10aac49fab9b1
    Reviewed-on: http://gerrit.cloudera.org:8080/15004
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
    Tested-by: Kudu Jenkins
---
 .ycm_extra_conf.py                 | 44 ++++++++--------------------
 build-support/clang_tidy_gerrit.py |  7 +++--
 build-support/compile_flags.py     | 60 --------------------------------------
 3 files changed, 16 insertions(+), 95 deletions(-)

diff --git a/.ycm_extra_conf.py b/.ycm_extra_conf.py
index 4e3d8ad..1660d49 100644
--- a/.ycm_extra_conf.py
+++ b/.ycm_extra_conf.py
@@ -38,30 +38,15 @@ import sys
 import ycm_core
 
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), "build-support"))
-from compile_flags import get_flags
-
-# These are the compilation flags that will be used in case there's no
-# compilation database set (by default, one is not set).
-flags = get_flags()
-
-# Set this to the absolute path to the folder (NOT the file!) containing the
-# compile_commands.json file to use that instead of 'flags'. See here for
-# more details: http://clang.llvm.org/docs/JSONCompilationDatabase.html
-#
-# Most projects will NOT need to set this to anything; you can just change the
-# 'flags' list of compilation flags. Notice that YCM itself uses that approach.
-compilation_database_folder = ''
-
-if os.path.exists( compilation_database_folder ):
-  database = ycm_core.CompilationDatabase( compilation_database_folder )
-else:
-  database = None
 
 SOURCE_EXTENSIONS = [ '.cpp', '.cxx', '.cc', '.c', '.m', '.mm' ]
 
 def DirectoryOfThisScript():
   return os.path.dirname( os.path.abspath( __file__ ) )
 
+build_latest = os.path.join( DirectoryOfThisScript(), "build", "latest" )
+
+database = ycm_core.CompilationDatabase( build_latest )
 
 def MakeRelativePathsInFlagsAbsolute( flags, working_directory ):
   if not working_directory:
@@ -116,20 +101,15 @@ def GetCompilationInfoForFile( filename ):
 
 
 def FlagsForFile( filename, **kwargs ):
-  if database:
-    # Bear in mind that compilation_info.compiler_flags_ does NOT return a
-    # python list, but a "list-like" StringVec object
-    compilation_info = GetCompilationInfoForFile( filename )
-    if not compilation_info:
-      return None
-
-    final_flags = MakeRelativePathsInFlagsAbsolute(
-      compilation_info.compiler_flags_,
-      compilation_info.compiler_working_dir_ )
-
-  else:
-    relative_to = DirectoryOfThisScript()
-    final_flags = MakeRelativePathsInFlagsAbsolute( flags, relative_to )
+  # Bear in mind that compilation_info.compiler_flags_ does NOT return a
+  # python list, but a "list-like" StringVec object
+  compilation_info = GetCompilationInfoForFile( filename )
+  if not compilation_info:
+    return None
+
+  final_flags = MakeRelativePathsInFlagsAbsolute(
+    compilation_info.compiler_flags_,
+    compilation_info.compiler_working_dir_ )
 
   return {
     'flags': final_flags,
diff --git a/build-support/clang_tidy_gerrit.py b/build-support/clang_tidy_gerrit.py
index 0419bf6..c10a0e0 100755
--- a/build-support/clang_tidy_gerrit.py
+++ b/build-support/clang_tidy_gerrit.py
@@ -19,7 +19,6 @@
 
 import argparse
 import collections
-import compile_flags
 import json
 import logging
 import multiprocessing
@@ -35,6 +34,8 @@ from kudu_util import init_logging
 
 ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
 
+BUILD_PATH = os.path.join(ROOT, "build", "latest")
+
 CLANG_TIDY_DIFF = os.path.join(
     ROOT, "thirdparty/installed/uninstrumented/share/clang/clang-tidy-diff.py")
 CLANG_TIDY = os.path.join(
@@ -67,8 +68,8 @@ def run_tidy(sha="HEAD", is_rev_range=False):
         cmdline = [CLANG_TIDY_DIFF,
                    "-clang-tidy-binary", CLANG_TIDY,
                    "-p0",
-                   "--",
-                   "-DCLANG_TIDY"] + compile_flags.get_flags()
+                   "-path", BUILD_PATH,
+                   "-extra-arg=-DCLANG_TIDY"]
         return subprocess.check_output(
             cmdline,
             stdin=file(patch_file.name),
diff --git a/build-support/compile_flags.py b/build-support/compile_flags.py
deleted file mode 100644
index 67022a6..0000000
--- a/build-support/compile_flags.py
+++ /dev/null
@@ -1,60 +0,0 @@
-#!/usr/bin/env python
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-
-import os
-from os.path import join
-
-ROOT = join(os.path.dirname(__file__), "..")
-
-def get_flags():
-    """
-    Return the set of flags that are used during compilation.
-
-    TODO(todd) it would be nicer to somehow grab these from CMake, but it's
-    not clear how to do so.
-    """
-    return [
-        '-x',
-        'c++',
-        '-DKUDU_HEADERS_NO_STUBS=1',
-        '-DKUDU_HEADERS_USE_RICH_SLICE=1',
-        '-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1',
-        '-DKUDU_STATIC_DEFINE',
-        '-D__STDC_FORMAT_MACROS',
-        '-fno-strict-aliasing',
-        '-msse4.2',
-        '-Wall',
-        '-Wno-sign-compare',
-        '-Wno-deprecated',
-        '-pthread',
-        '-ggdb',
-        '-Qunused-arguments',
-        '-Wno-ambiguous-member-template',
-        '-std=c++11',
-        '-g',
-        '-fPIC',
-        '-I',
-        join(ROOT, 'src'),
-        '-I',
-        join(ROOT, 'build/latest/src'),
-        '-isystem',
-        join(ROOT, 'thirdparty/installed/common/include'),
-        '-isystem',
-        join(ROOT, 'thirdparty/installed/uninstrumented/include'),
-    ]


Mime
View raw message