[PATCH] D156781: Use the canonical path for the include header search.

[Jonas via Phabricator via cfe-commits]

felberj created this revision.
Herald added subscribers: kadircet, arphaman, hiraditya.
Herald added a project: All.
felberj requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, ilya-biryukov.
Herald added projects: clang, LLVM, clang-tools-extra.

This is done so that clangd can propose the correct include path when
a include directory contains a symlink.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156781

Files:
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang/lib/Lex/InitHeaderSearch.cpp
  llvm/lib/Support/VirtualFileSystem.cpp


Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1160,6 +1160,11 @@
   if (auto EC = makeAbsolute(Output))
     return EC;
   llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+  if (auto Node = lookupNode(Output, true)) {
+    auto Name = Node.getName();
+    Output.clear();
+    Output.append(Name.begin(), Name.end());
+  }
   return {};
 }
 
Index: clang/lib/Lex/InitHeaderSearch.cpp
===================================================================
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -165,9 +165,23 @@
 
   // If the directory exists, add it.
   if (auto DE = FM.getOptionalDirectoryRef(MappedPathStr)) {
-    IncludePath.emplace_back(Group, DirectoryLookup(*DE, Type, isFramework),
-                             UserEntryIdx);
-    return true;
+    StringRef canonical = FM.getCanonicalName(*DE);
+    if (canonical == MappedPathStr) {
+      // It is a normal directory
+      IncludePath.emplace_back(Group, DirectoryLookup(*DE, Type, isFramework),
+                               UserEntryIdx);
+      return true;
+    }
+    if (Verbose) {
+      llvm::errs() << "rewrite the include " << MappedPathStr
+                   << " to its canonical path " << canonical << "\n";
+    }
+    // If it is a symlink, we add the canonical path.
+    if (auto cDE = FM.getOptionalDirectoryRef(canonical)) {
+      IncludePath.emplace_back(Group, DirectoryLookup(*cDE, Type, isFramework),
+                               UserEntryIdx);
+      return true;
+    }
   }
 
   // Check to see if this is an apple-style headermap (which are not allowed to
Index: clang-tools-extra/clangd/unittests/TestFS.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -26,13 +26,14 @@
 // directories.
 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 buildTestFS(llvm::StringMap<std::string> const &Files,
+            llvm::StringMap<std::string> const &Symlinks = {},
             llvm::StringMap<time_t> const &Timestamps = {});
 
 // A VFS provider that returns TestFSes containing a provided set of files.
 class MockFS : public ThreadsafeFS {
 public:
   IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
-    auto MemFS = buildTestFS(Files, Timestamps);
+    auto MemFS = buildTestFS(Files, Symlinks, Timestamps);
     if (!OverlayRealFileSystemForModules)
       return MemFS;
     llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem =
@@ -43,6 +44,7 @@
 
   // If relative paths are used, they are resolved with testPath().
   llvm::StringMap<std::string> Files;
+  llvm::StringMap<std::string> Symlinks;
   llvm::StringMap<time_t> Timestamps;
   // If true, real file system will be used as fallback for the in-memory one.
   // This is useful for testing module support.
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -31,6 +31,7 @@
 
 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
 buildTestFS(llvm::StringMap<std::string> const &Files,
+            llvm::StringMap<std::string> const &Symlinks,
             llvm::StringMap<time_t> const &Timestamps) {
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> MemFS(
       new llvm::vfs::InMemoryFileSystem);
@@ -41,6 +42,11 @@
         File, Timestamps.lookup(File),
         llvm::MemoryBuffer::getMemBufferCopy(FileAndContents.second, File));
   }
+  for (auto &SymlinkAndContents : Symlinks) {
+    llvm::StringRef Src = SymlinkAndContents.first();
+    llvm::StringRef Dest = SymlinkAndContents.second;
+    MemFS->addSymbolicLink(Src, Dest, Timestamps.lookup(Src));
+  }
   return MemFS;
 }
 
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -301,6 +301,20 @@
               ElementsAre(Subdir, testPath("foo/bar"), testPath("foo")));
 }
 
+TEST_F(HeadersTest, SymlinkedSearchPath) {
+  MainFile = testPath("sub/main.cpp");
+  std::string Path = testPath("outer/foo/bar.h");
+  FS.Files[Path] = "";
+
+  std::string Symlink = testPath("sub/symlink");
+  FS.Symlinks[Symlink] = "../outer";
+
+  SearchDirArg = (llvm::Twine("-I") + Symlink).str();
+  CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+  std::string test = testPath("outer/foo/bar.h");
+  EXPECT_EQ(calculate(test), "\"foo/bar.h\"");
+}
+
 TEST_F(HeadersTest, InsertInclude) {
   std::string Path = testPath("sub/bar.h");
   FS.Files[Path] = "";

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits