Skip to content

Commit 46c9210

Browse files
committed
[clangd] Always retrieve ProjectInfo from Base in OverlayCDB
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
1 parent b9a6fb6 commit 46c9210

File tree

3 files changed

+19
-9
lines changed

3 files changed

+19
-9
lines changed

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -298,15 +298,11 @@ void OverlayCDB::setCompileCommand(
298298
}
299299

300300
llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
301-
{
302-
std::lock_guard<std::mutex> Lock(Mutex);
303-
auto It = Commands.find(removeDots(File));
304-
if (It != Commands.end())
305-
return ProjectInfo{};
306-
}
301+
// It wouldn't make much sense to treat files with overridden commands
302+
// specially when we can't do the same for the (unknown) local headers they
303+
// include or changing behavior mid-air after receiving an override.
307304
if (Base)
308305
return Base->getProjectInfo(File);
309-
310306
return llvm::None;
311307
}
312308
} // namespace clangd

clang-tools-extra/clangd/GlobalCompilationDatabase.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ std::unique_ptr<GlobalCompilationDatabase>
119119
getQueryDriverDatabase(llvm::ArrayRef<std::string> QueryDriverGlobs,
120120
std::unique_ptr<GlobalCompilationDatabase> Base);
121121

122-
123122
/// Wraps another compilation database, and supports overriding the commands
124123
/// using an in-memory mapping.
125124
class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@ class OverlayCDB : public GlobalCompilationDatabase {
134133
llvm::Optional<tooling::CompileCommand>
135134
getCompileCommand(PathRef File) const override;
136135
tooling::CompileCommand getFallbackCommand(PathRef File) const override;
136+
/// Project info is gathered purely from the inner compilation database to
137+
/// ensure consistency.
137138
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
138139

139140
/// Sets or clears the compilation command for a particular file.

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,22 @@ TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
313313
llvm::sys::path::append(File, "blabla", "..", "a.cc");
314314

315315
EXPECT_TRUE(DB.getCompileCommand(File));
316-
EXPECT_TRUE(DB.getProjectInfo(File));
316+
EXPECT_FALSE(DB.getProjectInfo(File));
317317
}
318318

319+
TEST_F(OverlayCDBTest, GetProjectInfo) {
320+
OverlayCDB DB(Base.get());
321+
Path File = testPath("foo.cc");
322+
Path Header = testPath("foo.h");
323+
324+
EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
325+
EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
326+
327+
// Shouldn't change after an override.
328+
DB.setCompileCommand(File, tooling::CompileCommand());
329+
EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
330+
EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
331+
}
319332
} // namespace
320333
} // namespace clangd
321334
} // namespace clang

0 commit comments

Comments
 (0)