Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Mar 26, 2025

Backport 51bceb4

Requested by: @mstorsjo

@llvmbot llvmbot requested a review from a team as a code owner March 26, 2025 20:19
@llvmbot llvmbot added this to the LLVM 20.X Release milestone Mar 26, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 26, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Mar 26, 2025

@mordante What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from mordante March 26, 2025 20:19
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 26, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Mar 26, 2025

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport 51bceb4

Requested by: @mstorsjo


Full diff: https://github.com/llvm/llvm-project/pull/133153.diff

1 Files Affected:

  • (modified) libcxx/test/tools/clang_tidy_checks/CMakeLists.txt (+4-3)
diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
index 0f8f0e8864d0f..da045fac92ce4 100644
--- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
+++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
@@ -8,6 +8,10 @@ set(Clang_DIR_SAVE ${Clang_DIR})
 # 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(LLVM_DIR "${LLVM_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for LLVM." FORCE)
+set(Clang_DIR "${Clang_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for Clang." FORCE)
+
 if(NOT Clang_FOUND)
   message(STATUS "Clang-tidy tests are disabled since the "
                  "Clang development package is unavailable.")
@@ -19,9 +23,6 @@ if(NOT TARGET clangTidy)
   return()
 endif()
 
-set(LLVM_DIR "${LLVM_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for LLVM." FORCE)
-set(Clang_DIR "${Clang_DIR_SAVE}" CACHE PATH "The directory containing a CMake configuration file for Clang." FORCE)
-
 message(STATUS "Found system-installed LLVM ${LLVM_PACKAGE_VERSION} with headers in ${LLVM_INCLUDE_DIRS}")
 
 set(CMAKE_CXX_STANDARD 20)

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Mar 27, 2025
In 664f345, a fix was introduced,
attempting to restore LLVM_DIR and Clang_DIR after doing
find_package(Clang).

However, 6775285 added a return if the
clangTidy target wasn't found. If this is hit, we don't restore LLVM_DIR
and Clang_DIR, which causes strange effects if CMake is rerun a second
time.

Move the code for restoring LLVM_DIR and Clang_DIR to directly after the
find_package calls, to make sure they are restored, regardless of the
find_package outcome.

(cherry picked from commit 51bceb4)
@tstellar tstellar merged commit 44a6f6a into llvm:release/20.x Mar 29, 2025
13 of 18 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Mar 29, 2025
Copy link

@mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants