Correct include suggestion when search path includes symlink

[puremourning]

Summary

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.

[puremourning]

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.

[lebedev.ri]

Tests?

[puremourning]

In D66254#1630277, @lebedev.ri wrote:

Tests?

Per my comment: 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.

[puremourning]

In D66254#1630283, @puremourning wrote:

In D66254#1630277, @lebedev.ri wrote:

Tests?

Per my comment: 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.

OK I found test/SemaCXX/missing-header

Might be able to use the _real_ filesystem and go via the diagnostic rather than clangd

clang/include/clang/Lex/HeaderSearch.h

public:
 
   /// Suggest a path by which the specified file could be found, for use in
   /// diagnostics to suggest a #include. Returned path will only contain forward
   /// slashes as separators. MainFile is the absolute path of the file that we
   /// are generating the diagnostics for. It will try to shorten the path using
   /// 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,
                                               llvm::StringRef WorkingDir,
                                               llvm::StringRef MainFile,
                                               bool *IsSystem = nullptr);
 
   void PrintStats();

clang/lib/Lex/HeaderSearch.cpp

std::string HeaderSearch::suggestPathToFileForDiagnostics(
   // Checks whether Dir and File shares a common prefix, if they do and that's
   // the longest prefix we've seen so for it returns true and updates the
   // BestPrefixLength accordingly.
   auto CheckDir = [&](llvm::StringRef Dir) -> bool {
     llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
     if (!WorkingDir.empty() && !path::is_absolute(Dir))
       fs::make_absolute(WorkingDir, DirPath);
     path::remove_dots(DirPath, /*remove_dot_dot=*/true);
+    // 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) {
       // '.' components in File are ignored.
       while (NI != NE && *NI == ".")
         ++NI;
       if (NI == NE)
         break;