[Lldb-commits] [lldb] 924a7d8 - Use CLANG_RESOURCE_DIR more consistently (#103388)

[via lldb-commits]

Author: Kim Gräsman
Date: 2024-08-26T14:49:56-03:00
New Revision: 924a7d83b4287b3b85dd1ca29d2d3e1f0a10ea68

URL: https://github.com/llvm/llvm-project/commit/924a7d83b4287b3b85dd1ca29d2d3e1f0a10ea68
DIFF: https://github.com/llvm/llvm-project/commit/924a7d83b4287b3b85dd1ca29d2d3e1f0a10ea68.diff

LOG: Use CLANG_RESOURCE_DIR more consistently (#103388)

When Clang is consumed as a library, the CLANG_RESOURCE_DIR definition
is not exported from the CMake system, so external clients will be
unable to compute the same resource dir as Clang itself would, because
they don't know what to pass for the optional CustomResourceDir
argument.

All call sites except one would pass CLANG_RESOURCE_DIR to
Driver::GetResourcesPath. It seems the one exception in libclang
CIndexer was an oversight.

Move the use of CLANG_RESOURCE_DIR into GetResourcesPath and remove the
optional argument to avoid this inconsistency between internal and
external clients.

Added: 
    

Modified: 
    clang/include/clang/Driver/Driver.h
    clang/lib/Driver/Driver.cpp
    clang/lib/Frontend/CompilerInvocation.cpp
    lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
    lldb/unittests/Expression/ClangParserTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 84eadd42880a50..9177d56718ee77 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -379,8 +379,7 @@ class Driver {
 
   /// Takes the path to a binary that's either in bin/ or lib/ and returns
   /// the path to clang's resource directory.
-  static std::string GetResourcesPath(StringRef BinaryPath,
-                                      StringRef CustomResourceDir = "");
+  static std::string GetResourcesPath(StringRef BinaryPath);
 
   Driver(StringRef ClangExecutable, StringRef TargetTriple,
          DiagnosticsEngine &Diags, std::string Title = "clang LLVM compiler",

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index e12416e51f8d24..43002add33774b 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -171,18 +171,18 @@ getHIPOffloadTargetTriple(const Driver &D, const ArgList &Args) {
 }
 
 // static
-std::string Driver::GetResourcesPath(StringRef BinaryPath,
-                                     StringRef CustomResourceDir) {
+std::string Driver::GetResourcesPath(StringRef BinaryPath) {
   // Since the resource directory is embedded in the module hash, it's important
   // that all places that need it call this function, so that they get the
   // exact same string ("a/../b/" and "b/" get 
diff erent hashes, for example).
 
   // Dir is bin/ or lib/, depending on where BinaryPath is.
-  std::string Dir = std::string(llvm::sys::path::parent_path(BinaryPath));
-
+  StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
   SmallString<128> P(Dir);
-  if (CustomResourceDir != "") {
-    llvm::sys::path::append(P, CustomResourceDir);
+
+  StringRef ConfiguredResourceDir(CLANG_RESOURCE_DIR);
+  if (!ConfiguredResourceDir.empty()) {
+    llvm::sys::path::append(P, ConfiguredResourceDir);
   } else {
     // On Windows, libclang.dll is in bin/.
     // On non-Windows, libclang.so/.dylib is in lib/.
@@ -239,7 +239,7 @@ Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
 #endif
 
   // Compute the path to the resource directory.
-  ResourceDir = GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+  ResourceDir = GetResourcesPath(ClangExecutable);
 }
 
 void Driver::setDriverMode(StringRef Value) {

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 0bb4175dd021ee..32628c5e84332d 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3130,7 +3130,7 @@ std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
                                                  void *MainAddr) {
   std::string ClangExecutable =
       llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
-  return Driver::GetResourcesPath(ClangExecutable, CLANG_RESOURCE_DIR);
+  return Driver::GetResourcesPath(ClangExecutable);
 }
 
 static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
index 6064c02c7fd67d..6de851081598fd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -53,7 +53,7 @@ static bool DefaultComputeClangResourceDirectory(FileSpec &lldb_shlib_spec,
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
   static const std::string clang_resource_path =
-      clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
+      clang::driver::Driver::GetResourcesPath("bin/lldb");
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
       // LLVM.org's build of LLDB uses the clang resource directory placed

diff  --git a/lldb/unittests/Expression/ClangParserTest.cpp b/lldb/unittests/Expression/ClangParserTest.cpp
index 6f682f6c97fdb5..fab4487c737195 100644
--- a/lldb/unittests/Expression/ClangParserTest.cpp
+++ b/lldb/unittests/Expression/ClangParserTest.cpp
@@ -42,8 +42,8 @@ TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib\\";
 #endif
-  std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
-      path_to_liblldb + "liblldb", CLANG_RESOURCE_DIR);
+  std::string path_to_clang_dir =
+      clang::driver::Driver::GetResourcesPath(path_to_liblldb + "liblldb");
   llvm::SmallString<256> path_to_clang_lib_dir_real;
   llvm::sys::fs::real_path(path_to_clang_dir, path_to_clang_lib_dir_real);