This revision was automatically updated to reflect the committed changes.
Closed by commit rL325522: [clangd] Do not reuse preamble if compile args
changed (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43454?vs=134878&id=134941#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43454
Files:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -373,6 +373,46 @@
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
}
+TEST_F(ClangdVFSTest, ForceReparseCompileCommandDefines) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*AsyncThreadsCount=*/0,
+ /*StorePreamblesInMemory=*/true);
+
+ // No need to sync reparses, because reparses are performed on the calling
+ // thread.
+ auto FooCpp = testPath("foo.cpp");
+ const auto SourceContents = R"cpp(
+#ifdef WITH_ERROR
+this
+#endif
+
+int main() { return 0; }
+)cpp";
+ FS.Files[FooCpp] = "";
+ FS.ExpectedFile = FooCpp;
+
+ // Parse with define, we expect to see the errors.
+ CDB.ExtraClangFlags = {"-DWITH_ERROR"};
+ Server.addDocument(FooCpp, SourceContents);
+ EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
+
+ // Parse without the define, no errors should be produced.
+ CDB.ExtraClangFlags = {};
+ // Currently, addDocument never checks if CompileCommand has changed, so we
+ // expect to see the errors.
+ Server.addDocument(FooCpp, SourceContents);
+ EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
+ // But forceReparse should reparse the file with proper flags.
+ Server.forceReparse(FooCpp);
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
+ // Subsequent addDocument call should finish without errors too.
+ Server.addDocument(FooCpp, SourceContents);
+ EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
+}
+
TEST_F(ClangdVFSTest, MemoryUsage) {
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -34,6 +34,13 @@
namespace {
+bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
+ const tooling::CompileCommand &RHS) {
+ // We don't check for Output, it should not matter to clangd.
+ return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+ llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine);
+}
+
template <class T> std::size_t getUsedBytes(const std::vector<T> &Vec) {
return Vec.capacity() * sizeof(T);
}
@@ -417,7 +424,7 @@
// Compute updated Preamble.
std::shared_ptr<const PreambleData> NewPreamble =
- rebuildPreamble(*CI, Inputs.FS, *ContentsBuffer);
+ rebuildPreamble(*CI, Inputs.CompileCommand, Inputs.FS, *ContentsBuffer);
// Remove current AST to avoid wasting memory.
AST = llvm::None;
@@ -445,6 +452,7 @@
}
// Write the results of rebuild into class fields.
+ Command = std::move(Inputs.CompileCommand);
Preamble = std::move(NewPreamble);
AST = std::move(NewAST);
return Diagnostics;
@@ -471,11 +479,12 @@
std::shared_ptr<const PreambleData>
CppFile::rebuildPreamble(CompilerInvocation &CI,
+ const tooling::CompileCommand &Command,
IntrusiveRefCntPtr<vfs::FileSystem> FS,
llvm::MemoryBuffer &ContentsBuffer) const {
const auto &OldPreamble = this->Preamble;
auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), &ContentsBuffer, 0);
- if (OldPreamble &&
+ if (OldPreamble && compileCommandsAreEqual(this->Command, Command) &&
OldPreamble->Preamble.CanReuse(CI, &ContentsBuffer, Bounds, FS.get())) {
log("Reusing preamble for file " + Twine(FileName));
return OldPreamble;
Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -152,12 +152,15 @@
/// This method is const to ensure we don't incidentally modify any fields.
std::shared_ptr<const PreambleData>
rebuildPreamble(CompilerInvocation &CI,
+ const tooling::CompileCommand &Command,
IntrusiveRefCntPtr<vfs::FileSystem> FS,
llvm::MemoryBuffer &ContentsBuffer) const;
const Path FileName;
const bool StorePreamblesInMemory;
+ /// The last CompileCommand used to build AST and Preamble.
+ tooling::CompileCommand Command;
/// The last parsed AST.
llvm::Optional<ParsedAST> AST;
/// The last built Preamble.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
