[libcxx] [llvm] [RFC][libc++] Reworks clang-tidy selection. (PR #81362)
Mark de Wever via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 10 08:08:00 PST 2024
https://github.com/mordante created https://github.com/llvm/llvm-project/pull/81362
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.
>From 10a15df28061f1898c6beaa3e4e38f2fcbc52859 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] [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.
---
cmake/Modules/LLVMVersion.cmake | 15 ++++
libcxx/CMakeLists.txt | 4 +
libcxx/test/configs/cmake-bridge.cfg.in | 1 +
.../tools/clang_tidy_checks/CMakeLists.txt | 78 ++++++++++++++++++-
libcxx/utils/ci/run-buildbot | 9 +++
libcxx/utils/libcxx/test/config.py | 7 ++
libcxx/utils/libcxx/test/features.py | 30 +------
llvm/CMakeLists.txt | 13 +---
runtimes/CMakeLists.txt | 2 +
9 files changed, 117 insertions(+), 42 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/CMakeLists.txt b/libcxx/CMakeLists.txt
index d392e95077ac57..fbe426a253f182 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -124,6 +124,10 @@ option(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS
the shared library they shipped should turn this on and see `include/__availability`
for more details." OFF)
option(LIBCXX_ENABLE_CLANG_TIDY "Whether to compile and run clang-tidy checks" OFF)
+set(LIBCXX_CLANG_TIDY_EXECUTABLE "" CACHE FILEPATH
+ "The clang-tidy executable to use to run the clang-tidy checks.
+ When the variable is not set cmake will search for clang-tidy.
+ This is only used when LIBCXX_ENABLE_CLANG_TIDY is ON.")
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(LIBCXX_DEFAULT_TEST_CONFIG "llvm-libc++-shared-gcc.cfg.in")
diff --git a/libcxx/test/configs/cmake-bridge.cfg.in b/libcxx/test/configs/cmake-bridge.cfg.in
index 84b3270a8940ac..1f294af48818a3 100644
--- a/libcxx/test/configs/cmake-bridge.cfg.in
+++ b/libcxx/test/configs/cmake-bridge.cfg.in
@@ -25,6 +25,7 @@ config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
# Add substitutions for bootstrapping the test suite configuration
import shlex
config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+config.substitutions.append(('%{clang-tidy}', shlex.quote('@LIBCXX_CLANG_TIDY@')))
config.substitutions.append(('%{libcxx-dir}', '@LIBCXX_SOURCE_DIR@'))
config.substitutions.append(('%{include-dir}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
config.substitutions.append(('%{target-include-dir}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
index 978e7095216522..1b195f72319eda 100644
--- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
+++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
@@ -5,11 +5,82 @@
set(LLVM_DIR_SAVE ${LLVM_DIR})
set(Clang_DIR_SAVE ${Clang_DIR})
-find_package(Clang 18)
-if (NOT Clang_FOUND)
- find_package(Clang 17)
+# libc++ only supports the latest two released versions of clang-tidy. During
+# the release period there the new unreleased version is also supported. To keep
+# this code simple, always accept the last 3 versions.
+set(HEAD ${LLVM_VERSION_MAJOR})
+math(EXPR MINUS_1 "${HEAD} - 1")
+math(EXPR MINUS_2 "${MINUS_1} - 1")
+math(EXPR MINUS_3 "${MINUS_2} - 1")
+
+if("${LIBCXX_CLANG_TIDY_EXECUTABLE}" STREQUAL "")
+ set(NAMES "clang-tidy-${HEAD}"
+ "clang-tidy-${MINUS_1}"
+ "clang-tidy-${MINUS_2}"
+ "clang-tidy-${MINUS_3}"
+ "clang-tidy")
+ if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+ string(REGEX MATCH
+ "[0-9]+"
+ LIBCXX_CLANG_MAJOR
+ "${CMAKE_CXX_COMPILER_VERSION}")
+ set(NAMES "clang-tidy-${LIBCXX_CLANG_MAJOR}"
+ "clang-tidy")
+ endif()
+ find_program(LIBCXX_CLANG_TIDY NAMES ${NAMES})
+else()
+ set(LIBCXX_CLANG_TIDY "${LIBCXX_CLANG_TIDY_EXECUTABLE}" CACHE INTERNAL "")
+endif()
+
+if("${LIBCXX_CLANG_TIDY}" STREQUAL "")
+ set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
+ message(STATUS "Could not find a clang-tidy executable;
+ custom libc++ clang-tidy checks will not be available.")
+ return()
+endif()
+
+execute_process(COMMAND
+ "${LIBCXX_CLANG_TIDY}" "--version"
+ OUTPUT_VARIABLE CLANG_TIDY_VERSION_OUTPUT
+ COMMAND_ERROR_IS_FATAL ANY)
+string(REGEX MATCH
+ "[0-9]+\.[0-9]+\.[0-9]+"
+ LIBCXX_CLANG_TIDY_VERSION
+ "${CLANG_TIDY_VERSION_OUTPUT}")
+
+if("${LIBCXX_CLANG_TIDY_VERSION}" VERSION_LESS "${MINUS_3}")
+ set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
+ message(STATUS "The clang-tidy version found ${LIBCXX_CLANG_TIDY_VERSION} is
+ too old, version ${MINUS_3}.0.0 is required;
+ custom libc++ clang-tidy checks will not be available.")
+ return()
endif()
+# For modules clang-tidy 17 or newer is required. This is due to the state of
+# implementation in clang/clang-tidy. This additional restriction should be
+# temporary.
+# TODO(LLVM-19) Remove this check
+if("${LIBCXX_CLANG_TIDY_VERSION}" VERSION_LESS "17.0.0")
+ message(STATUS "The clang-tidy version found ${LIBCXX_CLANG_TIDY_VERSION} is
+ too old, version 17.0.0 is required;
+ custom libc++ clang-tidy checks will not be available.")
+endif()
+
+if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND
+ NOT CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL "${LIBCXX_CLANG_TIDY_VERSION}")
+
+ set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
+ message(STATUS "The version of clang ${CMAKE_CXX_COMPILER_VERSION} differs
+ from the version of clang-tidy ${LIBCXX_CLANG_TIDY_VERSION};
+ custom libc++ clang-tidy checks will not be available.")
+ return()
+endif()
+
+# Since the Clang C++ ABI is not stable the Clang libraries and clang-tidy
+# version 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 "${LIBCXX_CLANG_TIDY_VERSION}")
+
set(SOURCES
abi_tag_on_virtual.cpp
header_exportable_declarations.cpp
@@ -23,6 +94,7 @@ set(SOURCES
)
if(NOT Clang_FOUND)
+ set(LIBCXX_CLANG_TIDY "" CACHE INTERNAL "")
message(STATUS "Could not find a suitable version of the Clang development package;
custom libc++ clang-tidy checks will not be available.")
return()
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index ddfe53d2d8bd63..3719ab7f0453bb 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -47,6 +47,10 @@ CLANG_FORMAT The clang-format binary to use when generating the format
ENABLE_CLANG_TIDY Whether to compile and run clang-tidy checks. This variable
is optional.
+CLANG_TIDY_EXECUTABLE
+ The clang-tidy executable to use. Its version should match
+ the clang version used. This variable is optional.
+
EOF
}
@@ -115,6 +119,10 @@ if [ -z "${ENABLE_CLANG_TIDY}" ]; then
ENABLE_CLANG_TIDY=Off
fi
+if [ -z "${CLANG_TIDY_EXECUTABLE}" ]; then
+ CLANG_TIDY_EXECUTABLE=''
+fi
+
function generate-cmake-base() {
echo "--- Generating CMake"
${CMAKE} \
@@ -127,6 +135,7 @@ function generate-cmake-base() {
-DLIBCXXABI_ENABLE_WERROR=YES \
-DLIBUNWIND_ENABLE_WERROR=YES \
-DLIBCXX_ENABLE_CLANG_TIDY=${ENABLE_CLANG_TIDY} \
+ -DLIBCXX_CLANG_TIDY_EXECUTABLE=${CLANG_TIDY_EXECUTABLE} \
-DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests" \
"${@}"
}
diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index 9a71c494030d70..1b19ea29120c80 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -9,6 +9,13 @@
import os
+def _hasSubstitution(substitution, config):
+ for k, v in config.substitutions:
+ if k == substitution:
+ return True
+ return False
+
+
def _getSubstitution(substitution, config):
for (orig, replacement) in config.substitutions:
if orig == substitution:
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 6ef40755c59d97..da4aa3b8447097 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -7,6 +7,7 @@
# ===----------------------------------------------------------------------===##
from libcxx.test.dsl import *
+import libcxx.test.config as config
from lit.BooleanExpression import BooleanExpression
import re
import shutil
@@ -24,29 +25,6 @@
_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(
@@ -299,10 +277,8 @@ def _getAndroidDeviceApi(cfg):
),
Feature(
name="has-clang-tidy",
- when=lambda cfg: _getSuitableClangTidy(cfg) is not None,
- actions=[
- AddSubstitution("%{clang-tidy}", lambda cfg: _getSuitableClangTidy(cfg))
- ],
+ when=lambda cfg: config._hasSubstitution("%{clang-tidy}", cfg)
+ and config._getSubstitution("%{clang-tidy}", cfg) != "",
),
# Whether module support for the platform is available.
Feature(
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 81f2753a4edd85..0f83a3e2212552 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)
if (NOT PACKAGE_VERSION)
set(PACKAGE_VERSION
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 634ffe710b06b8..d9415237cbefde 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
More information about the llvm-commits
mailing list