simark updated this revision to Diff 159764.
simark added a comment.
Herald added a subscriber: arphaman.
New version. I tried to share the code between this and SymbolCollector, but
didn't find a good clean way. Do you have concrete suggestions on how to do
this? Otherwise, would it be acceptable to merge it as-is?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48687
Files:
clangd/FindSymbols.cpp
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/XRefs.cpp
unittests/clangd/TestFS.cpp
unittests/clangd/TestFS.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -312,27 +312,65 @@
}
TEST(GoToDefinition, RelPathsInCompileCommand) {
+ // The source is in "/clangd-test/src".
+ // We build in "/clangd-test/build".
+
Annotations SourceAnnotations(R"cpp(
+#include "header_in_preamble.h"
int [[foo]];
-int baz = f^oo;
+#include "header_not_in_preamble.h"
+int baz = f$p1^oo + bar_pre$p2^amble + bar_not_pre$p3^amble;
+)cpp");
+
+ Annotations HeaderInPreambleAnnotations(R"cpp(
+int [[bar_preamble]];
+)cpp");
+
+ Annotations HeaderNotInPreambleAnnotations(R"cpp(
+int [[bar_not_preamble]];
)cpp");
+ // Make the compilation paths appear as ../src/foo.cpp in the compile
+ // commands.
+ SmallString<32> RelPathPrefix("..");
+ llvm::sys::path::append(RelPathPrefix, "src");
+ std::string BuildDir = testPath("build");
+ MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
+
IgnoreDiagnostics DiagConsumer;
- MockCompilationDatabase CDB(/*UseRelPaths=*/true);
MockFSProvider FS;
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
- auto FooCpp = testPath("foo.cpp");
+ // Fill the filesystem.
+ auto FooCpp = testPath("src/foo.cpp");
FS.Files[FooCpp] = "";
+ auto HeaderInPreambleH = testPath("src/header_in_preamble.h");
+ FS.Files[HeaderInPreambleH] = HeaderInPreambleAnnotations.code();
+ auto HeaderNotInPreambleH = testPath("src/header_not_in_preamble.h");
+ FS.Files[HeaderNotInPreambleH] = HeaderNotInPreambleAnnotations.code();
- Server.addDocument(FooCpp, SourceAnnotations.code());
runAddDocument(Server, FooCpp, SourceAnnotations.code());
+
+ // Go to a definition in main source file.
auto Locations =
- runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
+ runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p1"));
EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-
EXPECT_THAT(*Locations, ElementsAre(Location{URIForFile{FooCpp},
SourceAnnotations.range()}));
+
+ // Go to a definition in header_in_preamble.h.
+ Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p2"));
+ EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ EXPECT_THAT(*Locations,
+ ElementsAre(Location{URIForFile{HeaderInPreambleH},
+ HeaderInPreambleAnnotations.range()}));
+
+ // Go to a definition in header_not_in_preamble.h.
+ Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p3"));
+ EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ EXPECT_THAT(*Locations,
+ ElementsAre(Location{URIForFile{HeaderNotInPreambleH},
+ HeaderNotInPreambleAnnotations.range()}));
}
TEST(Hover, All) {
Index: unittests/clangd/TestFS.h
===================================================================
--- unittests/clangd/TestFS.h
+++ unittests/clangd/TestFS.h
@@ -40,15 +40,22 @@
// A Compilation database that returns a fixed set of compile flags.
class MockCompilationDatabase : public GlobalCompilationDatabase {
public:
- /// When \p UseRelPaths is true, uses relative paths in compile commands.
- /// When \p UseRelPaths is false, uses absoulte paths.
- MockCompilationDatabase(bool UseRelPaths = false);
+ /// If \p Directory is not null, use that as the Directory field of the
+ /// CompileCommand.
+ ///
+ /// If \p RelPathPrefix is not null, use that as a prefix in front of the
+ /// source file name, instead of using an absolute path.
+ MockCompilationDatabase(StringRef Directory = StringRef(),
+ StringRef RelPathPrefix = StringRef());
llvm::Optional<tooling::CompileCommand>
getCompileCommand(PathRef File) const override;
std::vector<std::string> ExtraClangFlags;
- const bool UseRelPaths;
+
+private:
+ StringRef Directory;
+ StringRef RelPathPrefix;
};
// Returns an absolute (fake) test directory for this OS.
Index: unittests/clangd/TestFS.cpp
===================================================================
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -32,22 +32,36 @@
return MemFS;
}
-MockCompilationDatabase::MockCompilationDatabase(bool UseRelPaths)
- : ExtraClangFlags({"-ffreestanding"}), UseRelPaths(UseRelPaths) {
+MockCompilationDatabase::MockCompilationDatabase(StringRef Directory,
+ StringRef RelPathPrefix)
+ : ExtraClangFlags({"-ffreestanding"}), Directory(Directory),
+ RelPathPrefix(RelPathPrefix) {
// -ffreestanding avoids implicit stdc-predef.h.
}
Optional<tooling::CompileCommand>
MockCompilationDatabase::getCompileCommand(PathRef File) const {
if (ExtraClangFlags.empty())
return None;
- auto CommandLine = ExtraClangFlags;
auto FileName = sys::path::filename(File);
+
+ // Build the compile command.
+ auto CommandLine = ExtraClangFlags;
CommandLine.insert(CommandLine.begin(), "clang");
- CommandLine.insert(CommandLine.end(), UseRelPaths ? FileName : File);
- return {tooling::CompileCommand(sys::path::parent_path(File), FileName,
- std::move(CommandLine), "")};
+ if (RelPathPrefix == StringRef()) {
+ // Use the absolute path in the compile command.
+ CommandLine.push_back(File);
+ } else {
+ // Build a relative path using RelPathPrefix.
+ SmallString<32> RelativeFilePath(RelPathPrefix);
+ llvm::sys::path::append(RelativeFilePath, FileName);
+ CommandLine.push_back(RelativeFilePath.str());
+ }
+
+ return {tooling::CompileCommand(
+ Directory != StringRef() ? Directory : sys::path::parent_path(File),
+ FileName, std::move(CommandLine), "")};
}
const char *testRoot() {
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ 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(
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -62,8 +62,8 @@
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);
+llvm::Optional<std::string> getRealPath(const FileEntry *F,
+ const SourceManager &SourceMgr);
} // namespace clangd
} // namespace clang
#endif
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -186,18 +186,32 @@
}
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);
+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();}
} // namespace clangd
} // namespace clang
Index: clangd/FindSymbols.cpp
===================================================================
--- clangd/FindSymbols.cpp
+++ 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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
