[libcxx-commits] [libcxx] [llvm] [RFC][libc++] Reworks clang-tidy selection. (PR #81362)
Mark de Wever via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Mar 9 03:32:44 PST 2024
https://github.com/mordante updated https://github.com/llvm/llvm-project/pull/81362
>From 2dcc61840ede365775f8cd974c6ea73e43c0598a Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 10 Feb 2024 17:05:37 +0100
Subject: [PATCH 1/2] [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.)
---
cmake/Modules/LLVMVersion.cmake | 15 +++++++
libcxx/test/tools/CMakeLists.txt | 5 +++
.../tools/clang_tidy_checks/CMakeLists.txt | 8 ++--
libcxx/utils/libcxx/test/features.py | 31 -------------
libcxx/utils/libcxx/test/params.py | 43 ++++++++++++++++++-
llvm/CMakeLists.txt | 13 +-----
runtimes/CMakeLists.txt | 2 +
7 files changed, 69 insertions(+), 48 deletions(-)
create mode 100644 cmake/Modules/LLVMVersion.cmake
diff --git a/cmake/Modules/LLVMVersion.cmake b/cmake/Modules/LLVMVersion.cmake
new file mode 100644
index 00000000000000..5e28283fbc1c60
--- /dev/null
+++ b/cmake/Modules/LLVMVersion.cmake
@@ -0,0 +1,15 @@
+# The LLVM Version number information
+
+if(NOT DEFINED LLVM_VERSION_MAJOR)
+ set(LLVM_VERSION_MAJOR 19)
+endif()
+if(NOT DEFINED LLVM_VERSION_MINOR)
+ set(LLVM_VERSION_MINOR 0)
+endif()
+if(NOT DEFINED LLVM_VERSION_PATCH)
+ set(LLVM_VERSION_PATCH 0)
+endif()
+if(NOT DEFINED LLVM_VERSION_SUFFIX)
+ set(LLVM_VERSION_SUFFIX git)
+endif()
+
diff --git a/libcxx/test/tools/CMakeLists.txt b/libcxx/test/tools/CMakeLists.txt
index 10be63e8b50aa5..e30ad6cdd8201f 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 978e7095216522..74905a0c3ed1c4 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 6ef40755c59d97..3f0dc0c50a0d00 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -23,30 +23,6 @@
_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 @@ def _getAndroidDeviceApi(cfg):
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 89d9f22e9dc6df..695e01115aa4ba 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 @@ def getSizeOptimizationFlag(cfg):
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
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 111c8cfa15d828..02162fce24293c 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -15,18 +15,7 @@ if(NOT LLVM_NO_INSTALL_NAME_DIR_FOR_BUILD_TREE)
set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
endif()
-if(NOT DEFINED LLVM_VERSION_MAJOR)
- set(LLVM_VERSION_MAJOR 19)
-endif()
-if(NOT DEFINED LLVM_VERSION_MINOR)
- set(LLVM_VERSION_MINOR 0)
-endif()
-if(NOT DEFINED LLVM_VERSION_PATCH)
- set(LLVM_VERSION_PATCH 0)
-endif()
-if(NOT DEFINED LLVM_VERSION_SUFFIX)
- set(LLVM_VERSION_SUFFIX git)
-endif()
+include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
set_directory_properties(PROPERTIES LLVM_VERSION_MAJOR "${LLVM_VERSION_MAJOR}")
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 29b47b862c219c..6f24fbcccec955 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -6,6 +6,8 @@ set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
NO_POLICY_SCOPE)
+include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
+
project(Runtimes C CXX ASM)
list(INSERT CMAKE_MODULE_PATH 0
>From 1093af3da63311277b6ccab73f5e194660721d77 Mon Sep 17 00:00:00 2001
From: Mark de Wever <koraq at xs4all.nl>
Date: Sat, 9 Mar 2024 12:31:47 +0100
Subject: [PATCH 2/2] Undo version number changes.
---
cmake/Modules/LLVMVersion.cmake | 15 ---------------
llvm/CMakeLists.txt | 13 ++++++++++++-
runtimes/CMakeLists.txt | 2 --
3 files changed, 12 insertions(+), 18 deletions(-)
delete mode 100644 cmake/Modules/LLVMVersion.cmake
diff --git a/cmake/Modules/LLVMVersion.cmake b/cmake/Modules/LLVMVersion.cmake
deleted file mode 100644
index 5e28283fbc1c60..00000000000000
--- a/cmake/Modules/LLVMVersion.cmake
+++ /dev/null
@@ -1,15 +0,0 @@
-# The LLVM Version number information
-
-if(NOT DEFINED LLVM_VERSION_MAJOR)
- set(LLVM_VERSION_MAJOR 19)
-endif()
-if(NOT DEFINED LLVM_VERSION_MINOR)
- set(LLVM_VERSION_MINOR 0)
-endif()
-if(NOT DEFINED LLVM_VERSION_PATCH)
- set(LLVM_VERSION_PATCH 0)
-endif()
-if(NOT DEFINED LLVM_VERSION_SUFFIX)
- set(LLVM_VERSION_SUFFIX git)
-endif()
-
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 02162fce24293c..111c8cfa15d828 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -15,7 +15,18 @@ if(NOT LLVM_NO_INSTALL_NAME_DIR_FOR_BUILD_TREE)
set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
endif()
-include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
+if(NOT DEFINED LLVM_VERSION_MAJOR)
+ set(LLVM_VERSION_MAJOR 19)
+endif()
+if(NOT DEFINED LLVM_VERSION_MINOR)
+ set(LLVM_VERSION_MINOR 0)
+endif()
+if(NOT DEFINED LLVM_VERSION_PATCH)
+ set(LLVM_VERSION_PATCH 0)
+endif()
+if(NOT DEFINED LLVM_VERSION_SUFFIX)
+ set(LLVM_VERSION_SUFFIX git)
+endif()
set_directory_properties(PROPERTIES LLVM_VERSION_MAJOR "${LLVM_VERSION_MAJOR}")
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 6f24fbcccec955..29b47b862c219c 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -6,8 +6,6 @@ set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
NO_POLICY_SCOPE)
-include(${LLVM_COMMON_CMAKE_UTILS}/Modules/LLVMVersion.cmake)
-
project(Runtimes C CXX ASM)
list(INSERT CMAKE_MODULE_PATH 0
More information about the libcxx-commits
mailing list