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); } Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -63,12 +63,11 @@ const tooling::Replacements &Repls); /// Get the absolute file path of a given file entry. -llvm::Optional getAbsoluteFilePath(const FileEntry *F, - const SourceManager &SourceMgr); - TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M, const LangOptions &L); +llvm::Optional getRealPath(const FileEntry *F, + const SourceManager &SourceMgr); } // namespace clangd } // namespace clang #endif Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ clangd/SourceCode.cpp @@ -185,18 +185,34 @@ return Edits; } -llvm::Optional -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 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, 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: 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 getCompileCommand(PathRef File) const override; std::vector 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,8 +32,10 @@ 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. } @@ -42,12 +44,24 @@ 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.empty()) { + // 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: 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) {