This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 684725fd457e308ee71889d92d8346dd081d4e5a
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Wed Dec 18 21:53:32 2019 -0800
clang-tidy: no warnings on structs with all public data members
Since the LLVM 9 upgrade clang-tidy has been warning on things like this:
src/kudu/tserver/tablet_copy_source_session.h:66:11: warning: member variable 'size'
has public visibility [misc-non-private-member-variables-in-classes]
int64_t size;
^
'size' is indeed a public data member, but as per the Google Style Guide[1],
structs should consist of nothing but public data members.
I couldn't find a way to configure clang-tidy correctly on this issue, so
instead I reconfigured the corresponding check to not warn about any struct
or class where all data members are public. In the process, I rediscovered
our existing .clang-tidy configuration file and moved the recently disabled
checks there.
1. https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes
Change-Id: Ibf9913fd0b0410b44cf36f044256ef0266949d95
Reviewed-on: http://gerrit.cloudera.org:8080/14930
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <awong@cloudera.com>
---
build-support/clang_tidy_gerrit.py | 29 ++--------------------
src/kudu/.clang-tidy | 50 +++++++++++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/build-support/clang_tidy_gerrit.py b/build-support/clang_tidy_gerrit.py
index cd500b1..0419bf6 100755
--- a/build-support/clang_tidy_gerrit.py
+++ b/build-support/clang_tidy_gerrit.py
@@ -45,31 +45,6 @@ GERRIT_PASSWORD = os.getenv('TIDYBOT_PASSWORD')
GERRIT_URL = "https://gerrit.cloudera.org"
-DISABLED_TIDY_CHECKS=[
- # Although useful in production code, we use magic numbers liberally in
- # tests, and many would be net less clear as named constants.
- 'readability-magic-numbers',
-
- # IWYU has specific rules for ordering '-inl.h' include files, i.e.
- # 'header.h' and its 'header-inl.h' counterpart. It seems in some cases
- # including 'header-inl.h' before 'header.h' might even lead to compilation
- # failures. So, IWYU intentionally re-orders them even if 'header-inl.h'
- # comes before 'header.h' lexicographically in default C locale:
- # https://github.com/apache/kudu/blob/ \
- # 89ce529e945731c48445db4a6f8af11f9f905aab/build-support/iwyu/ \
- # fix_includes.py#L1786-L1787
- #
- # That ordering contradicts with what clang-tidy recommends when using the
- # 'llvm-include-order' check. To avoid confusion, let's disable the
- # 'llvm-include-order'.
- #
- # TODO(aserbin): clarify whether it's possible to customize clang-tidy
- # behavior w.r.t. the sorting of such header files using
- # the format style options described at
- # https://clang.llvm.org/docs/ClangFormatStyleOptions.html
- 'llvm-include-order',
-]
-
def run_tidy(sha="HEAD", is_rev_range=False):
diff_cmdline = ["git", "diff" if is_rev_range else "show", sha]
@@ -79,6 +54,8 @@ def run_tidy(sha="HEAD", is_rev_range=False):
# Produce a separate diff for each file and run clang-tidy-diff on it
# in parallel.
+ #
+ # Note: this will incorporate any configuration from .clang-tidy.
def tidy_on_path(path):
patch_file = tempfile.NamedTemporaryFile()
cmd = diff_cmdline + [
@@ -87,11 +64,9 @@ def run_tidy(sha="HEAD", is_rev_range=False):
"--",
path]
subprocess.check_call(cmd, stdout=patch_file, cwd=ROOT)
- checks_arg_value = ",".join([ "-" + c for c in DISABLED_TIDY_CHECKS ])
cmdline = [CLANG_TIDY_DIFF,
"-clang-tidy-binary", CLANG_TIDY,
"-p0",
- "-checks=" + checks_arg_value,
"--",
"-DCLANG_TIDY"] + compile_flags.get_flags()
return subprocess.check_output(
diff --git a/src/kudu/.clang-tidy b/src/kudu/.clang-tidy
index e4af41b..1b6c4ea 100644
--- a/src/kudu/.clang-tidy
+++ b/src/kudu/.clang-tidy
@@ -14,7 +14,49 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,-*,readability-*,-readability-braces-around-statements,llvm-include-order,misc-*,-modernize-*,performance-*,-readability-implicit-bool-conversion,google-*,-google-readability-braces-around-statements,-readability-redundant-string-init,modernize-make-shared,modernize-shrink-to-fit,modernize-use-emplace,modernize-pass-by-value,modernize-use-nullptr,bugprone-*,clang-analyzer-deadcode.DeadStores,hicpp-noexcept-move'
+
+Checks: >-
+ -*,
+ bugprone-*,
+ clang-analyzer-*,
+ clang-analyzer-deadcode.DeadStores,
+ -clang-analyzer-alpha*,
+ clang-diagnostic-*,
+ google-*,
+ -google-readability-braces-around-statements,
+ hicpp-noexcept-move,
+ # IWYU has specific rules for ordering '-inl.h' include files, i.e.
+ # 'header.h' and its 'header-inl.h' counterpart. It seems in some cases
+ # including 'header-inl.h' before 'header.h' might even lead to compilation
+ # failures. So, IWYU intentionally re-orders them even if 'header-inl.h'
+ # comes before 'header.h' lexicographically in default C locale:
+ # https://github.com/apache/kudu/blob/ \
+ # 89ce529e945731c48445db4a6f8af11f9f905aab/build-support/iwyu/ \
+ # fix_includes.py#L1786-L1787
+ #
+ # That ordering contradicts with what clang-tidy recommends when using the
+ # 'llvm-include-order' check.
+ #
+ # TODO(aserbin): clarify whether it's possible to customize clang-tidy
+ # behavior w.r.t. the sorting of such header files using
+ # the format style options described at
+ # https://clang.llvm.org/docs/ClangFormatStyleOptions.html
+ -llvm-include-order,
+ misc-*,
+ -modernize-*,
+ modernize-make-shared,
+ modernize-pass-by-value,
+ modernize-shrink-to-fit,
+ modernize-use-emplace,
+ modernize-use-nullptr,
+ performance-*,
+ readability-*,
+ -readability-braces-around-statements,
+ -readability-implicit-bool-conversion,
+ # Although useful in production code, we use magic numbers liberally in
+ # tests, and many would be net less clear as named constants.
+ -readability-magic-numbers,
+ -readability-redundant-string-init
HeaderFilterRegex: '.*,-*.pb.h'
CheckOptions:
- key: google-runtime-references.WhiteListTypes
@@ -35,3 +77,9 @@ CheckOptions:
value: CamelCase
- key: readability-identifier-naming.GlobalConstantPrefix
value: 'k'
+ # The Google Style Guide allows for structs to contain public data members.
+ # Unfortunately there doesn't seem to be a way to configure clang-tidy to
+ # permit this, so we allow classes/structs where all data members are public
+ # as a proxy.
+ - key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
+ value: '1'
|