Skip to content

Commit 253f419

Browse files
JacobfaibJacobfaib
committed
Add minimal clang-tidy pass to cmake and CI
1 parent d795dca commit 253f419

File tree

17 files changed

+536
-36
lines changed

17 files changed

+536
-36
lines changed

‎.clang-tidy‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES.
2+
# All rights reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
---
5+
Checks:
6+
# enable everything first
7+
- 'performance-*'
8+
- 'modernize-*'
9+
- 'readability-*'
10+
- 'clang-analyzer-*'
11+
- 'clang-diagnostic-*'
12+
- 'bugprone-*'
13+
- 'misc-*'
14+
- 'core-*'
15+
- 'mpi-*'
16+
- 'cert-*'
17+
- 'portability-*'
18+
- 'google-*'
19+
- 'cppcoreguidelines-*'
20+
- 'concurrency-*'
21+
# TODO: BEGIN REMOVE ME
22+
- '-*'
23+
- 'performance-implicit-conversion-in-loop'
24+
# END REMOVE ME
25+
# HICPP is 99% aliased to other checks (mostly modernize-* and bugprone-*). We don't
26+
# want to also enable it because then we need to duplicate the NOLINT. The only 2 checks
27+
# that aren't aliases are:
28+
#
29+
# - hicpp-multiway-paths-covered. This check is useful for detecting degenerate if-else
30+
# branches, but the switch cases are IMO better handled by -Wswitch. To make -Wswitch
31+
# even stronger, we should ban default: cases entirely,
32+
#
33+
# - hicpp-signed-bitwise. This check is also covered by
34+
# clang.llvm.org/extra/clang-tidy/checks/clang-analyzer/core.BitwiseShift.html
35+
#
36+
# - 'hicpp-*'
37+
#
38+
# LLVM checks are extremely specific to the LLVM project and aren't suitable for
39+
# downstream users.
40+
#
41+
# - 'llvm-*'
42+
#
43+
# then disable the stuff we don't want
44+
- '-cert-dcl21-cpp' # returning non-const from operator-- or operator++
45+
- '-cert-dcl50-cpp' # allow c-style variadics
46+
# No reserved identifiers, both of these are aliased to bugprone-reserved-identifier,
47+
# which we do enable. Leaving these enabled however, leads to needing to specify all
48+
# three (bugprone-reserved-identifier, cert-dcl51-cpp, and cert-dcl37-c) in NOLINT lines
49+
# which is a hassle. Since bugprone-reserved-identifier is enabled, the check still
50+
# fires.
51+
- '-cert-dcl51-cpp,-cert-dcl37-c,-cert-oop54-cpp'
52+
# Covered by bugprone-throwing-static-initialization
53+
- '-cert-err58-cpp'
54+
- '-modernize-use-trailing-return-type'
55+
- '-readability-function-cognitive-complexity'
56+
- '-readability-implicit-bool-conversion'
57+
- '-readability-braces-around-statements'
58+
- '-readability-qualified-auto'
59+
- '-readability-isolate-declaration'
60+
- '-modernize-avoid-c-arrays'
61+
- '-cppcoreguidelines-avoid-c-arrays'
62+
- '-readability-named-parameter'
63+
- '-readability-identifier-length'
64+
- '-misc-non-private-member-variables-in-classes'
65+
- '-bugprone-easily-swappable-parameters'
66+
- '-bugprone-implicit-widening-of-multiplication-result'
67+
- '-misc-include-cleaner'
68+
- '-misc-header-include-cycle'
69+
- '-modernize-macro-to-enum'
70+
- '-cppcoreguidelines-macro-to-enum'
71+
- '-misc-no-recursion'
72+
- '-bugprone-dynamic-static-initializers'
73+
- '-portability-avoid-pragma-once'
74+
# Generally speaking if something is static then it has an express reason to be. 99% of
75+
# the candidates identified by this check were because they just so happened not to
76+
# touch any member variables, not because they are logically static. So we disable the
77+
# check.
78+
- '-readability-convert-member-functions-to-static'
79+
# When running in a conda env, ignore options that may have been added by conda but are unused
80+
- '-clang-diagnostic-unused-command-line-argument'
81+
# Given
82+
#
83+
# std::make_pair<some_type, some_other_type>(...)
84+
#
85+
# Results in: error: for C++11-compatibility, use pair directly. But we don't care about
86+
# C++11, and so we don't care about this warning.
87+
- '-google-build-explicit-make-pair'
88+
# This check is incredibly expensive for absolutely no reason, and since we a) use
89+
# modern google-test and b) don't have clang-tidy enabled on our testing code, we don't
90+
# need to enable it!
91+
- '-google-upgrade-googletest-case'
92+
# This warning will warn if you have redundant default-initializers for class
93+
# members. For example:
94+
#
95+
# class Foo
96+
# {
97+
# int x_{}; // WARNING: redundant initializer here
98+
#
99+
# public:
100+
# Foo(int x) : x_{x} { }
101+
# };
102+
#
103+
# Since Foo can only ever be constructed with an explicit value for x_ via its constructor,
104+
# the default initializer is technically redundant. However, if we change the definition
105+
# of Foo to now allow a default ctor, then the initializer becomes non-redundant
106+
# again. It is easier to just follow the rule of "always explicitly default initialize
107+
# members" than to remember to change 2 places at once.
108+
- '-readability-redundant-member-init'
109+
# Alias for readability-enum-initial-value, disable this one because the readability-
110+
# name is easier to understand, and we don't want to silence 2 things for the same
111+
# warning.
112+
- '-cert-int09-c'
113+
# This one is potentially controversial. This check warns when iterating over unordered
114+
# containers of pointers:
115+
#
116+
# {
117+
# int a = 1, b = 2;
118+
# std::unordered_set<int *> set = {&a, &b};
119+
#
120+
# for (auto *i : set) { // iteration order not deterministic
121+
# f(i);
122+
# }
123+
# }
124+
#
125+
# On the one hand, this is a clear case of non-determinism. But on the other hand, I
126+
# feel it is obvious that a user does not care about the order of iteration
127+
# because... they are using unordered containers!
128+
- '-bugprone-nondeterministic-pointer-iteration-order'
129+
- '-cppcoreguidelines-pro-type-reinterpret-cast'
130+
# We don't use GSL
131+
- '-cppcoreguidelines-owning-memory'
132+
# Covered by modernize-use-override already
133+
- '-cppcoreguidelines-explicit-virtual-functions'
134+
# Covered by readability-magic-numbers
135+
- '-cppcoreguidelines-avoid-magic-numbers'
136+
# This check does more harm than good, as it appears to be very rudimentary. For
137+
# example, it emits warnings saying that:
138+
#
139+
# #define DEFINE_OPERATOR(op) \
140+
# _CCCL_HOST_DEVICE custom_numeric operator op() const \
141+
# { \
142+
# return custom_numeric(op value[0]); \
143+
# }
144+
#
145+
# Should be replaced by a template function, but this is not possible. We instead rely
146+
# on best judgment of reviewers to catch macros that can be functions.
147+
- '-cppcoreguidelines-macro-usage'
148+
# This check does not understand more complex macro usage where brackets are not allowed
149+
#
150+
# error: macro replacement list should be enclosed in parentheses [bugprone-macro-parentheses]
151+
# 36 | #define __THRUST_HOST_SYSTEM_INCLUDE(filename) <__THRUST_HOST_SYSTEM_ROOT/filename>
152+
# | ^
153+
# | ( )
154+
#
155+
# So better left disabled.
156+
- '-bugprone-macro-parentheses'
157+
# TODO: ironically enough, enable this someday
158+
- '-google-readability-todo'
159+
# Covered by modernize-avoid-c-style-cast, and also is buggier. It seems to fire on
160+
# `_v`-style constexpr bools as well sometimes.
161+
- '-google-readability-casting'
162+
# Covered by performance-noexcept-move-constructor
163+
- '-cppcoreguidelines-noexcept-move-operations'
164+
# REVIEW ME: This warns about any usage of operator[], suggesting usage of .at() instead. Could
165+
- '-cppcoreguidelines-pro-bounds-avoid-unchecked-container-access'
166+
# Covered by bugprone-narrowing-conversions
167+
- '-cppcoreguidelines-narrowing-conversions'
168+
# Covered by misc-unconventional-assign-operator
169+
- '-cppcoreguidelines-c-copy-assignment-signature'
170+
# Covered by performance-noexcept-swap
171+
- '-cppcoreguidelines-noexcept-swap'
172+
# Covered by modernize-use-default-member-init
173+
- '-cppcoreguidelines-use-default-member-init'
174+
# clang-tidy documentation says that for a given file, it matches the closest
175+
# .clang-tidy up the directory stack and applies the configuration from it. However, by
176+
# "it" `clang-tidy` means the closest `.clang-tidy` to the *source* file, not the file
177+
# that was included by the source file.
178+
#
179+
# So `cuda/std/foo.h` included by `cccl/foo/bar/baz.cpp` will never "match" against
180+
# `cccl/libcudacxx/.clang-tidy` because `clang-tidy` finds `cccl/.clang-tidy` instead.
181+
#
182+
# So we disable this for now. In the future we should add a custom check that bans
183+
# reserved identifiers except symbols from libcu++.
184+
- '-bugprone-reserved-identifier'
185+
WarningsAsErrors: '*'
186+
HeaderFileExtensions:
187+
- ''
188+
- h
189+
- hh
190+
- hpp
191+
- hxx
192+
- cuh
193+
- inl
194+
- ipp
195+
ImplementationFileExtensions:
196+
- c
197+
- cc
198+
- cpp
199+
- cxx
200+
- cu
201+
SystemHeaders: false
202+
ExtraArgsBefore:
203+
- '-Wno-unknown-warning-option'
204+
- '-Wno-error=unused-command-line-argument'
205+
# To match _CCCL_DOXYGEN_INVOKED
206+
- '-D_CCCL_CLANG_TIDY_INVOKED=1'
207+
CheckOptions:
208+
cert-dcl16-c.NewSuffixes: 'L;LL;LU;LLU'
209+
cert-err33-c.CheckedFunctions: '::aligned_alloc;::asctime_s;::at_quick_exit;::atexit;::bsearch;::bsearch_s;::btowc;::c16rtomb;::c32rtomb;::calloc;::clock;::cnd_broadcast;::cnd_init;::cnd_signal;::cnd_timedwait;::cnd_wait;::ctime_s;::fclose;::fflush;::fgetc;::fgetpos;::fgets;::fgetwc;::fopen;::fopen_s;::fprintf;::fprintf_s;::fputc;::fputs;::fputwc;::fputws;::fread;::freopen;::freopen_s;::fscanf;::fscanf_s;::fseek;::fsetpos;::ftell;::fwprintf;::fwprintf_s;::fwrite;::fwscanf;::fwscanf_s;::getc;::getchar;::getenv;::getenv_s;::gets_s;::getwc;::getwchar;::gmtime;::gmtime_s;::localtime;::localtime_s;::malloc;::mbrtoc16;::mbrtoc32;::mbsrtowcs;::mbsrtowcs_s;::mbstowcs;::mbstowcs_s;::memchr;::mktime;::mtx_init;::mtx_lock;::mtx_timedlock;::mtx_trylock;::mtx_unlock;::printf_s;::putc;::putwc;::raise;::realloc;::remove;::rename;::scanf;::scanf_s;::setlocale;::setvbuf;::signal;::snprintf;::snprintf_s;::sprintf;::sprintf_s;::sscanf;::sscanf_s;::strchr;::strerror_s;::strftime;::strpbrk;::strrchr;::strstr;::strtod;::strtof;::strtoimax;::strtok;::strtok_s;::strtol;::strtold;::strtoll;::strtoul;::strtoull;::strtoumax;::strxfrm;::swprintf;::swprintf_s;::swscanf;::swscanf_s;::thrd_create;::thrd_detach;::thrd_join;::thrd_sleep;::time;::timespec_get;::tmpfile;::tmpfile_s;::tmpnam;::tmpnam_s;::tss_create;::tss_get;::tss_set;::ungetc;::ungetwc;::vfprintf;::vfprintf_s;::vfscanf;::vfscanf_s;::vfwprintf;::vfwprintf_s;::vfwscanf;::vfwscanf_s;::vprintf_s;::vscanf;::vscanf_s;::vsnprintf;::vsnprintf_s;::vsprintf;::vsprintf_s;::vsscanf;::vsscanf_s;::vswprintf;::vswprintf_s;::vswscanf;::vswscanf_s;::vwprintf_s;::vwscanf;::vwscanf_s;::wcrtomb;::wcschr;::wcsftime;::wcspbrk;::wcsrchr;::wcsrtombs;::wcsrtombs_s;::wcsstr;::wcstod;::wcstof;::wcstoimax;::wcstok;::wcstok_s;::wcstol;::wcstold;::wcstoll;::wcstombs;::wcstombs_s;::wcstoul;::wcstoull;::wcstoumax;::wcsxfrm;::wctob;::wctrans;::wctype;::wmemchr;::wprintf_s;::wscanf;::wscanf_s;'
210+
llvm-else-after-return.WarnOnUnfixable: 'false'
211+
cert-str34-c.DiagnoseSignedUnsignedCharComparisons: 'false'
212+
cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: 'true'
213+
google-readability-braces-around-statements.ShortStatementLines: '1'
214+
llvm-qualified-auto.AddConstToQualified: 'false'
215+
llvm-else-after-return.WarnOnConditionVariables: 'false'
216+
cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField: 'false'
217+
performance-move-const-arg.CheckTriviallyCopyableMove: 'false'
218+
performance-inefficient-string-concatenation.StrictMode: 'true'
219+
readability-simplify-boolean-expr.ChainedConditionalReturn: 'true'
220+
readability-simplify-boolean-expr.ChainedConditionalAssignment: 'true'
221+
bugprone-dangling-handle.HandleClasses: '::cuda::std::span;::cuda::std::mdspan'
222+
bugprone-misplaced-widening-cast.CheckImplicitCasts: 'true'
223+
bugprone-unused-return-value.AllowCastToVoid: 'true'
224+
readability-enum-initial-value.AllowExplicitZeroFirstInitialValue: 'false'
225+
readability-enum-initial-value.AllowExplicitSequentialInitialValues: 'false'
226+
readability-redundant-access-specifiers.CheckFirstDeclaration: 'true'
227+
bugprone-lambda-function-name.IgnoreMacros: 'true'
228+
# readability-identifier-naming.ClassCase: 'lower_case'
229+
# readability-identifier-naming.UnionCase: 'lower_case'
230+
# readability-identifier-naming.ClassConstantCase: 'UPPER_CASE'
231+
# readability-identifier-naming.ClassIgnoredRegexp: 'tuple|has_.*|is_.*|as_.*|.*_tag|tag|.*_of'
232+
# readability-identifier-naming.ConstantMemberCase: 'UPPER_CASE'
233+
# readability-identifier-naming.ConstantMemberIgnoredRegexp: 'value'
234+
# readability-identifier-naming.EnumCase: 'lower_case'
235+
# readability-identifier-naming.EnumConstantCase: 'UPPER_CASE'
236+
# readability-identifier-naming.FunctionCase: 'lower_case'
237+
# readability-identifier-naming.GlobalConstantCase: 'UPPER_CASE'
238+
# readability-identifier-naming.GlobalConstantIgnoredRegexp: '.*_v'
239+
# readability-identifier-naming.LocalVariableCase: 'lower_case'
240+
# # We want to allow constexpr auto MY_VAL1
241+
# readability-identifier-naming.LocalVariableIgnoredRegexp: '[A-Z_0-9]+'
242+
# readability-identifier-naming.MacroDefinitionCase: 'UPPER_CASE'
243+
# # We want to allow MY_MACRO_PRIVATE_1_
244+
# readability-identifier-naming.MacroDefinitionIgnoredRegexp: '[A-Z_0-9]+'
245+
# readability-identifier-naming.NamespaceCase: 'lower_case'
246+
# readability-identifier-naming.PrivateMemberCase: 'lower_case'
247+
# readability-identifier-naming.PrivateMemberSuffix: '_'
248+
# readability-identifier-naming.PrivateMethodCase: 'lower_case'
249+
# readability-identifier-naming.PrivateMethodSuffix: '_'
250+
# readability-identifier-naming.ProtectedMemberCase: 'lower_case'
251+
# readability-identifier-naming.ProtectedMemberSuffix: '_'
252+
# readability-identifier-naming.ProtectedMethodCase: 'lower_case'
253+
# readability-identifier-naming.ProtectedMethodSuffix: '_'
254+
# readability-identifier-naming.PublicMethodCase: 'lower_case'
255+
# readability-identifier-naming.ScopedEnumCase: 'lower_case'
256+
# readability-identifier-naming.ScopedEnumConstantCase: 'UPPER_CASE'
257+
readability-magic-numbers.IgnoredIntegerValues: '0;1;2;3;4;5;6;7;8;9'
258+
# There are some single-argument functions that we would not like to ignore
259+
# (`to_string(bool provenance = false)` comes to mind), but setting this to false makes
260+
# clang-tidy warn on a bunch of "obvious" calls, like `set_dim(1)` or
261+
# `with_concurrent(true)`.
262+
bugprone-argument-comment.IgnoreSingleArgument: true
263+
bugprone-argument-comment.CommentBoolLiterals: true
264+
bugprone-argument-comment.CommentIntegerLiterals: true
265+
bugprone-argument-comment.CommentFloatLiterals: true
266+
bugprone-argument-comment.CommentStringLiterals: true
267+
bugprone-argument-comment.CommentCharacterLiterals: true
268+
bugprone-argument-comment.CommentUserDefinedLiterals: true
269+
bugprone-argument-comment.CommentNullPtrs: true
270+
cppcoreguidelines-macro-usage.AllowedRegexp: '^DEBUG_.*|THRUST_PP_.*|_CCCL.*_PP_.*'
271+
bugprone-reserved-identifier.AllowedIdentifiers: '^_CCCL.*'
272+
...

