[PATCH] D57345: Make clang/test/Index/pch-from-libclang.c pass in more places

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: arphaman.

- fixes the test on macOS with LLVM_ENABLE_PIC=OFF
- together with D57343 <https://reviews.llvm.org/D57343>, gets the test to pass on Windows
- makes it run everywhere (it seems to just pass on Linux)

The main change is in CIndexer.cpp, see comments there.


https://reviews.llvm.org/D57345

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Index/pch-from-libclang.c
  clang/tools/libclang/CIndexer.cpp


Index: clang/tools/libclang/CIndexer.cpp
===================================================================
--- clang/tools/libclang/CIndexer.cpp
+++ clang/tools/libclang/CIndexer.cpp
@@ -14,6 +14,7 @@
 #include "CXString.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/MD5.h"
@@ -73,10 +74,20 @@
   LibClangPath += llvm::sys::path::parent_path(info.dli_fname);
 #endif
 
-  llvm::sys::path::append(LibClangPath, "clang", CLANG_VERSION_STRING);
+  // On Windows, libclang.dll is in bin/.
+  // On non-Windows, libclang.so/.dylib is in lib/.
+  // With a static-library build of libclang, LibClangPath will contain the
+  // path of the embedding binary, which for LLVM binaries will be in bin/.
+  // ../lib gets us to lib/ in both cases.
+  // This should be similar to CompilerInvocation::GetResourcesPath() and
+  // Driver::Driver().
+  SmallString<128> P;
+  P = llvm::sys::path::parent_path(LibClangPath);
+  llvm::sys::path::append(P, Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
+                          CLANG_VERSION_STRING);
 
   // Cache our result.
-  ResourcesPath = LibClangPath.str();
+  ResourcesPath = P.str();
   return ResourcesPath;
 }
 
Index: clang/test/Index/pch-from-libclang.c
===================================================================
--- clang/test/Index/pch-from-libclang.c
+++ clang/test/Index/pch-from-libclang.c
@@ -1,7 +1,11 @@
 // Check that clang can use a PCH created from libclang.
 
-// FIXME: Non-darwin bots fail. Would need investigation using -module-file-info to see what is the difference in modules generated from libclang vs the compiler invocation, in those systems.
-// REQUIRES: system-darwin
+// This test doesn't use -fdisable-module-hash and hence requires that
+// CompilerInvocation::getModuleHash() computes exactly the same hash
+// for c-index-test and clang, which in turn requires that the both use
+// exactly the same resource-dir, even without calling realpath() on it:
+// - a/../b/ and b/ are not considered the same
+// - on Windows, c:\ and C:\ (only different in case) are not the same
 
 // RUN: %clang_cc1 -fsyntax-only %s -verify
 // RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1891,6 +1891,8 @@
 
 std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
                                                  void *MainAddr) {
+  // This should be similar to CIndexer::getClangResourcesPath() and
+  // Driver::Driver().
   std::string ClangExecutable =
       llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
   StringRef Dir = llvm::sys::path::parent_path(ClangExecutable);
@@ -1900,9 +1902,11 @@
   SmallString<128> P(Dir);
   if (ClangResourceDir != "")
     llvm::sys::path::append(P, ClangResourceDir);
-  else
-    llvm::sys::path::append(P, "..", Twine("lib") + CLANG_LIBDIR_SUFFIX,
+  else {
+    P = llvm::sys::path::parent_path(Dir);
+    llvm::sys::path::append(P, Twine("lib") + CLANG_LIBDIR_SUFFIX,
                             "clang", CLANG_VERSION_STRING);
+  }
 
   return P.str();
 }
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -119,6 +119,8 @@
 #endif
 
   // Compute the path to the resource directory.
+  // This should be similar to CompilerInvocation::GetResourcesPath() and
+  // CIndexer::getClangResourcesPath().
   StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
   SmallString<128> P(Dir);
   if (ClangResourceDir != "") {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57345.183915.patch
Type: text/x-patch
Size: 3907 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190128/98854bd5/attachment-0001.bin>