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 != "") {
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 != "") {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits