Skip to content

Commit e19e860

Browse files
authored
[RFC][libc++] Reworks clang-tidy selection. (llvm#81362)
The current selection is done in the test scripts which gives the user no control where to find clang-tidy. The version selection itself is hard-coded and not based on the version of clang used. This moves the selection to configuration time and tries to find a better match. - Mixing the version of clang-tidy and the clang libraries causes ODR violations. This results in practice in crashes or incorrect results. - Mixing the version of clang-tidy and the clang binary sometimes causes issues with supported diagnostic flags. For example, flags tested against clang 17 may not be available in clang-tidy 16. Currently clang-tidy 18.1 can be used, it tests against the clang libraries version 18. This is caused by the new LLVM version numbering scheme. The new selection tries to match the clang version or the version of the HEAD and the last 3 releases. (During the release period libc++ supports 4 versions instead of the typical 3 versions.)
1 parent 99f5e96 commit e19e860

File tree

4 files changed

+51
-36
lines changed

4 files changed

+51
-36
lines changed

libcxx/test/tools/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,10 @@ set(LIBCXX_TEST_TOOLS_PATH ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)
33

44
# TODO: Remove LIBCXX_ENABLE_CLANG_TIDY
55
if(LIBCXX_ENABLE_CLANG_TIDY)
6+
if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
7+
message(STATUS "Clang-tidy can only be used when building libc++ with "
8+
"a clang compiler.")
9+
return()
10+
endif()
611
add_subdirectory(clang_tidy_checks)
712
endif()

libcxx/test/tools/clang_tidy_checks/CMakeLists.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
set(LLVM_DIR_SAVE ${LLVM_DIR})
66
set(Clang_DIR_SAVE ${Clang_DIR})
77

8-
find_package(Clang 18)
9-
if (NOT Clang_FOUND)
10-
find_package(Clang 17)
11-
endif()
8+
# Since the Clang C++ ABI is not stable the Clang libraries and clang-tidy
9+
# versions must match. Otherwise there likely will be ODR-violations. This had
10+
# led to crashes and incorrect output of the clang-tidy based checks.
11+
find_package(Clang ${CMAKE_CXX_COMPILER_VERSION})
1212

1313
set(SOURCES
1414
abi_tag_on_virtual.cpp

libcxx/utils/libcxx/test/features.py

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,30 +23,6 @@
2323
_isMSVC = lambda cfg: "_MSC_VER" in compilerMacros(cfg)
2424
_msvcVersion = lambda cfg: (int(compilerMacros(cfg)["_MSC_VER"]) // 100, int(compilerMacros(cfg)["_MSC_VER"]) % 100)
2525

26-
27-
def _getSuitableClangTidy(cfg):
28-
try:
29-
# If we didn't build the libcxx-tidy plugin via CMake, we can't run the clang-tidy tests.
30-
if runScriptExitCode(cfg, ["stat %{test-tools-dir}/clang_tidy_checks/libcxx-tidy.plugin"]) != 0:
31-
return None
32-
33-
# TODO MODULES require ToT due module specific fixes.
34-
if runScriptExitCode(cfg, ['clang-tidy-18 --version']) == 0:
35-
return 'clang-tidy-18'
36-
37-
# TODO This should be the last stable release.
38-
# LLVM RELEASE bump to latest stable version
39-
if runScriptExitCode(cfg, ["clang-tidy-16 --version"]) == 0:
40-
return "clang-tidy-16"
41-
42-
# LLVM RELEASE bump version
43-
if int(re.search("[0-9]+", commandOutput(cfg, ["clang-tidy --version"])).group()) >= 16:
44-
return "clang-tidy"
45-
46-
except ConfigurationRuntimeError:
47-
return None
48-
49-
5026
def _getAndroidDeviceApi(cfg):
5127
return int(
5228
programOutput(
@@ -297,13 +273,6 @@ def _getAndroidDeviceApi(cfg):
297273
name="executor-has-no-bash",
298274
when=lambda cfg: runScriptExitCode(cfg, ["%{exec} bash -c 'bash --version'"]) != 0,
299275
),
300-
Feature(
301-
name="has-clang-tidy",
302-
when=lambda cfg: _getSuitableClangTidy(cfg) is not None,
303-
actions=[
304-
AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
305-
],
306-
),
307276
# Whether module support for the platform is available.
308277
Feature(
309278
name="has-no-cxx-module-support",

libcxx/utils/libcxx/test/params.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,37 @@ def getSizeOptimizationFlag(cfg):
110110
)
111111

112112

113+
def testClangTidy(cfg, version, executable):
114+
try:
115+
if version in commandOutput(cfg, [f"{executable} --version"]):
116+
return executable
117+
except ConfigurationRuntimeError:
118+
return None
119+
120+
121+
def getSuitableClangTidy(cfg):
122+
# If we didn't build the libcxx-tidy plugin via CMake, we can't run the clang-tidy tests.
123+
if (
124+
runScriptExitCode(
125+
cfg, ["stat %{test-tools-dir}/clang_tidy_checks/libcxx-tidy.plugin"]
126+
)
127+
!= 0
128+
):
129+
return None
130+
131+
version = "{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(
132+
**compilerMacros(cfg)
133+
)
134+
exe = testClangTidy(
135+
cfg, version, "clang-tidy-{__clang_major__}".format(**compilerMacros(cfg))
136+
)
137+
138+
if not exe:
139+
exe = testClangTidy(cfg, version, "clang-tidy")
140+
141+
return exe
142+
143+
113144
# fmt: off
114145
DEFAULT_PARAMETERS = [
115146
Parameter(
@@ -366,6 +397,16 @@ def getSizeOptimizationFlag(cfg):
366397
default=f"{shlex.quote(sys.executable)} {shlex.quote(str(Path(__file__).resolve().parent.parent.parent / 'run.py'))}",
367398
help="Custom executor to use instead of the configured default.",
368399
actions=lambda executor: [AddSubstitution("%{executor}", executor)],
369-
)
400+
),
401+
Parameter(
402+
name='clang-tidy-executable',
403+
type=str,
404+
default=lambda cfg: getSuitableClangTidy(cfg),
405+
help="Selects the clang-tidy executable to use.",
406+
actions=lambda exe: [] if exe is None else [
407+
AddFeature('has-clang-tidy'),
408+
AddSubstitution('%{clang-tidy}', exe),
409+
]
410+
),
370411
]
371412
# fmt: on

0 commit comments

Comments
 (0)