simark updated this revision to Diff 155057.
simark added a comment.

This is an update of my work in progress.  I haven't done any sharing with
SymbolCollector, as it's not clear to me how to proceed with that.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687

Files:
  clangd/SourceCode.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
@@ -39,15 +39,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
@@ -30,22 +30,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/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -185,18 +185,39 @@
   return Edits;
 }
 
+/// Turn \p InputPath, a relative or absolute path, to a real path.
+static llvm::Optional<std::string>
+getAbsoluteFilePath(StringRef InputPath, const FileManager &FileMgr) {
+  llvm::SmallString<128> Path = InputPath;
+
+  if (!llvm::sys::path::is_absolute(Path)) {
+    if (!FileMgr.makeAbsolutePath(Path)) {
+      log("Could not turn relative path to absolute: {0}", Path);
+      return llvm::None;
+    }
+  }
+
+  llvm::SmallString<128> AbsolutePath;
+  if (FileMgr.getVirtualFileSystem()->getRealPath(Path, AbsolutePath)) {
+    log("Could not compute real path: {0}", Path);
+    return Path.str().str();
+  }
+
+  return AbsolutePath.str().str();
+}
+
 llvm::Optional<std::string>
 getAbsoluteFilePath(const FileEntry *F, const SourceManager &SourceMgr) {
+  // Ideally, we get the real path from the FileEntry object.
   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);
-      return llvm::None;
-    }
+  if (!FilePath.empty()) {
+    return FilePath.str().str();
   }
-  return FilePath.str().str();
+
+  // Otherwise, we try to compute ourselves.
+  vlog("FileEntry for {0} did not contain the real path.", F->getName());
+
+  return getAbsoluteFilePath(F->getName(), SourceMgr.getFileManager());
 }
 
 } // namespace clangd
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to