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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66254
Files:
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
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,
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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
