Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jan 14, 2025

Backport acbd822

Requested by: @ChuanqiXu9

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Jan 14, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jan 14, 2025

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

@llvmbot llvmbot requested a review from ChuanqiXu9 January 14, 2025 01:57
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jan 14, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jan 14, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: None (llvmbot)

Changes

Backport acbd822

Requested by: @ChuanqiXu9


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

2 Files Affected:

  • (modified) clang/lib/Driver/Driver.cpp (+5)
  • (modified) clang/test/Driver/modules-print-library-module-manifest-path.cpp (+26)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ecae475f75da00..f9dc8ab24fa9d7 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6189,6 +6189,11 @@ std::string Driver::GetFilePath(StringRef Name, const ToolChain &TC) const {
   if (auto P = SearchPaths(TC.getFilePaths()))
     return *P;
 
+  SmallString<128> R2(ResourceDir);
+  llvm::sys::path::append(R2, "..", "..", Name);
+  if (llvm::sys::fs::exists(Twine(R2)))
+    return std::string(R2);
+
   return std::string(Name);
 }
 
diff --git a/clang/test/Driver/modules-print-library-module-manifest-path.cpp b/clang/test/Driver/modules-print-library-module-manifest-path.cpp
index 3ba2709ad95cc8..8d17fe1549e34b 100644
--- a/clang/test/Driver/modules-print-library-module-manifest-path.cpp
+++ b/clang/test/Driver/modules-print-library-module-manifest-path.cpp
@@ -18,6 +18,28 @@
 // RUN:     --target=x86_64-linux-gnu 2>&1 \
 // RUN:   | FileCheck libcxx.cpp
 
+// for macos there is a different directory structure
+// where the library and libc++.modules.json file are in lib
+// directly but headers are in clang/ver directory which
+// is the resource directory
+// RUN: mkdir -p %t/Inputs/usr/lib/clang/20
+// RUN: touch %t/Inputs/usr/lib/libc++.so
+// RUN: touch %t/Inputs/usr/lib/libc++.modules.json
+// RUN: %clang -print-library-module-manifest-path \
+// RUN:     -stdlib=libc++ \
+// RUN:     -resource-dir=%t/Inputs/usr/lib/clang/20 \
+// RUN:     --target=arm64-apple-darwin24.1.0 2>&1 \
+// RUN:   | FileCheck libcxx.cpp.macos
+
+// RUN: rm %t/Inputs/usr/lib/libc++.so
+// RUN: touch %t/Inputs/usr/lib/libc++.a
+// RUN: touch %t/Inputs/usr/lib/libc++.modules.json
+// RUN: %clang -print-library-module-manifest-path \
+// RUN:     -stdlib=libc++ \
+// RUN:     -resource-dir=%t/Inputs/usr/lib/clang/20 \
+// RUN:     --target=arm64-apple-darwin24.1.0 2>&1 \
+// RUN:   | FileCheck libcxx.cpp.macos
+
 // RUN: rm %t/Inputs/usr/lib/x86_64-linux-gnu/libc++.so
 // RUN: touch %t/Inputs/usr/lib/x86_64-linux-gnu/libc++.a
 // RUN: %clang -print-library-module-manifest-path \
@@ -40,6 +62,10 @@
 
 // CHECK: {{.*}}/Inputs/usr/lib/x86_64-linux-gnu{{/|\\}}libc++.modules.json
 
+//--- libcxx.cpp.macos
+
+// CHECK: {{.*}}libc++.modules.json
+
 //--- libcxx-no-shared-lib.cpp
 
 // Note this might find a different path depending whether search path

@ChuanqiXu9
Copy link
Member

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

It is necessary for using std module on MacOS with CMake. I think it is important.

@tru tru merged commit cd70802 into llvm:release/19.x Jan 14, 2025
1 check was pending
This commit fixes -print-library-module-manifest-path on macos.
Currently, this only works on linux systems. This is because on macos
systems the library and header files are installed in a different
location. The module manifest is next to the libraries and the search
function was not looking in both places. There is also a test included.

(cherry picked from commit acbd822)
Copy link

@ChuanqiXu9 (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
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants