From 79921fbd5c6223ff7e6c75ed75974b4d16cad529 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Wed, 8 May 2024 20:17:59 +0200 Subject: [PATCH] [libc++][CI] Reenables clang-tidy. (#90077) The patch does several things: - fixes module exports - disables clang-tidy with Clang-17 due to known issues - disabled clang-tidy on older libstdc++ versions since it lacks C++20 features used - fixes the CMake dependency The issue why clang-tidy was not used in the CI was the last issue; the plugin was not a dependency of the tests. Without a plugin the tests disable clang-tidy. This was noticed while investigating https://github.com/llvm/llvm-project/issues/89898 --- libcxx/modules/std/chrono.inc | 18 +++++++++----- libcxx/modules/std/ranges.inc | 7 +++--- libcxx/test/CMakeLists.txt | 6 +++++ libcxx/test/libcxx/clang_tidy.gen.py | 3 +++ .../tools/clang_tidy_checks/CMakeLists.txt | 24 +++++++++++++++++-- 5 files changed, 47 insertions(+), 11 deletions(-) diff --git a/libcxx/modules/std/chrono.inc b/libcxx/modules/std/chrono.inc index 1265e21dc54ef6..813322a1797f6a 100644 --- a/libcxx/modules/std/chrono.inc +++ b/libcxx/modules/std/chrono.inc @@ -190,10 +190,11 @@ export namespace std { using std::chrono::make12; using std::chrono::make24; -#if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \ - !defined(_LIBCPP_HAS_NO_LOCALIZATION) +#ifdef _LIBCPP_ENABLE_EXPERIMENTAL + +# if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \ + !defined(_LIBCPP_HAS_NO_LOCALIZATION) -# ifdef _LIBCPP_ENABLE_EXPERIMENTAL // [time.zone.db], time zone database using std::chrono::tzdb; using std::chrono::tzdb_list; @@ -213,11 +214,16 @@ export namespace std { using std::chrono::ambiguous_local_time; using std::chrono::nonexistent_local_time; # endif // if 0 +# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && + // !defined(_LIBCPP_HAS_NO_LOCALIZATION) // [time.zone.info], information classes using std::chrono::local_info; using std::chrono::sys_info; +# if !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && \ + !defined(_LIBCPP_HAS_NO_LOCALIZATION) + # if 0 // [time.zone.timezone], class time_zone using std::chrono::choose; @@ -246,9 +252,9 @@ export namespace std { // [time.format], formatting using std::chrono::local_time_format; # endif -# endif // _LIBCPP_ENABLE_EXPERIMENTAL -#endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && - // !defined(_LIBCPP_HAS_NO_LOCALIZATION) +# endif // !defined(_LIBCPP_HAS_NO_TIME_ZONE_DATABASE) && !defined(_LIBCPP_HAS_NO_FILESYSTEM) && + // !defined(_LIBCPP_HAS_NO_LOCALIZATION) +#endif // _LIBCPP_ENABLE_EXPERIMENTAL } // namespace chrono diff --git a/libcxx/modules/std/ranges.inc b/libcxx/modules/std/ranges.inc index 80f31c79a1a405..f71efe948ede10 100644 --- a/libcxx/modules/std/ranges.inc +++ b/libcxx/modules/std/ranges.inc @@ -138,9 +138,6 @@ export namespace std { } #endif // _LIBCPP_HAS_NO_LOCALIZATION -#if _LIBCPP_STD_VER >= 23 - // [range.adaptor.object], range adaptor objects - using std::ranges::range_adaptor_closure; // Note: This declaration not in the synopsis or explicitly in the wording. // However it is needed for the range adaptors. // [range.adaptor.object]/3 @@ -151,7 +148,11 @@ export namespace std { // involving an object of type cv D as an operand to the | operator is // undefined if overload resolution selects a program-defined operator| // function. + // This is used internally in C++20 mode. using std::ranges::operator|; +#if _LIBCPP_STD_VER >= 23 + // [range.adaptor.object], range adaptor objects + using std::ranges::range_adaptor_closure; #endif // [range.all], all view diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt index e0d3a0dbc40031..fd57aa9fe8b375 100644 --- a/libcxx/test/CMakeLists.txt +++ b/libcxx/test/CMakeLists.txt @@ -1,5 +1,11 @@ include(HandleLitArguments) add_subdirectory(tools) +# When the tools add clang-tidy support, the dependencies need to be updated. +# This cannot be done in the tools CMakeLists.txt since that does not update +# the status in this (a parent) directory. +if(TARGET cxx-tidy) + list(APPEND LIBCXX_TEST_DEPS cxx-tidy) +endif() # By default, libcxx and libcxxabi share a library directory. if (NOT LIBCXX_CXX_ABI_LIBRARY_PATH) diff --git a/libcxx/test/libcxx/clang_tidy.gen.py b/libcxx/test/libcxx/clang_tidy.gen.py index f29447d006557f..76b9db2d5cb836 100644 --- a/libcxx/test/libcxx/clang_tidy.gen.py +++ b/libcxx/test/libcxx/clang_tidy.gen.py @@ -26,6 +26,9 @@ // The GCC compiler flags are not always compatible with clang-tidy. // UNSUPPORTED: gcc +// Clang 17 has false positives. +// UNSUPPORTED: clang-17 + {lit_header_restrictions.get(header, '')} // TODO: run clang-tidy with modules enabled once they are supported diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt index 28eed614458336..28c1dbf8aca3c1 100644 --- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt +++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt @@ -64,6 +64,28 @@ if(NOT HAS_CLANG_TIDY_HEADERS) "clang-tidy headers are not present.") return() endif() + +# The clangTidy plugin uses C++20, so ensure that we support C++20 when using libstdc++. +# This is required because some versions of libstdc++ used as a system library on build platforms +# we support do not support C++20 yet. +# Note it has not been tested whether version 11 works. +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test.cpp" " +#include +#if defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE < 12 + # error The libstdc++ version is too old. +#endif +int main(){} +") +try_compile(HAS_NEWER_STANDARD_LIBRARY + "${CMAKE_CURRENT_BINARY_DIR}" + "${CMAKE_CURRENT_BINARY_DIR}/test.cpp" + LINK_LIBRARIES clangTidy) + +if(NOT HAS_NEWER_STANDARD_LIBRARY) + message(STATUS "Clang-tidy tests are disabled due to using " + "stdlibc++ older than version 12") + return() +endif() message(STATUS "Clang-tidy tests are enabled.") set(SOURCES @@ -88,5 +110,3 @@ set_target_properties(cxx-tidy PROPERTIES set_target_properties(cxx-tidy PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set(CMAKE_SHARED_MODULE_SUFFIX_CXX .plugin) # Use a portable suffix to simplify how we can find it from Lit - -list(APPEND LIBCXX_TEST_DEPS cxx-tidy)