Skip to content

Commit 10a15df

Browse files
committed
[RFC][libc++] Reworks clang-tidy selection.
The current selection is done in the test scipts 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 binaray sometimes causes issues with supported diagnotic 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 a 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.) Note the LLVM version changes should reviewed seperately.
1 parent f66f44e commit 10a15df

File tree

9 files changed

+117
-42
lines changed

9 files changed

+117
-42
lines changed

cmake/Modules/LLVMVersion.cmake

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# The LLVM Version number information
2+
3+
if(NOT DEFINED LLVM_VERSION_MAJOR)
4+
set(LLVM_VERSION_MAJOR 19)
5+
endif()
6+
if(NOT DEFINED LLVM_VERSION_MINOR)
7+
set(LLVM_VERSION_MINOR 0)
8+
endif()
9+
if(NOT DEFINED LLVM_VERSION_PATCH)
10+
set(LLVM_VERSION_PATCH 0)
11+
endif()
12+
if(NOT DEFINED LLVM_VERSION_SUFFIX)
13+
set(LLVM_VERSION_SUFFIX git)
14+
endif()
15+

libcxx/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ option(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS
124124
the shared library they shipped should turn this on and see `include/__availability`
125125
for more details." OFF)
126126
option(LIBCXX_ENABLE_CLANG_TIDY "Whether to compile and run clang-tidy checks" OFF)
127+
set(LIBCXX_CLANG_TIDY_EXECUTABLE "" CACHE FILEPATH
128+
"The clang-tidy executable to use to run the clang-tidy checks.
129+
When the variable is not set cmake will search for clang-tidy.
130+
This is only used when LIBCXX_ENABLE_CLANG_TIDY is ON.")
127131

128132
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
129133
set(LIBCXX_DEFAULT_TEST_CONFIG "llvm-libc++-shared-gcc.cfg.in")

libcxx/test/configs/cmake-bridge.cfg.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
2525
# Add substitutions for bootstrapping the test suite configuration
2626
import shlex
2727
config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
28+
config.substitutions.append(('%{clang-tidy}', shlex.quote('@LIBCXX_CLANG_TIDY@')))
2829
config.substitutions.append(('%{libcxx-dir}', '@LIBCXX_SOURCE_DIR@'))
2930
config.substitutions.append(('%{include-dir}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
3031
config.substitutions.append(('%{target-include-dir}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))

libcxx/test/tools/clang_tidy_checks/CMakeLists.txt

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,82 @@
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)
8+
# libc++ only supports the latest two released versions of clang-tidy. During
9+
# the release period there the new unreleased version is also supported. To keep
10+
# this code simple, always accept the last 3 versions.
11+
set(HEAD ${LLVM_VERSION_MAJOR})
12+
math(EXPR MINUS_1 "${HEAD} - 1")
13+
math(EXPR MINUS_2 "${MINUS_1} - 1")
14+
math(EXPR MINUS_3 "${MINUS_2} - 1")
15+
16+
if("${LIBCXX_CLANG_TIDY_EXECUTABLE}" STREQUAL "")
17+
set(NAMES "clang-tidy-${HEAD}"
18+
"clang-tidy-${MINUS_1}"
19+
"clang-tidy-${MINUS_2}"
20+
"clang-tidy-${MINUS_3}"
21+
"clang-tidy")
22+
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
23+
string(REGEX MATCH
24+
"[0-9]+"
25+
LIBCXX_CLANG_MAJOR
26+
"${CMAKE_CXX_COMPILER_VERSION}")
27+
set(NAMES "clang-tidy-${LIBCXX_CLANG_MAJOR}"
28+
"clang-tidy")
29+
endif()
30+
find_program(LIBCXX_CLANG_TIDY NAMES ${NAMES})
31+
else()
32+
set(LIBCXX_CLANG_TIDY "${LIBCXX_CLANG_TIDY_EXECUTABLE}" CACHE INTERNAL "")
33+
endif()
34+
35+
if("${LIBCXX_CLANG_TIDY}" STREQUAL "")
36+
set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
37+
message(STATUS "Could not find a clang-tidy executable;
38+
custom libc++ clang-tidy checks will not be available.")
39+
return()
40+
endif()
41+
42+
execute_process(COMMAND
43+
"${LIBCXX_CLANG_TIDY}" "--version"
44+
OUTPUT_VARIABLE CLANG_TIDY_VERSION_OUTPUT
45+
COMMAND_ERROR_IS_FATAL ANY)
46+
string(REGEX MATCH
47+
"[0-9]+\.[0-9]+\.[0-9]+"
48+
LIBCXX_CLANG_TIDY_VERSION
49+
"${CLANG_TIDY_VERSION_OUTPUT}")
50+
51+
if("${LIBCXX_CLANG_TIDY_VERSION}" VERSION_LESS "${MINUS_3}")
52+
set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
53+
message(STATUS "The clang-tidy version found ${LIBCXX_CLANG_TIDY_VERSION} is
54+
too old, version ${MINUS_3}.0.0 is required;
55+
custom libc++ clang-tidy checks will not be available.")
56+
return()
1157
endif()
1258

59+
# For modules clang-tidy 17 or newer is required. This is due to the state of
60+
# implementation in clang/clang-tidy. This additional restriction should be
61+
# temporary.
62+
# TODO(LLVM-19) Remove this check
63+
if("${LIBCXX_CLANG_TIDY_VERSION}" VERSION_LESS "17.0.0")
64+
message(STATUS "The clang-tidy version found ${LIBCXX_CLANG_TIDY_VERSION} is
65+
too old, version 17.0.0 is required;
66+
custom libc++ clang-tidy checks will not be available.")
67+
endif()
68+
69+
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND
70+
NOT CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL "${LIBCXX_CLANG_TIDY_VERSION}")
71+
72+
set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
73+
message(STATUS "The version of clang ${CMAKE_CXX_COMPILER_VERSION} differs
74+
from the version of clang-tidy ${LIBCXX_CLANG_TIDY_VERSION};
75+
custom libc++ clang-tidy checks will not be available.")
76+
return()
77+
endif()
78+
79+
# Since the Clang C++ ABI is not stable the Clang libraries and clang-tidy
80+
# version must match. Otherwise there likely will be ODR-violations. This had
81+
# led to crashes and incorrect output of the clang-tidy based checks.
82+
find_package(Clang "${LIBCXX_CLANG_TIDY_VERSION}")
83+
1384
set(SOURCES
1485
abi_tag_on_virtual.cpp
1586
header_exportable_declarations.cpp
@@ -23,6 +94,7 @@ set(SOURCES
2394
)
2495

2596
if(NOT Clang_FOUND)
97+
set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
2698
message(STATUS "Could not find a suitable version of the Clang development package;
2799
custom libc++ clang-tidy checks will not be available.")
28100
return()

libcxx/utils/ci/run-buildbot

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ CLANG_FORMAT The clang-format binary to use when generating the format
4747
ENABLE_CLANG_TIDY Whether to compile and run clang-tidy checks. This variable
4848
is optional.
4949
50+
CLANG_TIDY_EXECUTABLE
51+
The clang-tidy executable to use. Its version should match
52+
the clang version used. This variable is optional.
53+
5054
EOF
5155
}
5256

@@ -115,6 +119,10 @@ if [ -z "${ENABLE_CLANG_TIDY}" ]; then
115119
ENABLE_CLANG_TIDY=Off
116120
fi
117121

122+
if [ -z "${CLANG_TIDY_EXECUTABLE}" ]; then
123+
CLANG_TIDY_EXECUTABLE=''
124+
fi
125+
118126
function generate-cmake-base() {
119127
echo "--- Generating CMake"
120128
${CMAKE} \
@@ -127,6 +135,7 @@ function generate-cmake-base() {
127135
-DLIBCXXABI_ENABLE_WERROR=YES \
128136
-DLIBUNWIND_ENABLE_WERROR=YES \
129137
-DLIBCXX_ENABLE_CLANG_TIDY=${ENABLE_CLANG_TIDY} \
138+
-DLIBCXX_CLANG_TIDY_EXECUTABLE=${CLANG_TIDY_EXECUTABLE} \
130139
-DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests" \
131140
"${@}"
132141
}

libcxx/utils/libcxx/test/config.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@
99
import os
1010

1111

12+
def _hasSubstitution(substitution, config):
13+
for k, v in config.substitutions:
14+
if k == substitution:
15+
return True
16+
return False
17+
18+
1219
def _getSubstitution(substitution, config):
1320
for (orig, replacement) in config.substitutions:
1421
if orig == substitution:

libcxx/utils/libcxx/test/features.py

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# ===----------------------------------------------------------------------===##
88

99
from libcxx.test.dsl import *
10+
import libcxx.test.config as config
1011
from lit.BooleanExpression import BooleanExpression
1112
import re
1213
import shutil
@@ -24,29 +25,6 @@
2425
_msvcVersion = lambda cfg: (int(compilerMacros(cfg)["_MSC_VER"]) // 100, int(compilerMacros(cfg)["_MSC_VER"]) % 100)
2526

2627

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-
5028
def _getAndroidDeviceApi(cfg):
5129
return int(
5230
programOutput(
@@ -299,10 +277,8 @@ def _getAndroidDeviceApi(cfg):
299277
),
300278
Feature(
301279
name="has-clang-tidy",
302-
when=lambda cfg: _getSuitableClangTidy(cfg) is not None,
303-
actions=[
304-
AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
305-
],
280+
when=lambda cfg: config._hasSubstitution("%{clang-tidy}", cfg)
281+
and config._getSubstitution("%{clang-tidy}", cfg) != "",
306282
),
307283
# Whether module support for the platform is available.
308284
Feature(

llvm/CMakeLists.txt

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,7 @@ if(NOT LLVM_NO_INSTALL_NAME_DIR_FOR_BUILD_TREE)
1515
set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
1616
endif()
1717

18-
if(NOT DEFINED LLVM_VERSION_MAJOR)
19-
set(LLVM_VERSION_MAJOR 19)
20-
endif()
21-
if(NOT DEFINED LLVM_VERSION_MINOR)
22-
set(LLVM_VERSION_MINOR 0)
23-
endif()
24-
if(NOT DEFINED LLVM_VERSION_PATCH)
25-
set(LLVM_VERSION_PATCH 0)
26-
endif()
27-
if(NOT DEFINED LLVM_VERSION_SUFFIX)
28-
set(LLVM_VERSION_SUFFIX git)
29-
endif()
18+
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
3019

3120
if (NOT PACKAGE_VERSION)
3221
set(PACKAGE_VERSION

runtimes/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
66
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
77
NO_POLICY_SCOPE)
88

9+
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
10+
911
project(Runtimes C CXX ASM)
1012

1113
list(INSERT CMAKE_MODULE_PATH 0

0 commit comments

Comments
 (0)