Skip to content

Commit

Permalink
[clangd] Always retrieve ProjectInfo from Base in OverlayCDB
Browse files Browse the repository at this point in the history
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.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83934
  • Loading branch information
kadircet committed Jul 16, 2020
1 parent b9a6fb6 commit 46c9210
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
10 changes: 3 additions & 7 deletions clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,11 @@ void OverlayCDB::setCompileCommand(
}

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
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/GlobalCompilationDatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ 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 {
Expand All @@ -134,6 +133,8 @@ class OverlayCDB : public GlobalCompilationDatabase {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,22 @@ TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
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

0 comments on commit 46c9210

Please sign in to comment.