‎CMakeLists.txt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ endif()
2323
# Enable CXX so CMake can configure install paths
2424
project(CCCL LANGUAGES CXX)
2525

26+
# Needed for clang-tidy so that relative paths can be disambiguated
27+
set(CCCL_TOPLEVEL_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}")
28+
2629
include(cmake/CCCLInstallRules.cmake)
2730

2831
# Support adding CCCL to a parent project via add_subdirectory.
@@ -77,6 +80,18 @@ else()
7780
set(CCCL_ENABLE_CUDAX OFF)
7881
endif()
7982

83+
if (
84+
CCCL_TOPLEVEL_PROJECT
85+
OR CCCL_ENABLE_TESTING
86+
OR CCCL_ENABLE_EXAMPLES
87+
OR CCCL_ENABLE_BENCHMARKS
88+
)
89+
# cccl_add_tidy_target() is called by cccl_add_executable(). It expects that we have
90+
# called cccl_tidy_init() before then, so do that now.
91+
include(cmake/CCCLAddTidyTarget.cmake)
92+
cccl_tidy_init()
93+
endif()
94+
8095
include(CTest)
8196
enable_testing()
8297

‎CMakePresets.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,15 @@
104104
"cudax_ENABLE_CUDASTF_BOUNDSCHECK": true
105105
}
106106
},
107+
{
108+
"name": "all-tidy",
109+
"displayName": "clang-tidy",
110+
"inherits": "all-dev-debug",
111+
"cacheVariables": {
112+
"CMAKE_CXX_STANDARD": "17",
113+
"CMAKE_CUDA_STANDARD": "17"
114+
}
115+
},
107116
{
108117
"name": "libcudacxx-codegen",
109118
"displayName": "libcu++: codegen",
@@ -467,6 +476,13 @@
467476
"name": "all-dev-debug",
468477
"configurePreset": "all-dev-debug"
469478
},
479+
{
480+
"name": "all-tidy",
481+
"configurePreset": "all-tidy",
482+
"targets": [
483+
"tidy"
484+
]
485+
},
470486
{
471487
"name": "install",
472488
"configurePreset": "install"

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ Does your project use CCCL? [Open a PR to add your project to this list!](https:
511511
- [Qiskit](https://github.com/Qiskit/qiskit-aer) - High performance simulator for quantum circuits
512512
- [QUDA](https://github.com/lattice/quda) - Lattice quantum chromodynamics (QCD) computations
513513
- [RAFT](https://github.com/rapidsai/raft) - Algorithms and primitives for machine learning
514-
- [SGLang](https://github.com/sgl-project/sglang) - LLM serving framework
514+
- [SGLang](https://github.com/sgl-project/sglang) - LLM serving framework
515515
- [TensorFlow](https://github.com/tensorflow/tensorflow) - End-to-end platform for machine learning
516516
- [TensorRT](https://github.com/NVIDIA/TensorRT) - Deep learning inference
517517
- [TensorRT-LLM](https://github.com/NVIDIA/TensorRT-LLM) - Optimized LLM inference

‎c/experimental/stf/test/CMakeLists.txt‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@ function(cccl_c_experimental_stf_add_test target_name_var source)
1010
)
1111
set(target_name_var ${target_name} PARENT_SCOPE)
1212

13-
add_executable(${target_name} "${source}")
14-
cccl_configure_target(${target_name} DIALECT 20)
13+
cccl_add_executable(
14+
${target_name}
15+
ADD_CTEST
16+
NO_METATARGETS
17+
DIALECT 20
18+
SOURCES "${source}"
19+
)
1520

1621
set_target_properties(${target_name} PROPERTIES CUDA_RUNTIME_LIBRARY STATIC)
1722
target_link_libraries(
@@ -24,8 +29,6 @@ function(cccl_c_experimental_stf_add_test target_name_var source)
2429
cccl.c2h.main
2530
CUDA::cuda_driver
2631
)
27-
28-
add_test(NAME ${target_name} COMMAND ${target_name})
2932
endfunction()
3033

3134
file(

‎c/parallel/test/CMakeLists.txt‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@ function(cccl_c_parallel_add_test target_name_var source)
1111
)
1212
set(target_name_var ${target_name} PARENT_SCOPE)
1313

14-
add_executable(${target_name} "${source}")
15-
cccl_configure_target(${target_name} DIALECT 20)
14+
cccl_add_executable(
15+
${target_name}
16+
ADD_CTEST
17+
NO_METATARGETS
18+
DIALECT 20
19+
SOURCES "${source}"
20+
)
1621

1722
set_target_properties(${target_name} PROPERTIES CUDA_RUNTIME_LIBRARY STATIC)
1823
target_link_libraries(
@@ -36,8 +41,6 @@ function(cccl_c_parallel_add_test target_name_var source)
3641
TEST_CTK_PATH="-I${CUDA_FIRST_INCLUDE_DIR}"
3742
TEST_INCLUDE_PATH="${CMAKE_CURRENT_SOURCE_DIR}"
3843
)
39-
40-
add_test(NAME ${target_name} COMMAND ${target_name})
4144
endfunction()
4245

4346
file(

0 commit comments

Comments
 (0)