[clangd] Avoid duplicates in findDefinitions response
Summary:
When compile_commands.json contains some source files expressed as
relative paths, we can get duplicate responses to findDefinitions. The
responses only differ by the URI, which are different versions of the
same file:
"result": [
{
...
"uri": "file:///home/emaisin/src/ls-interact/cpp-test/build/../src/first.h"
},
{
...
"uri": "file:///home/emaisin/src/ls-interact/cpp-test/src/first.h"
}
]
In getAbsoluteFilePath, we try to obtain the realpath of the FileEntry
by calling tryGetRealPathName. However, this can fail and return an
empty string. It may be bug a bug in clang, but in any case we should
fall back to computing it ourselves if it happens.
I changed getAbsoluteFilePath so that if tryGetRealPathName succeeds, we
return right away (a real path is always absolute). Otherwise, we try
to build an absolute path, as we did before, but we also call
VFS->getRealPath to make sure to get the canonical path (e.g. without
any ".." in it).
Reviewers: malaperle
Subscribers: hokein, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48687
llvm-svn: 339483
diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp
index c1ee678..cc7f084 100644
--- a/clang-tools-extra/clangd/FindSymbols.cpp
+++ b/clang-tools-extra/clangd/FindSymbols.cpp
@@ -193,7 +193,7 @@
const FileEntry *F = SM.getFileEntryForID(SM.getMainFileID());
if (!F)
return;
- auto FilePath = getAbsoluteFilePath(F, SM);
+ auto FilePath = getRealPath(F, SM);
if (FilePath)
MainFileUri = URIForFile(*FilePath);
}
diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 88ec2c9..931ad3c 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -185,18 +185,34 @@
return Edits;
}
-llvm::Optional<std::string>
-getAbsoluteFilePath(const FileEntry *F, const SourceManager &SourceMgr) {
- SmallString<64> FilePath = F->tryGetRealPathName();
- if (FilePath.empty())
- FilePath = F->getName();
- if (!llvm::sys::path::is_absolute(FilePath)) {
- if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
- log("Could not turn relative path to absolute: {0}", FilePath);
+llvm::Optional<std::string> getRealPath(const FileEntry *F,
+ const SourceManager &SourceMgr) {
+ // Ideally, we get the real path from the FileEntry object.
+ SmallString<128> FilePath = F->tryGetRealPathName();
+ if (!FilePath.empty()) {
+ return FilePath.str().str();
+ }
+
+ // Otherwise, we try to compute ourselves.
+ vlog("FileEntry for {0} did not contain the real path.", F->getName());
+
+ llvm::SmallString<128> Path = F->getName();
+
+ if (!llvm::sys::path::is_absolute(Path)) {
+ if (!SourceMgr.getFileManager().makeAbsolutePath(Path)) {
+ log("Could not turn relative path to absolute: {0}", Path);
return llvm::None;
}
}
- return FilePath.str().str();
+
+ llvm::SmallString<128> RealPath;
+ if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath(
+ Path, RealPath)) {
+ log("Could not compute real path: {0}", Path);
+ return Path.str().str();
+ }
+
+ return RealPath.str().str();
}
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h
index cb0c672..e633a15 100644
--- a/clang-tools-extra/clangd/SourceCode.h
+++ b/clang-tools-extra/clangd/SourceCode.h
@@ -62,13 +62,20 @@
std::vector<TextEdit> replacementsToEdits(StringRef Code,
const tooling::Replacements &Repls);
-/// Get the absolute file path of a given file entry.
-llvm::Optional<std::string> getAbsoluteFilePath(const FileEntry *F,
- const SourceManager &SourceMgr);
-
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
const LangOptions &L);
+/// Get the real/canonical path of \p F. This means:
+///
+/// - Absolute path
+/// - Symlinks resolved
+/// - No "." or ".." component
+/// - No duplicate or trailing directory separator
+///
+/// This function should be used when sending paths to clients, so that paths
+/// are normalized as much as possible.
+llvm::Optional<std::string> getRealPath(const FileEntry *F,
+ const SourceManager &SourceMgr);
} // namespace clangd
} // namespace clang
#endif
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index e95730b..f5e2b1e 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -192,7 +192,7 @@
Range R = {Begin, End};
Location L;
- auto FilePath = getAbsoluteFilePath(F, SourceMgr);
+ auto FilePath = getRealPath(F, SourceMgr);
if (!FilePath) {
log("failed to get path!");
return llvm::None;
@@ -286,7 +286,7 @@
std::string HintPath;
const FileEntry *FE =
SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
- if (auto Path = getAbsoluteFilePath(FE, SourceMgr))
+ if (auto Path = getRealPath(FE, SourceMgr))
HintPath = *Path;
// Query the index and populate the empty slot.
Index->lookup(