[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added a comment. Gentle ping. Are we waiting on anything to ship this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 527591.
fahadnayyar added a comment.
Edits in comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148266/new/
https://reviews.llvm.org/D148266
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-header-search-libcxx.cpp
clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to
prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the
toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and
without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "/usr/bin/ld"
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
Index: clang/test/Driver/darwin-header-search-libcxx.cpp
===
--- clang/test/Driver/darwin-header-search-libcxx.cpp
+++ clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -92,7 +92,7 @@
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem"
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem"
"[[SYSROOT]]/usr/include/c++/v1"
-// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the
toolchain
+// Make sure that using -nostdinc, -nostdinc++ or -nostdlibinc will drop both
the toolchain
// C++ include path and the sysroot one.
//
// RUN: %clang -### %s -fsyntax-only 2>&1 \
@@ -116,7 +116,7 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -stdlib=platform \
// RUN: -nostdinc++ \
-// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
// CHECK-LIBCXX-NOSTDINCXX: "-cc1"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -417,6 +417,17 @@
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Pass -L /bin/../lib/ to linker to prioritize
toolchain's
+ // libc++.dylib over the sysroot-provided one. This matches what we do for
+ // determining which libc++ headers to use.
+ llvm::SmallString<128> InstallBinPath =
+ llvm::StringRef(D.getInstalledDir()); // /bin
+ llvm::SmallString<128> LibCxxDylibDirPath = InstallBinPath;
+ llvm::sys::path::append(LibCxxDylibDirPath, "..",
+ "lib"); // /bin/../lib
+ CmdArgs.push_back("-L");
+ CmdArgs.push_back(C.getArgs().MakeArgString(LibCxxDylibDirPath));
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
StringRef sysroot = C.getSysRoot();
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: |
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added inline comments. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:105 // RUN: -nostdinc \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ ldionne wrote: > Nevermind. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:133 // RUN: -nostdlibinc \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ ldionne wrote: > Nevermind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. This LGTM w/ comments applied. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added inline comments. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:95 // Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the toolchain // C++ include path and the sysroot one. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:105 // RUN: -nostdinc \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:133 // RUN: -nostdlibinc \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:119 // RUN: -nostdinc++ \ -// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ +// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ fahadnayyar wrote: > ldionne wrote: > > Is this change really needed anymore? Why? > This is unrelated small bug in darwin-header-search-libcxx.cpp. I thought > maybe we fix that small issue also in this patch. What you think? Ah, I see. I think I understand the bug. Yeah I think it makes sense to fix it here but I added a few more fixes in comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar marked 2 inline comments as done. fahadnayyar added inline comments. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:119 // RUN: -nostdinc++ \ -// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ +// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ ldionne wrote: > Is this change really needed anymore? Why? This is unrelated small bug in darwin-header-search-libcxx.cpp. I thought maybe we fix that small issue also in this patch. What you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 527512.
fahadnayyar added a comment.
Simplifying the test case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148266/new/
https://reviews.llvm.org/D148266
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-header-search-libcxx.cpp
clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to
prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the
toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and
without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "/usr/bin/ld"
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
Index: clang/test/Driver/darwin-header-search-libcxx.cpp
===
--- clang/test/Driver/darwin-header-search-libcxx.cpp
+++ clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -116,7 +116,7 @@
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -stdlib=platform \
// RUN: -nostdinc++ \
-// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
// RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
// RUN: --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
// CHECK-LIBCXX-NOSTDINCXX: "-cc1"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -417,6 +417,17 @@
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Pass -L /bin/../lib/ to linker to prioritize
toolchain's
+ // libc++.dylib over the sysroot-provided one. This matches what we do for
+ // determining which libc++ headers to use.
+ llvm::SmallString<128> InstallBinPath =
+ llvm::StringRef(D.getInstalledDir()); // /bin
+ llvm::SmallString<128> LibCxxDylibDirPath = InstallBinPath;
+ llvm::sys::path::append(LibCxxDylibDirPath, "..",
+ "lib"); // /bin/../lib
+ CmdArgs.push_back("-L");
+ CmdArgs.push_back(C.getArgs().MakeArgString(LibCxxDylibDirPath));
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
StringRef sysroot = C.getSysRoot();
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN: | FileCheck
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne requested changes to this revision. ldionne added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:119 // RUN: -nostdinc++ \ -// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \ +// RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ Is this change really needed anymore? Why? Comment at: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp:11 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s I don't see `CHECK-TOOLCHAIN-LIBCXX-LINKING-2` defined anywhere? Comment at: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp:13 // Check with only headers alongside the installation (those should be used). // Some of these test cases and comments don't make sense for this test -- you copy-pasted the test for header search paths and that's fine, but please go through it to make sure you make the necessary edits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 527407. fahadnayyar added a comment. Rebasing to latest main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 Files: clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp === --- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp +++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp @@ -1,63 +1,48 @@ // UNSUPPORTED: system-windows -// General tests that the header search paths for libc++ detected by the driver -// and passed to CC1 are correct on Darwin platforms. +// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases. // Check without a sysroot and without headers alongside the installation -// (no include path should be added, and no warning or error). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with only headers alongside the installation (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: --sysroot="" \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s -// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ // RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s -// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with only headers in the sysroot (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s -// CHECK-LIBCXX-SYSROOT-1: "-cc1" -// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" -// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with both headers in the sysroot and headers alongside the installation -// (the headers in the toolchain should be preferred over the headers). -// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence -// over --sysroot. // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ @@ -65,9 +50,9 @@ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN:
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 527241. fahadnayyar added a comment. Passing -L /bin/../lib/ unconditionally to the linker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 Files: clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp === --- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp +++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp @@ -1,63 +1,48 @@ // UNSUPPORTED: system-windows -// General tests that the header search paths for libc++ detected by the driver -// and passed to CC1 are correct on Darwin platforms. +// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases. // Check without a sysroot and without headers alongside the installation -// (no include path should be added, and no warning or error). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with only headers alongside the installation (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: --sysroot="" \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s -// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ // RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s -// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with only headers in the sysroot (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s -// CHECK-LIBCXX-SYSROOT-1: "-cc1" -// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" -// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with both headers in the sysroot and headers alongside the installation -// (the headers in the toolchain should be preferred over the headers). -// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence -// over --sysroot. // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ @@ -65,9 +50,9 @@ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN:
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431 + // libc++.dylib in the toolchain. + if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc, +options::OPT_nostdincxx)) && arphaman wrote: > ldionne wrote: > > The more I look at this, the more I think it seems kind of weird to me that > > a linker argument would be impacted by `-nostdinc` & friends, which control > > header search paths. IMO we should use the simplest behavior and only check > > for `libc++.dylib` here, and not check for `libc++.dylib` when we determine > > the header search paths. > > > > @arphaman Do you have an opinion? > That makes sense to me. I'm not sure why specifically do we need to check if > the dylib exists when determining the header search path. Is there a specific > reason we couldn't mix the local headers and the system's dylib in that case? > We definitely don't want to mix the local headers and the system dylib. They can be configured differently (e.g. different ABI flags amongst other things). However it shouldn't really matter since when we build libc++, we build both the dylib and the headers so either both should be picked up or neither should be. I wouldn't bother checking. If we really wanted to check, we could actually error-out if one (but not both) are there, since that would point out to a mismatch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
arphaman added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431 + // libc++.dylib in the toolchain. + if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc, +options::OPT_nostdincxx)) && ldionne wrote: > The more I look at this, the more I think it seems kind of weird to me that a > linker argument would be impacted by `-nostdinc` & friends, which control > header search paths. IMO we should use the simplest behavior and only check > for `libc++.dylib` here, and not check for `libc++.dylib` when we determine > the header search paths. > > @arphaman Do you have an opinion? That makes sense to me. I'm not sure why specifically do we need to check if the dylib exists when determining the header search path. Is there a specific reason we couldn't mix the local headers and the system's dylib in that case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 514699. fahadnayyar added a comment. Fixing some more regression test errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 Files: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/mock-libcxx/lib/libc++.dylib clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp clang/test/Driver/stdlibxx-isystem.cpp clang/test/Tooling/Inputs/mock-libcxx/lib/libc++.dylib Index: clang/test/Driver/stdlibxx-isystem.cpp === --- clang/test/Driver/stdlibxx-isystem.cpp +++ clang/test/Driver/stdlibxx-isystem.cpp @@ -5,6 +5,8 @@ // By default, we should search for libc++ next to the driver. // RUN: mkdir -p %t/bin +// RUN: mkdir -p %t/lib +// RUN: touch %t/lib/libc++.dylib // RUN: mkdir -p %t/include/c++/v1 // RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \ // RUN: -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \ Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp === --- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp +++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp @@ -1,63 +1,49 @@ // UNSUPPORTED: system-windows -// General tests that the header search paths for libc++ detected by the driver -// and passed to CC1 are correct on Darwin platforms. +// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib +// whenever we are including the toolchain's libc++ headers // Check without a sysroot and without headers alongside the installation -// (no include path should be added, and no warning or error). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with only headers alongside the installation (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: --sysroot="" \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s -// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ // RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s -// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with only headers in the sysroot (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s -// CHECK-LIBCXX-SYSROOT-1: "-cc1" -// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" -// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with both headers in the sysroot and headers alongside the installation -// (the headers in the toolchain should be preferred over the headers). -// Ensure that both
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 514397. fahadnayyar added a comment. Herald added a subscriber: carlosgalvezp. Herald added a project: clang-tools-extra. Fixed regression error in clang-tidy test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 Files: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/mock-libcxx/lib/libc++.dylib clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp === --- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp +++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp @@ -1,63 +1,49 @@ // UNSUPPORTED: system-windows -// General tests that the header search paths for libc++ detected by the driver -// and passed to CC1 are correct on Darwin platforms. +// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib +// whenever we are including the toolchain's libc++ headers // Check without a sysroot and without headers alongside the installation -// (no include path should be added, and no warning or error). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with only headers alongside the installation (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: --sysroot="" \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s -// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ // RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s -// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with only headers in the sysroot (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s -// CHECK-LIBCXX-SYSROOT-1: "-cc1" -// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" -// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with both headers in the sysroot and headers alongside the installation -// (the headers in the toolchain should be preferred over the headers). -// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence -// over --sysroot. // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ @@ -65,9 +51,9 @@ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN:
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar added inline comments.
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
if (sysroot != "") {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
ldionne wrote:
> ldionne wrote:
> > Where do we set the usual `/usr/lib` search path? I think it might
> > make sense to move the code you added to that place.
> What I mean: there's a place where we must be adding `-L /usr/lib`
> in the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it?
> Would it make sense to move the code you added to that place instead?
/usr/local/include is added here in
```DarwinClang::AddClangSystemIncludeArgs```:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2346
and /usr/include is added here:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2374
.. But since we want to add a new argument to the linker invocation, I feel
```darwin::Linker::AddLinkArgs``` is the best place. But let me know if you
think otherwise.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148266/new/
https://reviews.llvm.org/D148266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added a subscriber: arphaman.
ldionne added inline comments.
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431
+ // libc++.dylib in the toolchain.
+ if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
+options::OPT_nostdincxx)) &&
The more I look at this, the more I think it seems kind of weird to me that a
linker argument would be impacted by `-nostdinc` & friends, which control
header search paths. IMO we should use the simplest behavior and only check for
`libc++.dylib` here, and not check for `libc++.dylib` when we determine the
header search paths.
@arphaman Do you have an opinion?
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
if (sysroot != "") {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
ldionne wrote:
> Where do we set the usual `/usr/lib` search path? I think it might
> make sense to move the code you added to that place.
What I mean: there's a place where we must be adding `-L /usr/lib` in
the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it? Would it
make sense to move the code you added to that place instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148266/new/
https://reviews.llvm.org/D148266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar updated this revision to Diff 514037. fahadnayyar added a comment. Herald added a subscriber: ormris. Added test case. Now checking the existence of libc++ headers in the toolchain when including the libc++.dylib form toolchain and vice versa. Also checking the absence of -nostdinc, -nostdinc++ or -nostdlib arguments before linking with libc++.dylib from the toolchain. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 Files: clang/lib/Driver/ToolChains/Darwin.cpp clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp === --- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp +++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp @@ -1,63 +1,49 @@ // UNSUPPORTED: system-windows -// General tests that the header search paths for libc++ detected by the driver -// and passed to CC1 are correct on Darwin platforms. +// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib +// whenever we are including the toolchain's libc++ headers // Check without a sysroot and without headers alongside the installation -// (no include path should be added, and no warning or error). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ -// RUN: | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s -// CHECK-LIBCXX-NONE: "-cc1" +// RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with only headers alongside the installation (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: --sysroot="" \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s -// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \ // RUN: | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \ // RUN: -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s -// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1" -// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" -// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s // Check with only headers in the sysroot (those should be used). // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \ -// RUN: --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s -// CHECK-LIBCXX-SYSROOT-1: "-cc1" -// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1" -// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1" +// RUN: --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s // Check with both headers in the sysroot and headers alongside the installation -// (the headers in the toolchain should be preferred over the headers). -// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence -// over --sysroot. // -// RUN: %clang -### %s -fsyntax-only 2>&1 \ +// RUN: %clang -### %s 2>&1 \ // RUN: --target=x86_64-apple-darwin \ // RUN: -stdlib=libc++ \ // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \ @@ -65,9 +51,9 @@ // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \ // RUN: | FileCheck
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne added inline comments.
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:413-414
+ // Including the path to the just-built libc++.dylib if libc++ is
bootstrapped
+ // and /bin/../lib/libc++.dylib is a valid path
+
I would reword this to:
```
// If the toolchain contains libc++.dylib (in
/bin/../lib/libc++.dylib), use that instead of the sysroot-provided
one. This matches what we do for determining which libc++ headers to use.
```
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
if (sysroot != "") {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
Where do we set the usual `/usr/lib` search path? I think it might
make sense to move the code you added to that place.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148266/new/
https://reviews.llvm.org/D148266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Thanks for working on this! This generally LGTM, with a few comments. You should be able to add tests in a way similar to `clang/test/Driver/darwin-header-search-libcxx.cpp`. The idea is basically to create a dummy sysroot that contains a `libc++.dylib` file in the right location and trick the compiler into adding `-L` as you want. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:416 + + llvm::SmallString<128> LibCXXDylibDirPath = + llvm::StringRef(D.getInstalledDir()); // /bin Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:421 + llvm::SmallString<128> LibCXXDylibPath = LibCXXDylibDirPath; + llvm::sys::path::append(LibCXXDylibPath, "..", "lib", "libc++.dylib"); + Otherwise you are forming the path `/bin/../lib/../lib/libc++.dylib`, which is correct but weird. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fdeazeve added a comment. Thanks for working on this! The idea looks good to me, but I am not familiar with this part of the codebase, so I'll defer the review to others Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148266/new/ https://reviews.llvm.org/D148266 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang
fahadnayyar created this revision.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
When libc++ is bootstrapped with clang using the cmake options
-DLLVM_ENABLE_PROJECTS="clang;llvm;lldb" and
-DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" then the resulting clang uses
the just-built libcxx headers from /bin/../include/c++/v1
. So the clang in this case should also use the just-built libc++.dylib
from /bin/../lib instead of using the system's libc++
libraries.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148266
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -410,6 +410,21 @@
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Including the path to the just-built libc++.dylib if libc++ is
bootstrapped
+ // and /bin/../lib/libc++.dylib is a valid path
+
+ llvm::SmallString<128> LibCXXDylibDirPath =
+ llvm::StringRef(D.getInstalledDir()); // /bin
+ llvm::sys::path::append(LibCXXDylibDirPath, "..", "lib");
+
+ llvm::SmallString<128> LibCXXDylibPath = LibCXXDylibDirPath;
+ llvm::sys::path::append(LibCXXDylibPath, "..", "lib", "libc++.dylib");
+
+ if (D.getVFS().exists(LibCXXDylibPath)) {
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibCXXDylibDirPath));
+ }
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
StringRef sysroot = C.getSysRoot();
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -410,6 +410,21 @@
Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
+ // Including the path to the just-built libc++.dylib if libc++ is bootstrapped
+ // and /bin/../lib/libc++.dylib is a valid path
+
+ llvm::SmallString<128> LibCXXDylibDirPath =
+ llvm::StringRef(D.getInstalledDir()); // /bin
+ llvm::sys::path::append(LibCXXDylibDirPath, "..", "lib");
+
+ llvm::SmallString<128> LibCXXDylibPath = LibCXXDylibDirPath;
+ llvm::sys::path::append(LibCXXDylibPath, "..", "lib", "libc++.dylib");
+
+ if (D.getVFS().exists(LibCXXDylibPath)) {
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibCXXDylibDirPath));
+ }
+
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
StringRef sysroot = C.getSysRoot();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
