aboutsummaryrefslogtreecommitdiff
path: root/libcxx
diff options
context:
space:
mode:
authorMark de Wever <koraq@xs4all.nl>2024-03-09 18:03:52 +0100
committerGitHub <noreply@github.com>2024-03-09 18:03:52 +0100
commite19e8600cf743690e1a23fb8a2b0dfbe2dafe559 (patch)
treed1c689144162ab6af9585d2bdd3252a1d64321fe /libcxx
parent99f5e9634b6921ebb6dad9bef1c76ade83adc847 (diff)
downloadllvm-e19e8600cf743690e1a23fb8a2b0dfbe2dafe559.zip
llvm-e19e8600cf743690e1a23fb8a2b0dfbe2dafe559.tar.gz
llvm-e19e8600cf743690e1a23fb8a2b0dfbe2dafe559.tar.bz2
[RFC][libc++] Reworks clang-tidy selection. (#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.)
Diffstat (limited to 'libcxx')
-rw-r--r--libcxx/test/tools/CMakeLists.txt5
-rw-r--r--libcxx/test/tools/clang_tidy_checks/CMakeLists.txt8
-rw-r--r--libcxx/utils/libcxx/test/features.py31
-rw-r--r--libcxx/utils/libcxx/test/params.py43
4 files changed, 51 insertions, 36 deletions
diff --git a/libcxx/test/tools/CMakeLists.txt b/libcxx/test/tools/CMakeLists.txt
index 10be63e..e30ad6c 100644
--- a/libcxx/test/tools/CMakeLists.txt
+++ b/libcxx/test/tools/CMakeLists.txt
@@ -3,5 +3,10 @@ set(LIBCXX_TEST_TOOLS_PATH ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)
# TODO: Remove LIBCXX_ENABLE_CLANG_TIDY
if(LIBCXX_ENABLE_CLANG_TIDY)
+ if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ message(STATUS "Clang-tidy can only be used when building libc++ with "
+ "a clang compiler.")
+ return()
+ endif()
add_subdirectory(clang_tidy_checks)
endif()
diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
index 978e709..74905a0 100644
--- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
+++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
@@ -5,10 +5,10 @@
set(LLVM_DIR_SAVE ${LLVM_DIR})
set(Clang_DIR_SAVE ${Clang_DIR})
-find_package(Clang 18)
-if (NOT Clang_FOUND)
- find_package(Clang 17)
-endif()
+# Since the Clang C++ ABI is not stable the Clang libraries and clang-tidy
+# versions must match. Otherwise there likely will be ODR-violations. This had
+# led to crashes and incorrect output of the clang-tidy based checks.
+find_package(Clang ${CMAKE_CXX_COMPILER_VERSION})
set(SOURCES
abi_tag_on_virtual.cpp
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 6ef4075..3f0dc0c5 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -23,30 +23,6 @@ _isClExe = lambda cfg: not _isAnyClangOrGCC(cfg)
_isMSVC = lambda cfg: "_MSC_VER" in compilerMacros(cfg)
_msvcVersion = lambda cfg: (int(compilerMacros(cfg)["_MSC_VER"]) // 100, int(compilerMacros(cfg)["_MSC_VER"]) % 100)
-
-def _getSuitableClangTidy(cfg):
- try:
- # If we didn't build the libcxx-tidy plugin via CMake, we can't run the clang-tidy tests.
- if runScriptExitCode(cfg, ["stat %{test-tools-dir}/clang_tidy_checks/libcxx-tidy.plugin"]) != 0:
- return None
-
- # TODO MODULES require ToT due module specific fixes.
- if runScriptExitCode(cfg, ['clang-tidy-18 --version']) == 0:
- return 'clang-tidy-18'
-
- # TODO This should be the last stable release.
- # LLVM RELEASE bump to latest stable version
- if runScriptExitCode(cfg, ["clang-tidy-16 --version"]) == 0:
- return "clang-tidy-16"
-
- # LLVM RELEASE bump version
- if int(re.search("[0-9]+", commandOutput(cfg, ["clang-tidy --version"])).group()) >= 16:
- return "clang-tidy"
-
- except ConfigurationRuntimeError:
- return None
-
-
def _getAndroidDeviceApi(cfg):
return int(
programOutput(
@@ -297,13 +273,6 @@ DEFAULT_FEATURES = [
name="executor-has-no-bash",
when=lambda cfg: runScriptExitCode(cfg, ["%{exec} bash -c 'bash --version'"]) != 0,
),
- Feature(
- name="has-clang-tidy",
- when=lambda cfg: _getSuitableClangTidy(cfg) is not None,
- actions=[
- AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
- ],
- ),
# Whether module support for the platform is available.
Feature(
name="has-no-cxx-module-support",
diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index 89d9f22..695e011 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -110,6 +110,37 @@ def getSizeOptimizationFlag(cfg):
)
+def testClangTidy(cfg, version, executable):
+ try:
+ if version in commandOutput(cfg, [f"{executable} --version"]):
+ return executable
+ except ConfigurationRuntimeError:
+ return None
+
+
+def getSuitableClangTidy(cfg):
+ # If we didn't build the libcxx-tidy plugin via CMake, we can't run the clang-tidy tests.
+ if (
+ runScriptExitCode(
+ cfg, ["stat %{test-tools-dir}/clang_tidy_checks/libcxx-tidy.plugin"]
+ )
+ != 0
+ ):
+ return None
+
+ version = "{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}".format(
+ **compilerMacros(cfg)
+ )
+ exe = testClangTidy(
+ cfg, version, "clang-tidy-{__clang_major__}".format(**compilerMacros(cfg))
+ )
+
+ if not exe:
+ exe = testClangTidy(cfg, version, "clang-tidy")
+
+ return exe
+
+
# fmt: off
DEFAULT_PARAMETERS = [
Parameter(
@@ -366,6 +397,16 @@ DEFAULT_PARAMETERS = [
default=f"{shlex.quote(sys.executable)} {shlex.quote(str(Path(__file__).resolve().parent.parent.parent / 'run.py'))}",
help="Custom executor to use instead of the configured default.",
actions=lambda executor: [AddSubstitution("%{executor}", executor)],
- )
+ ),
+ Parameter(
+ name='clang-tidy-executable',
+ type=str,
+ default=lambda cfg: getSuitableClangTidy(cfg),
+ help="Selects the clang-tidy executable to use.",
+ actions=lambda exe: [] if exe is None else [
+ AddFeature('has-clang-tidy'),
+ AddSubstitution('%{clang-tidy}', exe),
+ ]
+ ),
]
# fmt: on