Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Driver] Some improvements for path handling on NetBSD #66863

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

brad0
Copy link
Contributor

@brad0 brad0 commented Sep 20, 2023

  • Make use of concat macro with various paths
  • Replace usage of = with SysRoot

- Make use of concat macro with various paths
- Replace usage of = with SysRoot
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Sep 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Changes
  • Make use of concat macro with various paths
  • Replace usage of = with SysRoot

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

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/NetBSD.cpp (+13-13)
  • (added) clang/test/Driver/Inputs/basic_netbsd_tree/usr/include/g++/.keep ()
  • (modified) clang/test/Driver/netbsd.cpp (+11)
diff --git a/clang/lib/Driver/ToolChains/NetBSD.cpp b/clang/lib/Driver/ToolChains/NetBSD.cpp
index 1ec8f1b7da4c662..88be6ea0d5e7883 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -360,7 +360,7 @@ NetBSD::NetBSD(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
     // what all logic is needed to emulate the '=' prefix here.
     switch (Triple.getArch()) {
     case llvm::Triple::x86:
-      getFilePaths().push_back("=/usr/lib/i386");
+      getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/i386"));
       break;
     case llvm::Triple::arm:
     case llvm::Triple::armeb:
@@ -369,35 +369,35 @@ NetBSD::NetBSD(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
       switch (Triple.getEnvironment()) {
       case llvm::Triple::EABI:
       case llvm::Triple::GNUEABI:
-        getFilePaths().push_back("=/usr/lib/eabi");
+        getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/eabi"));
         break;
       case llvm::Triple::EABIHF:
       case llvm::Triple::GNUEABIHF:
-        getFilePaths().push_back("=/usr/lib/eabihf");
+        getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/eabihf"));
         break;
       default:
-        getFilePaths().push_back("=/usr/lib/oabi");
+        getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/oabi"));
         break;
       }
       break;
     case llvm::Triple::mips64:
     case llvm::Triple::mips64el:
       if (tools::mips::hasMipsAbiArg(Args, "o32"))
-        getFilePaths().push_back("=/usr/lib/o32");
+        getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/o32"));
       else if (tools::mips::hasMipsAbiArg(Args, "64"))
-        getFilePaths().push_back("=/usr/lib/64");
+        getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/64"));
       break;
     case llvm::Triple::ppc:
-      getFilePaths().push_back("=/usr/lib/powerpc");
+      getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/powerpc"));
       break;
     case llvm::Triple::sparc:
-      getFilePaths().push_back("=/usr/lib/sparc");
+      getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib/sparc"));
       break;
     default:
       break;
     }
 
-    getFilePaths().push_back("=/usr/lib");
+    getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib"));
   }
 }
 
@@ -467,11 +467,11 @@ void NetBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                    llvm::opt::ArgStringList &CC1Args) const {
   const std::string Candidates[] = {
     // directory relative to build tree
-    getDriver().Dir + "/../include/c++/v1",
+    concat(getDriver().Dir, "/../include/c++/v1"),
     // system install with full upstream path
-    getDriver().SysRoot + "/usr/include/c++/v1",
+    concat(getDriver().SysRoot, "/usr/include/c++/v1"),
     // system install from src
-    getDriver().SysRoot + "/usr/include/c++",
+    concat(getDriver().SysRoot, "/usr/include/c++"),
   };
 
   for (const auto &IncludePath : Candidates) {
@@ -486,7 +486,7 @@ void NetBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
 
 void NetBSD::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                       llvm::opt::ArgStringList &CC1Args) const {
-  addLibStdCXXIncludePaths(getDriver().SysRoot + "/usr/include/g++", "", "",
+  addLibStdCXXIncludePaths(concat(getDriver().SysRoot, "/usr/include/g++"), "", "",
                            DriverArgs, CC1Args);
 }
 
diff --git a/clang/test/Driver/Inputs/basic_netbsd_tree/usr/include/g++/.keep b/clang/test/Driver/Inputs/basic_netbsd_tree/usr/include/g++/.keep
new file mode 100644
index 000000000000000..e69de29bb2d1d64
diff --git a/clang/test/Driver/netbsd.cpp b/clang/test/Driver/netbsd.cpp
index c0445dd3c3ed9a0..ab24209b4577aa2 100644
--- a/clang/test/Driver/netbsd.cpp
+++ b/clang/test/Driver/netbsd.cpp
@@ -264,3 +264,14 @@
 // DRIVER-PASS-INCLUDES:      "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
 // DRIVER-PASS-INCLUDES:      "-internal-isystem" "[[RESOURCE]]{{/|\\\\}}include"
 // DRIVER-PASS-INCLUDES:      "-internal-externc-isystem" "{{.*}}/usr/include"
+
+// Test NetBSD with libstdc++ when the sysroot path ends with `/`.
+// RUN: %clangxx -### %s 2>&1 \
+// RUN:     --target=x86_64-unknown-netbsd \
+// RUN:     -stdlib=libstdc++ \
+// RUN:     --sysroot=%S/Inputs/basic_netbsd_tree/ \
+// RUN:     --gcc-toolchain="" \
+// RUN:   | FileCheck --check-prefix=CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH %s
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH: "-cc1"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-BASIC-LIBSTDCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/g++"

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

This is similar to a change that I did for the Linux toolchain a while ago (e917801), LGTM! 👍

@brad0
Copy link
Contributor Author

brad0 commented Sep 20, 2023

This is similar to a change that I did for the Linux toolchain a while ago (e917801), LGTM! 👍

Thanks. I have been doing the same with other Drivers.

@brad0 brad0 merged commit a009fa7 into llvm:main Sep 20, 2023
@brad0 brad0 deleted the clang_driver_netbsd_sysroot branch September 20, 2023 20:14
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants