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 <[email protected]>
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 <[email protected]>
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'),
- ]
