Skip to content

Commit 1ab45ba

Browse files
jrajahalmejrajahalme
committed
build: Add clang-tidy support with include cleaner
Add the following clang packages to install_clang function: - clangd-17 - lldb-17 - clang-tools-17 - cland-tidy-17 Add make targets: - compile_commands.json: create the compile commands database needed for both clang-tidy and clangd. Include absl which otherwise was unreachable for clangd. - tidy: Use run-clang-tidy-17 to run clang-tidy in parallel. It is still very slow. - tidy-fix: Run clang tidy and fix errors. Use with caution, as the fixes do not always compile. Add query option --incompatible_merge_fixed_and_default_shell_env to .bazelrc so that query does not trigger re-download of envoy dependency. This same option is set on 'build' in 'envoy.bazelrc'. Signed-off-by: Jarno Rajahalme <[email protected]>
1 parent f0d28a4 commit 1ab45ba

File tree

5 files changed

+149
-3
lines changed

5 files changed

+149
-3
lines changed

‎.bazelrc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,6 @@ build:release --define manual_stamp=manual_stamp
3030
# Always have LD_LIBRARY_PATH=/usr/cross-compat/lib defined in the test environment.
3131
# The path does not need to exist, but can be created when needed for running tests.
3232
build --test_env=LD_LIBRARY_PATH=/usr/cilium-cross-compat/lib
33+
34+
# use same env option for query as upstream is using for build
35+
query --incompatible_merge_fixed_and_default_shell_env

‎.clang-tidy

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
Checks: >
2+
-clang-analyzer-core.NonNullParamChecker,
3+
-clang-analyzer-optin.cplusplus.UninitializedObject,
4+
abseil-duration-*,
5+
abseil-faster-strsplit-delimiter,
6+
abseil-no-namespace,
7+
abseil-redundant-strcat-calls,
8+
abseil-str-cat-append,
9+
abseil-string-find-startswith,
10+
abseil-upgrade-duration-conversions,
11+
bugprone-assert-side-effect,
12+
bugprone-unused-raii,
13+
bugprone-use-after-move,
14+
clang-analyzer-core.DivideZero,
15+
misc-unused-using-decls,
16+
modernize-deprecated-headers,
17+
modernize-loop-convert,
18+
modernize-make-shared,
19+
modernize-make-unique,
20+
modernize-return-braced-init-list,
21+
modernize-use-default-member-init,
22+
modernize-use-equals-default,
23+
modernize-use-nullptr,
24+
modernize-use-override,
25+
modernize-use-using,
26+
performance-faster-string-find,
27+
performance-for-range-copy,
28+
performance-inefficient-algorithm,
29+
performance-inefficient-vector-operation,
30+
performance-noexcept-move-constructor,
31+
performance-move-constructor-init,
32+
performance-type-promotion-in-math-fn,
33+
performance-unnecessary-copy-initialization,
34+
readability-braces-around-statements,
35+
readability-container-size-empty,
36+
readability-identifier-naming,
37+
readability-redundant-control-flow,
38+
readability-redundant-member-init,
39+
readability-redundant-smartptr-get,
40+
readability-redundant-string-cstr
41+
42+
CheckOptions:
43+
- key: cppcoreguidelines-unused-variable.IgnorePattern
44+
value: "^_$"
45+
- key: bugprone-assert-side-effect.AssertMacros
46+
value: 'ASSERT'
47+
- key: bugprone-dangling-handle.HandleClasses
48+
value: 'std::basic_string_view;std::experimental::basic_string_view;absl::string_view'
49+
- key: misc-include-cleaner.IgnoreHeaders
50+
value: 'asm/unistd_32.h;asm/unistd_64.h;bits/basic_string\\.h;google/protobuf/.*;linux/in\\.h;linux/in6\\.h;mutex'
51+
- key: modernize-use-auto.MinTypeNameLength
52+
value: '10'
53+
- key: readability-identifier-naming.ClassCase
54+
value: 'CamelCase'
55+
- key: readability-identifier-naming.EnumCase
56+
value: 'CamelCase'
57+
- key: readability-identifier-naming.EnumConstantCase
58+
value: 'CamelCase'
59+
# Ignore GoogleTest function macros.
60+
- key: readability-identifier-naming.FunctionIgnoredRegexp
61+
# To have the regex chomped correctly fence all items with `|` (other than first/last)
62+
value: >-
63+
(^AbslHashValue$|
64+
|^called_count$|
65+
|^case_sensitive$|
66+
|^Create$|
67+
|^envoy_resolve_dns$|
68+
|^evconnlistener_free$|
69+
|^event_base_free$|
70+
|^(get|set)EVP_PKEY$|
71+
|^has_value$|
72+
|^Ip6(ntohl|htonl)$|
73+
|^get_$|
74+
|^HeaderHasValue(Ref)?$|
75+
|^HeaderValueOf$|
76+
|^Is(Superset|Subset)OfHeaders$|
77+
|^LLVMFuzzerInitialize$|
78+
|^LLVMFuzzerTestOneInput$|
79+
|^Locality$|
80+
|^MOCK_METHOD$|
81+
|^PrepareCall$|
82+
|^PrintTo$|
83+
|^resolve_dns$|
84+
|^result_type$|
85+
|Returns(Default)?WorkerId$|
86+
|^sched_getaffinity$|
87+
|^shutdownThread_$|
88+
|^envoy_dynamic_module(.*)$|
89+
|TEST|
90+
|^use_count$)
91+
- key: readability-identifier-naming.ParameterCase
92+
value: 'lower_case'
93+
- key: readability-identifier-naming.ParameterIgnoredRegexp
94+
value: (^cname_ttl_$)
95+
- key: readability-identifier-naming.PrivateMemberCase
96+
value: 'lower_case'
97+
- key: readability-identifier-naming.PrivateMemberSuffix
98+
value: '_'
99+
- key: readability-identifier-naming.StructCase
100+
value: 'CamelCase'
101+
- key: readability-identifier-naming.TypeAliasCase
102+
value: 'CamelCase'
103+
- key: readability-identifier-naming.TypeAliasIgnoredRegexp
104+
value: '(result_type)'
105+
- key: readability-identifier-naming.UnionCase
106+
value: 'CamelCase'
107+
- key: readability-identifier-naming.FunctionCase
108+
value: 'camelBack'
109+
110+
HeaderFilterRegex: '^./source/.*|^./contrib/.*|^./test/.*|^./envoy/.*'
111+
112+
UseColor: true
113+
114+
WarningsAsErrors: '*'
115+
116+
## The version here is arbitrary since any change to this file will
117+
## trigger a full run of clang-tidy against all files.
118+
## It can be useful as it seems some header changes may not trigger the
119+
## expected rerun.
120+
# v0

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
!\.bazelversion
55
!\.clangd
66
!\.clang-format
7+
!\.clang-tidy
78
!\.github
89
!\.gitignore
910

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ else
104104
# Install clang if needed
105105
define install_clang
106106
$(SUDO) apt info clang-17 || $(call add_clang_apt_source,$(shell lsb_release -cs))
107-
$(SUDO) apt install -y clang-17 llvm-17-dev lld-17 clang-format-17
107+
$(SUDO) apt install -y clang-17 clangd-17 llvm-17-dev lld-17 lldb-17 clang-format-17 clang-tools-17 clang-tidy-17
108108
endef
109109
endif
110110

