[PATCH] D66254: Correct include suggestion when search path includes symlink

Ben Jackson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 14 13:57:32 PDT 2019

puremourning created this revision.
puremourning added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, ilya-biryukov.
Herald added a project: clang.
puremourning added a comment.

FYI this fixes the issue in https://github.com/clangd/clangd/issues/124

I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem doesn't support symlinks and I didn't want to try and implement that for this patch. Let me know thoughts on alternative ways to regression test it.

When a header search path contained a symlink, the suggested include
spelling could be wrong, as the files to insert provided by clangd are
always canonical (i.e. with the symlinks removed), but the search paths
(which come from flags), might not be. As files from Sema also come from
the VFS, they are also canonical, I think.

When trying to suggest the spelling of the include to insert, we should
always compare canonical paths. This corrects the include suggestion in
this case.

  rG LLVM Github Monorepo



Index: clang/lib/Lex/HeaderSearch.cpp
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1723,7 +1723,13 @@
     if (!WorkingDir.empty() && !path::is_absolute(Dir))
       fs::make_absolute(WorkingDir, DirPath);
     path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-    Dir = DirPath;
+    // Find the canonical absolute path, as that's what we expect in File
+    auto& FS = FileMgr.getVirtualFileSystem();
+    SmallString<4096> CanonicalNameBuf;
+    if (!FS.getRealPath(DirPath, CanonicalNameBuf))
+      Dir = CanonicalNameBuf;
+    else
+      Dir = DirPath;
     for (auto NI = path::begin(File), NE = path::end(File),
               DI = path::begin(Dir), DE = path::end(Dir);
          /*termination condition in loop*/; ++NI, ++DI) {
Index: clang/include/clang/Lex/HeaderSearch.h
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -727,6 +727,7 @@
   /// MainFile location, if none of the include search directories were prefix
   /// of File.
+  /// \param File A canonical absolute path to the file to be included.
   /// \param WorkingDir If non-empty, this will be prepended to search directory
   /// paths that are relative.
   std::string suggestPathToFileForDiagnostics(llvm::StringRef File,

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66254.215235.patch
Type: text/x-patch
Size: 1419 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190814/7bd35852/attachment.bin>

More information about the cfe-commits mailing list