Skip to content

Commit

Permalink
[libc++][CI] Reenables clang-tidy. (llvm#90077)
Browse files Browse the repository at this point in the history
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
llvm#89898
  • Loading branch information
mordante authored May 8, 2024
1 parent 37b6ba9 commit 79921fb
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 11 deletions.
18 changes: 12 additions & 6 deletions libcxx/modules/std/chrono.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions libcxx/modules/std/ranges.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions libcxx/test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
3 changes: 3 additions & 0 deletions libcxx/test/libcxx/clang_tidy.gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 22 additions & 2 deletions libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 <version>
#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
Expand All @@ -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)

0 comments on commit 79921fb

Please sign in to comment.