‎Makefile.dev

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,33 @@ precheck: force-non-root
4444

4545
FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/" "./vendor/" "./go/" "./envoy_build_config/"
4646

47+
# The default set of sources assumes all relevant sources are dependecies of some tests!
48+
TIDY_SOURCES ?= $(shell bazel query 'kind("source file", deps(//tests/...))' 2>/dev/null | sed -n "s/\/\/cilium:/cilium\//p; s/\/\/tests:/tests\//p")
49+
50+
# Must pass our bazel options to avoid discarding the analysis cache due to different options
51+
# between this, check and build!
52+
# Depend on the WORKSPACE and TIDY_SOURCES so that the database will be re-built if
53+
# Envoy dependency or any of the source files has changed.
54+
compile_commands.json: WORKSPACE $(TIDY_SOURCES) force-non-root
55+
BAZEL_STARTUP_OPTION_LIST="$(BAZEL_OPTS)" BAZEL_BUILD_OPTION_LIST="$(BAZEL_BUILD_OPTS)" tools/gen_compilation_database.py --include_all //cilium/... //starter/... //tests/... @com_google_absl//absl/...
56+
57+
# Default number of jobs, derived from available memory
58+
TIDY_JOBS ?= $$(( $(shell sed -n "s/^MemAvailable: *\([0-9]*\).*\$$/\1/p" /proc/meminfo) / 3500000 ))
59+
60+
# tidy uses clang-tidy-17, .clang-tidy must be present in the project directory and configured to
61+
# ignore the same headers as .clangd. Unfortunately the configuration format is different.
62+
tidy: compile_commands.json force-non-root
63+
run-clang-tidy-17 -quiet -extra-arg="-Wno-unknown-pragmas" -checks=misc-include-cleaner -j $(TIDY_JOBS) $(TIDY_SOURCES)
64+
65+
tidy-fix: compile_commands.json force-non-root
66+
echo "clang-tidy fix results can contain duplicate includes, check before committing!"
67+
run-clang-tidy-17 -fix -format -style=file -quiet -extra-arg="-Wno-unknown-pragmas" -checks=misc-include-cleaner -j $(TIDY_JOBS) $(TIDY_SOURCES)
68+
4769
check: force-non-root
48-
$(BAZEL) $(BAZEL_OPTS) run @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check || echo "Format check failed, run 'make fix' locally to fix formatting errors."
70+
$(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check || echo "Format check failed, run 'make fix' locally to fix formatting errors."
4971

5072
fix: force-non-root
51-
$(BAZEL) $(BAZEL_OPTS) run @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
73+
$(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
5274

5375
# Run tests without debug by default.
5476
tests: $(COMPILER_DEP) force-non-root SOURCE_VERSION proxylib/libcilium.so install-bazelisk

0 commit comments

Comments
 (0)