[clang-tools-extra] 4fb1f2e - [clangd] Fix the modification detection logic in `ClangdLSPServer::applyConfiguration`. (#115438)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 14 23:39:13 PST 2024
Author: Michael Park
Date: 2024-11-15T02:39:09-05:00
New Revision: 4fb1f2e58a2f423362b75f233896ea0d7179fc7a
URL: https://github.com/llvm/llvm-project/commit/4fb1f2e58a2f423362b75f233896ea0d7179fc7a
DIFF: https://github.com/llvm/llvm-project/commit/4fb1f2e58a2f423362b75f233896ea0d7179fc7a.diff
LOG: [clangd] Fix the modification detection logic in `ClangdLSPServer::applyConfiguration`. (#115438)
Prior to this, the "old != new" check would always evaluate to true because it was comparing a pre-mangling new command to a post-mangling old command.
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.h
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 06573a57554245..05dd313d0a0d35 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1419,15 +1419,12 @@ void ClangdLSPServer::applyConfiguration(
const ConfigurationSettings &Settings) {
// Per-file update to the compilation database.
llvm::StringSet<> ModifiedFiles;
- for (auto &Entry : Settings.compilationDatabaseChanges) {
- PathRef File = Entry.first;
- auto Old = CDB->getCompileCommand(File);
- auto New =
- tooling::CompileCommand(std::move(Entry.second.workingDirectory), File,
- std::move(Entry.second.compilationCommand),
+ for (auto &[File, Command] : Settings.compilationDatabaseChanges) {
+ auto Cmd =
+ tooling::CompileCommand(std::move(Command.workingDirectory), File,
+ std::move(Command.compilationCommand),
/*Output=*/"");
- if (Old != New) {
- CDB->setCompileCommand(File, std::move(New));
+ if (CDB->setCompileCommand(File, std::move(Cmd))) {
ModifiedFiles.insert(File);
}
}
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 1d96667a8e9f4a..71e97ac4efd673 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -807,7 +807,7 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
return Cmd;
}
-void OverlayCDB::setCompileCommand(PathRef File,
+bool OverlayCDB::setCompileCommand(PathRef File,
std::optional<tooling::CompileCommand> Cmd) {
// We store a canonical version internally to prevent mismatches between set
// and get compile commands. Also it assures clients listening to broadcasts
@@ -815,12 +815,19 @@ void OverlayCDB::setCompileCommand(PathRef File,
std::string CanonPath = removeDots(File);
{
std::unique_lock<std::mutex> Lock(Mutex);
- if (Cmd)
- Commands[CanonPath] = std::move(*Cmd);
- else
+ if (Cmd) {
+ if (auto [It, Inserted] =
+ Commands.try_emplace(CanonPath, std::move(*Cmd));
+ !Inserted) {
+ if (It->second == *Cmd)
+ return false;
+ It->second = *Cmd;
+ }
+ } else
Commands.erase(CanonPath);
}
OnCommandChanged.broadcast({CanonPath});
+ return true;
}
DelegatingCDB::DelegatingCDB(const GlobalCompilationDatabase *Base)
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
index ea999fe8aee017..f8349c6efecb01 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -203,7 +203,9 @@ class OverlayCDB : public DelegatingCDB {
tooling::CompileCommand getFallbackCommand(PathRef File) const override;
/// Sets or clears the compilation command for a particular file.
- void
+ /// Returns true if the command was changed (including insertion and removal),
+ /// false if it was unchanged.
+ bool
setCompileCommand(PathRef File,
std::optional<tooling::CompileCommand> CompilationCommand);
diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index a2ffdefe1bbcb2..c9e01e52dac1f3 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -92,11 +92,13 @@ TEST_F(OverlayCDBTest, GetCompileCommand) {
EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), std::nullopt);
auto Override = cmd(testPath("foo.cc"), "-DA=3");
- CDB.setCompileCommand(testPath("foo.cc"), Override);
+ EXPECT_TRUE(CDB.setCompileCommand(testPath("foo.cc"), Override));
+ EXPECT_FALSE(CDB.setCompileCommand(testPath("foo.cc"), Override));
EXPECT_THAT(CDB.getCompileCommand(testPath("foo.cc"))->CommandLine,
Contains("-DA=3"));
EXPECT_EQ(CDB.getCompileCommand(testPath("missing.cc")), std::nullopt);
- CDB.setCompileCommand(testPath("missing.cc"), Override);
+ EXPECT_TRUE(CDB.setCompileCommand(testPath("missing.cc"), Override));
+ EXPECT_FALSE(CDB.setCompileCommand(testPath("missing.cc"), Override));
EXPECT_THAT(CDB.getCompileCommand(testPath("missing.cc"))->CommandLine,
Contains("-DA=3"));
}
@@ -111,7 +113,7 @@ TEST_F(OverlayCDBTest, NoBase) {
OverlayCDB CDB(nullptr, {"-DA=6"});
EXPECT_EQ(CDB.getCompileCommand(testPath("bar.cc")), std::nullopt);
auto Override = cmd(testPath("bar.cc"), "-DA=5");
- CDB.setCompileCommand(testPath("bar.cc"), Override);
+ EXPECT_TRUE(CDB.setCompileCommand(testPath("bar.cc"), Override));
EXPECT_THAT(CDB.getCompileCommand(testPath("bar.cc"))->CommandLine,
Contains("-DA=5"));
@@ -128,10 +130,10 @@ TEST_F(OverlayCDBTest, Watch) {
Changes.push_back(ChangedFiles);
});
- Inner.setCompileCommand("A.cpp", tooling::CompileCommand());
- Outer.setCompileCommand("B.cpp", tooling::CompileCommand());
- Inner.setCompileCommand("A.cpp", std::nullopt);
- Outer.setCompileCommand("C.cpp", std::nullopt);
+ EXPECT_TRUE(Inner.setCompileCommand("A.cpp", tooling::CompileCommand()));
+ EXPECT_TRUE(Outer.setCompileCommand("B.cpp", tooling::CompileCommand()));
+ EXPECT_TRUE(Inner.setCompileCommand("A.cpp", std::nullopt));
+ EXPECT_TRUE(Outer.setCompileCommand("C.cpp", std::nullopt));
EXPECT_THAT(Changes, ElementsAre(ElementsAre("A.cpp"), ElementsAre("B.cpp"),
ElementsAre("A.cpp"), ElementsAre("C.cpp")));
}
@@ -151,7 +153,7 @@ TEST_F(OverlayCDBTest, Adjustments) {
tooling::CompileCommand BarCommand;
BarCommand.Filename = testPath("bar.cc");
BarCommand.CommandLine = {"clang++", "-DB=1", testPath("bar.cc")};
- CDB.setCompileCommand(testPath("bar.cc"), BarCommand);
+ EXPECT_TRUE(CDB.setCompileCommand(testPath("bar.cc"), BarCommand));
Cmd = *CDB.getCompileCommand(testPath("bar.cc"));
EXPECT_THAT(
Cmd.CommandLine,
@@ -412,7 +414,7 @@ TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
llvm::SmallString<128> Root(testRoot());
llvm::sys::path::append(Root, "build", "..", "a.cc");
- DB.setCompileCommand(Root.str(), tooling::CompileCommand());
+ EXPECT_TRUE(DB.setCompileCommand(Root.str(), tooling::CompileCommand()));
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(testPath("a.cc")));
DiscoveredFiles.clear();
@@ -432,7 +434,7 @@ TEST_F(OverlayCDBTest, GetProjectInfo) {
EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
// Shouldn't change after an override.
- DB.setCompileCommand(File, tooling::CompileCommand());
+ EXPECT_TRUE(DB.setCompileCommand(File, tooling::CompileCommand()));
EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
}
More information about the cfe-commits
mailing list