[clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[kadircet]

Summary

Clangd is returning current working directory for overriden commands.
This can cause inconsistencies between:

• header and the main files, as OverlayCDB only contains entries for the main files it direct any queries for the headers to the base, creating a discrepancy between the two.
• different clangd instances, as the results will be different depending on the timing of execution of the query and override of the command. hence clangd might see two different project infos for the same file between different invocations.
• editors and the way user has invoked it, as current working directory of clangd will depend on those, hence even when there's no underlying base CWD might change depending on the editor, or the directory user has started the editor in.

This patch gets rid of that discrepency by always directing queries to base or
returning llvm::None in absence of it.

For a sample bug see https://reviews.llvm.org/D83099#2154185.

[sammccall]

clang-tools-extra/clangd/GlobalCompilationDatabase.h

136

nit: drop "note that" for readability.

137

I find this comment confusing, you say "might" when I think you mean "might otherwise" (i.e. given the old behavior).
However the claim is true anyway for another reason: headers *may* simply be from a different CDB.

Maybe enough for the header is:
It wouldn't make much sense to treat files with overridden commands specially when
we can't do the same for the (unknown) local headers they include.

Even so I think this comment might belong rather in the implementation file...

[kadircet]

• Split comments between implementation and header, and reword.

clang-tools-extra/clangd/GlobalCompilationDatabase.h

 
 /// Extracts system include search path from drivers matching QueryDriverGlobs
 /// and adds them to the compile flags. Base may not be nullptr.
 /// Returns Base when \p QueryDriverGlobs is empty.
 std::unique_ptr<GlobalCompilationDatabase>
 getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
                        std::unique_ptr<GlobalCompilationDatabase> Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
 public:
   // Base may be null, in which case no entries are inherited.
   // FallbackFlags are added to the fallback compile command.
   // Adjuster is applied to all commands, fallback or not.
   OverlayCDB(const GlobalCompilationDatabase *Base,
              std::vector<std::string> FallbackFlags = {},
              tooling::ArgumentsAdjuster Adjuster = nullptr);
 
   llvm::Optional<tooling::CompileCommand>
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Project info is gathered purely from the inner compilation database to
+  /// ensure consistency.
   llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.
   void
   setCompileCommand(PathRef File,
                     llvm::Optional<tooling::CompileCommand> CompilationCommand);
 
 private:

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

if (Cmd)
       Commands[CanonPath] = std::move(*Cmd);
     else
       Commands.erase(CanonPath);
   }
   OnCommandChanged.broadcast({CanonPath});
 }
 
 llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
-  {
-    std::lock_guard<std::mutex> Lock(Mutex);
-    auto It = Commands.find(removeDots(File));
-    if (It != Commands.end())
-      return ProjectInfo{};
-  }
+  // It wouldn't make much sense to treat files with overridden commands
+  // specially when we can't do the same for the (unknown) local headers they
+  // include or changing behavior mid-air after receiving an override.
   if (Base)
     return Base->getProjectInfo(File);
-
   return llvm::None;
 }
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
   DB.setCompileCommand(Root.str(), tooling::CompileCommand());
   EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(testPath("a.cc")));
   DiscoveredFiles.clear();
 
   llvm::SmallString<128> File(testRoot());
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang