Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -228,6 +228,9 @@ Semaphore &Barrier; /// Inputs, corresponding to the current state of AST. ParseInputs FileInputs; + /// Whether the diagnostics for the current FileInputs were reported to the + /// users before. + bool DiagsWereReported = false; /// Size of the last AST /// Guards members used by both TUScheduler and the worker thread. mutable std::mutex Mutex; @@ -330,7 +333,9 @@ std::tie(Inputs.CompileCommand, Inputs.Contents); tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand); + bool PrevDiagsWereReported = DiagsWereReported; FileInputs = Inputs; + DiagsWereReported = false; log("Updating file {0} with command [{1}] {2}", FileName, Inputs.CompileCommand.Directory, @@ -366,7 +371,12 @@ OldPreamble.reset(); PreambleWasBuilt.notify(); - if (CanReuseAST) { + if (CanReuseAST) + DiagsWereReported = PrevDiagsWereReported; + else + IdleASTs.take(this); // Remove the old AST if it's still in cache. + + if (DiagsWereReported) { // Take a shortcut and don't build the AST if neither the inputs nor the // preamble have changed. // Note that we do not report the diagnostics, since they should not have @@ -380,20 +390,25 @@ FileName); return; } - // Remove the old AST if it's still in cache. - IdleASTs.take(this); // Build the AST for diagnostics. - llvm::Optional AST = - buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs); + llvm::Optional> AST = IdleASTs.take(this); + if (!AST) { + llvm::Optional NewAST = + buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs); + AST = NewAST ? llvm::make_unique(std::move(*NewAST)) : nullptr; + } + // We want to report the diagnostics even if this update was cancelled. // It seems more useful than making the clients wait indefinitely if they // spam us with updates. - if (WantDiags != WantDiagnostics::No && AST) - OnUpdated(AST->getDiagnostics()); + // Note *AST can be still be null if buildAST fails. + if (WantDiags != WantDiagnostics::No && *AST) { + OnUpdated((*AST)->getDiagnostics()); + DiagsWereReported = true; + } // Stash the AST in the cache for further use. - IdleASTs.put(this, - AST ? llvm::make_unique(std::move(*AST)) : nullptr); + IdleASTs.put(this, std::move(*AST)); }; startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags); Index: unittests/clangd/TUSchedulerTests.cpp =================================================================== --- unittests/clangd/TUSchedulerTests.cpp +++ unittests/clangd/TUSchedulerTests.cpp @@ -390,5 +390,38 @@ ASSERT_FALSE(DoUpdate(getInputs(Source, OtherSourceContents))); } +TEST_F(TUSchedulerTests, NoChangeDiags) { + TUScheduler S( + /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), + /*StorePreambleInMemory=*/true, PreambleParsedCallback(), + /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), + ASTRetentionPolicy()); + + auto FooCpp = testPath("foo.cpp"); + auto Contents = "int a; int b;"; + + S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::No, + [](std::vector) { ADD_FAILURE() << "Should not be called."; }); + S.runWithAST("touchAST", FooCpp, [](llvm::Expected IA) { + // Make sure the AST was actually built. + cantFail(std::move(IA)); + }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); + + // Even though the inputs didn't change and AST can be reused, we need to + // report the diagnostics, as they were not reported previously. + std::atomic SeenDiags(false); + S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto, + [&](std::vector) { SeenDiags = true; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); + ASSERT_TRUE(SeenDiags); + + // Subsequent request does not get any diagnostics callback because the same + // diags have previously been reported and the inputs didn't change. + S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto, + [&](std::vector) { ADD_FAILURE() << "Should not be called."; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); +} + } // namespace clangd } // namespace clang