diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -52,7 +52,7 @@ /// This function does not check if preamble is valid to reuse. static llvm::Optional build(llvm::StringRef Filename, const ParseInputs &Inputs, - std::unique_ptr CI, + std::unique_ptr CI, bool ProduceDiagnostics, llvm::ArrayRef CompilerInvocationDiags, std::shared_ptr Preamble); diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -22,6 +22,7 @@ #include "support/Trace.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" @@ -243,6 +244,7 @@ llvm::Optional ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::unique_ptr CI, + bool ProduceDiagnostics, llvm::ArrayRef CompilerInvocationDiags, std::shared_ptr Preamble) { trace::Span Tracer("BuildAST"); @@ -267,8 +269,11 @@ // This is on-by-default in windows to allow parsing SDK headers, but it // breaks many features. Disable it for the main-file (not preamble). CI->getLangOpts()->DelayedTemplateParsing = false; + if (!ProduceDiagnostics) + CI->getDiagnosticOpts().IgnoreWarnings = true; StoreDiags ASTDiags; + IgnoringDiagConsumer IgnoreDiags; llvm::Optional Patch; if (Preamble) { @@ -278,7 +283,8 @@ auto Clang = prepareCompilerInstance( std::move(CI), PreamblePCH, llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS, - ASTDiags); + ProduceDiagnostics ? llvm::cast(ASTDiags) + : llvm::cast(IgnoreDiags)); if (!Clang) return None; @@ -297,9 +303,9 @@ // ancestors outside this scope). // In practice almost all checks work well without modifications. std::vector> CTChecks; - ast_matchers::MatchFinder CTFinder; + llvm::Optional CTFinder; llvm::Optional CTContext; - { + if (ProduceDiagnostics) { trace::Span Tracer("ClangTidyInit"); dlog("ClangTidy configuration for file {0}: {1}", Filename, tidy::configurationAsText(Inputs.Opts.ClangTidyOpts)); @@ -343,13 +349,14 @@ return DiagLevel; }); Preprocessor *PP = &Clang->getPreprocessor(); + CTFinder.emplace(); for (const auto &Check : CTChecks) { if (!Check->isLanguageVersionSupported(CTContext->getLangOpts())) continue; // FIXME: the PP callbacks skip the entire preamble. // Checks that want to see #includes in the main file do not see them. Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP); - Check->registerMatchers(&CTFinder); + Check->registerMatchers(CTFinder.getPointer()); } } @@ -357,8 +364,8 @@ // (e.g. incomplete type) and attach include insertion fixes to diagnostics. llvm::Optional FixIncludes; auto BuildDir = VFS->getCurrentWorkingDirectory(); - if (Inputs.Opts.SuggestMissingIncludes && Inputs.Index && - !BuildDir.getError()) { + if (ProduceDiagnostics && Inputs.Opts.SuggestMissingIncludes && + Inputs.Index && !BuildDir.getError()) { auto Style = getFormatStyleForFile(Filename, Inputs.Contents, VFS.get()); auto Inserter = std::make_shared( Filename, Inputs.Contents, Style, BuildDir.get(), @@ -424,15 +431,14 @@ std::vector ParsedDecls = Action->takeTopLevelDecls(); // AST traversals should exclude the preamble, to avoid performance cliffs. Clang->getASTContext().setTraversalScope(ParsedDecls); - { + if (CTFinder.hasValue()) { // Run the AST-dependent part of the clang-tidy checks. // (The preprocessor part ran already, via PPCallbacks). trace::Span Tracer("ClangTidyMatch"); - CTFinder.matchAST(Clang->getASTContext()); + CTFinder->matchAST(Clang->getASTContext()); } - // UnitDiagsConsumer is local, we can not store it in CompilerInstance that - // has a longer lifetime. + // Our diagnostic consumer is local, CompilerInstance must outlive it. Clang->getDiagnostics().setClient(new IgnoreDiagnostics); // CompilerInstance won't run this callback, do it directly. ASTDiags.EndSourceFile(); @@ -441,14 +447,17 @@ // So just inform the preprocessor of EOF, while keeping everything alive. Clang->getPreprocessor().EndSourceFile(); - std::vector Diags = CompilerInvocationDiags; - // Add diagnostics from the preamble, if any. - if (Preamble) - Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end()); - // Finally, add diagnostics coming from the AST. - { - std::vector D = ASTDiags.take(CTContext.getPointer()); - Diags.insert(Diags.end(), D.begin(), D.end()); + std::vector Diags; + if (ProduceDiagnostics) { + Diags = CompilerInvocationDiags; + // Add diagnostics from the preamble, if any. + if (Preamble) + Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end()); + // Finally, add diagnostics coming from the AST. + { + std::vector D = ASTDiags.take(CTContext.getPointer()); + Diags.insert(Diags.end(), D.begin(), D.end()); + } } return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -110,6 +110,7 @@ /// Indicates whether clang failed to build the TU. bool BuildFailed = false; /// Indicates whether we reused the prebuilt AST. + /// FIXME: always false, diagnostics are always built from fresh ASTs. bool ReuseAST = false; }; /// Serialize this to an LSP file status item. diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -675,9 +675,9 @@ llvm::Optional> AST = IdleASTs.take(this, &ASTAccessForRead); if (!AST) { - StoreDiags CompilerInvocationDiagConsumer; + IgnoreDiagnostics IgnoreDiags; std::unique_ptr Invocation = - buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer); + buildCompilerInvocation(FileInputs, IgnoreDiags); // Try rebuilding the AST. vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name, FileName, FileInputs.Version); @@ -687,7 +687,7 @@ llvm::Optional NewAST; if (Invocation) { NewAST = ParsedAST::build(FileName, FileInputs, std::move(Invocation), - CompilerInvocationDiagConsumer.take(), + /*ProduceDiagnostics=*/false, {}, getPossiblyStalePreamble()); ++ASTBuildCount; } @@ -796,9 +796,6 @@ void ASTWorker::generateDiagnostics( std::unique_ptr Invocation, ParseInputs Inputs, std::vector CIDiags) { - // Tracks ast cache accesses for publishing diags. - static constexpr trace::Metric ASTAccessForDiag( - "ast_access_diag", trace::Metric::Counter, "result"); assert(Invocation); // No need to rebuild the AST if we won't send the diagnostics. { @@ -824,40 +821,27 @@ Status.ASTActivity.K = ASTAction::Building; Status.ASTActivity.Name = std::move(TaskName); }); - // We might be able to reuse the last we've built for a read request. - // FIXME: It might be better to not reuse this AST. That way queued AST builds - // won't be required for diags. - llvm::Optional> AST = - IdleASTs.take(this, &ASTAccessForDiag); - if (!AST || !InputsAreLatest) { - auto RebuildStartTime = DebouncePolicy::clock::now(); - llvm::Optional NewAST = ParsedAST::build( - FileName, Inputs, std::move(Invocation), CIDiags, LatestPreamble); - auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime; - ++ASTBuildCount; - // Try to record the AST-build time, to inform future update debouncing. - // This is best-effort only: if the lock is held, don't bother. - std::unique_lock Lock(Mutex, std::try_to_lock); - if (Lock.owns_lock()) { - // Do not let RebuildTimes grow beyond its small-size (i.e. - // capacity). - if (RebuildTimes.size() == RebuildTimes.capacity()) - RebuildTimes.erase(RebuildTimes.begin()); - RebuildTimes.push_back(RebuildDuration); - Lock.unlock(); - } - Status.update([&](TUStatus &Status) { - Status.Details.ReuseAST = false; - Status.Details.BuildFailed = !NewAST.hasValue(); - }); - AST = NewAST ? std::make_unique(std::move(*NewAST)) : nullptr; - } else { - log("Skipping rebuild of the AST for {0}, inputs are the same.", FileName); - Status.update([](TUStatus &Status) { - Status.Details.ReuseAST = true; - Status.Details.BuildFailed = false; - }); + auto RebuildStartTime = DebouncePolicy::clock::now(); + llvm::Optional AST = + ParsedAST::build(FileName, Inputs, std::move(Invocation), + /*ProduceDiagnostics=*/true, CIDiags, LatestPreamble); + auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime; + ++ASTBuildCount; + // Try to record the AST-build time, to inform future update debouncing. + // This is best-effort only: if the lock is held, don't bother. + std::unique_lock Lock(Mutex, std::try_to_lock); + if (Lock.owns_lock()) { + // Do not let RebuildTimes grow beyond its small-size (i.e. + // capacity). + if (RebuildTimes.size() == RebuildTimes.capacity()) + RebuildTimes.erase(RebuildTimes.begin()); + RebuildTimes.push_back(RebuildDuration); + Lock.unlock(); } + Status.update([&](TUStatus &Status) { + Status.Details.ReuseAST = false; + Status.Details.BuildFailed = !AST.hasValue(); + }); // Publish diagnostics. auto RunPublish = [&](llvm::function_ref Publish) { @@ -867,9 +851,9 @@ if (CanPublishResults) Publish(); }; - if (*AST) { + if (AST) { trace::Span Span("Running main AST callback"); - Callbacks.onMainAST(FileName, **AST, RunPublish); + Callbacks.onMainAST(FileName, *AST, RunPublish); } else { // Failed to build the AST, at least report diagnostics from the // command line if there were any. @@ -882,8 +866,10 @@ // queue raced ahead while we were waiting on the preamble. In that case the // queue can't reuse the AST. if (InputsAreLatest) { - RanASTCallback = *AST != nullptr; - IdleASTs.put(this, std::move(*AST)); + RanASTCallback = AST.hasValue(); + IdleASTs.take(this); // Put requires the entry doesn't exist. + IdleASTs.put(this, + AST ? std::make_unique(std::move(*AST)) : nullptr); } } diff --git a/clang-tools-extra/clangd/test/metrics.test b/clang-tools-extra/clangd/test/metrics.test --- a/clang-tools-extra/clangd/test/metrics.test +++ b/clang-tools-extra/clangd/test/metrics.test @@ -4,7 +4,7 @@ {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}} # Don't verify value, timestamp, or order. # CHECK-DAG: d,lsp_latency,textDocument/didOpen, -# CHECK-DAG: c,ast_access_diag,miss, +# CHECK-DAG: d,span_latency,BuildAST, --- {"jsonrpc":"2.0","id":5,"method":"shutdown"} --- diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -24,6 +24,7 @@ #include "TestFS.h" #include "TestTU.h" #include "clang/AST/DeclTemplate.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" @@ -44,6 +45,7 @@ using ::testing::AllOf; using ::testing::ElementsAre; using ::testing::ElementsAreArray; +using ::testing::IsEmpty; MATCHER_P(DeclNamed, Name, "") { if (NamedDecl *ND = dyn_cast(arg)) @@ -56,6 +58,8 @@ return false; } +MATCHER_P(Diag, Message, "") { return arg.Message == Message; } + // Matches if the Decl has template args equal to ArgName. If the decl is a // NamedDecl and ArgName is an empty string it also matches. MATCHER_P(WithTemplateArgs, ArgName, "") { @@ -452,8 +456,9 @@ ASSERT_TRUE(EmptyPreamble); TU.Code = "#include "; Includes.clear(); - auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(), - std::move(CI), {}, EmptyPreamble); + auto PatchedAST = + ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), + /*ProduceDiagnostics=*/true, {}, EmptyPreamble); ASSERT_TRUE(PatchedAST); // Make sure includes were seen only once. EXPECT_THAT(Includes, @@ -461,6 +466,66 @@ WithFileName("a.h"))); } +TEST(ParsedASTTest, NoDiagnostics) { + static bool TidyCheckIsError; + // A simple clang-tidy check, which we can verify: + // - is used when ProduceDiagnostics is on + // - does no work (isn't instantiated) when ProduceDiagnostics is off. + struct NoVars : public tidy::ClangTidyCheck { + NoVars(StringRef N, tidy::ClangTidyContext *C) : ClangTidyCheck(N, C) { + EXPECT_FALSE(TidyCheckIsError) << "Shouldn't even be instantiated!"; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override { + Finder->addMatcher(ast_matchers::varDecl().bind("var"), this); + } + void check(const ast_matchers::MatchFinder::MatchResult &Result) override { + diag(Result.Nodes.getNodeAs("var")->getLocation(), "tidy"); + } + }; + struct NoVarsModule : public tidy::ClangTidyModule { + void addCheckFactories(tidy::ClangTidyCheckFactories &CF) override { + CF.registerCheck("no-vars"); + } + }; + static tidy::ClangTidyModuleRegistry::Add X("no-vars-mod", ""); + + StoreDiags StoreDriverDiags; + TestTU TU; + TU.ExtraArgs.push_back("-flag"); + TU.AdditionalFiles["header.h"] = "#error header"; + TU.Code = R"cpp( + #include "header.h" + #error preamble + int foo; + #error body + )cpp"; + TU.ClangTidyChecks = "no-vars"; + + auto CI = *buildCompilerInvocation(TU.inputs(), StoreDriverDiags); + auto Preamble = TU.preamble(); + auto Inputs = TU.inputs(); + auto DriverDiags = StoreDriverDiags.take(); + + // Build with diagnostics, to sanity-check the inputs. + TidyCheckIsError = false; + auto AST = ParsedAST::build( + testPath(TU.Filename), Inputs, std::make_unique(CI), + /*ProduceDiagnostics=*/true, DriverDiags, Preamble); + EXPECT_THAT(AST->getDiagnostics(), + ElementsAre(Diag("unknown argument: '-flag'"), + Diag("in included file: header"), Diag("preamble"), + Diag("body"), Diag("tidy"))); + EXPECT_EQ(Decl::Var, findDecl(*AST, "foo").getKind()); + + // Build with no diagnostics. + TidyCheckIsError = true; // Verify we don't do *any* clang-tidy work. + AST = ParsedAST::build(testPath(TU.Filename), Inputs, + std::make_unique(CI), + /*ProduceDiagnostics=*/false, DriverDiags, Preamble); + EXPECT_THAT(AST->getDiagnostics(), IsEmpty()) << "ProduceDiagnostics = false"; + EXPECT_EQ(Decl::Var, findDecl(*AST, "foo").getKind()) << "AST still good"; +} + TEST(ParsedASTTest, PatchesAdditionalIncludes) { llvm::StringLiteral ModifiedContents = R"cpp( #include "baz.h" @@ -489,13 +554,14 @@ auto EmptyPreamble = buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); ASSERT_TRUE(EmptyPreamble); - EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, testing::IsEmpty()); + EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty()); // Now build an AST using empty preamble and ensure patched includes worked. TU.Code = ModifiedContents.str(); Inputs = TU.inputs(); - auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), - {}, EmptyPreamble); + auto PatchedAST = + ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), + /*ProduceDiagnostics=*/false, {}, EmptyPreamble); ASSERT_TRUE(PatchedAST); ASSERT_TRUE(PatchedAST->getDiagnostics().empty()); @@ -538,8 +604,9 @@ // correctly parsed. TU.Code = ""; Inputs = TU.inputs(); - auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), - {}, BaselinePreamble); + auto PatchedAST = + ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI), + /*ProduceDiagnostics=*/false, {}, BaselinePreamble); ASSERT_TRUE(PatchedAST); // Ensure source location information is correct. diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -205,8 +205,8 @@ ADD_FAILURE() << "Failed to build compiler invocation"; return llvm::None; } - return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), {}, - BaselinePreamble); + return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), + /*ProduceDiagnostics=*/false, {}, BaselinePreamble); } std::string getPreamblePatch(llvm::StringRef Baseline, diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -521,7 +521,6 @@ } TEST_F(TUSchedulerTests, EvictedAST) { - std::atomic BuiltASTCounter(0); auto Opts = optsForTest(); Opts.AsyncThreadsCount = 1; Opts.RetentionPolicy.MaxRetainedASTs = 2; @@ -532,52 +531,45 @@ int* a; double* b = a; )cpp"; - llvm::StringLiteral OtherSourceContents = R"cpp( - int* a; - double* b = a + 0; - )cpp"; auto Foo = testPath("foo.cpp"); auto Bar = testPath("bar.cpp"); auto Baz = testPath("baz.cpp"); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0)); - // Build one file in advance. We will not access it later, so it will be the - // one that the cache will evict. - updateWithCallback(S, Foo, SourceContents, WantDiagnostics::Yes, - [&BuiltASTCounter]() { ++BuiltASTCounter; }); + // Build one file in advance. + S.update(Foo, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); - ASSERT_EQ(BuiltASTCounter.load(), 1); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(1)); - - // Build two more files. Since we can retain only 2 ASTs, these should be - // the ones we see in the cache later. - updateWithCallback(S, Bar, SourceContents, WantDiagnostics::Yes, - [&BuiltASTCounter]() { ++BuiltASTCounter; }); - updateWithCallback(S, Baz, SourceContents, WantDiagnostics::Yes, - [&BuiltASTCounter]() { ++BuiltASTCounter; }); + // AST should be cached after diagnostics are produced. + ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Foo)); + + // Build two more files. + S.update(Bar, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes); + S.update(Baz, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); - ASSERT_EQ(BuiltASTCounter.load(), 3); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(2)); + // We can retain only 2 ASTs, Foo should have been evicted. + ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz)); - // Check only the last two ASTs are retained. + // No reads yet. + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0)); + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0)); + + // Run action on file in AST cache. + S.runWithAST("Cached", Bar, + [](llvm::Expected E) { EXPECT_TRUE(bool(E)); }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); + // Should have hit the cache, same files still present. ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz)); + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(1)); + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0)); // Access the old file again. - updateWithCallback(S, Foo, OtherSourceContents, WantDiagnostics::Yes, - [&BuiltASTCounter]() { ++BuiltASTCounter; }); + S.runWithAST("Uncached", Foo, + [](llvm::Expected E) { EXPECT_TRUE(bool(E)); }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); - ASSERT_EQ(BuiltASTCounter.load(), 4); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(1)); - - // Check the AST for foo.cpp is retained now and one of the others got - // evicted. - EXPECT_THAT(S.getFilesWithCachedAST(), - UnorderedElementsAre(Foo, AnyOf(Bar, Baz))); + // We missed the cache. Now Foo is retained and Baz got evicted (LRU). + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0)); + EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(1)); + EXPECT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Foo, Bar)); } // We send "empty" changes to TUScheduler when we think some external event @@ -798,8 +790,6 @@ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0)); EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0)); updateWithDiags( S, FooCpp, Contents, WantDiagnostics::No, [](std::vector) { ADD_FAILURE() << "Should not be called."; }); @@ -811,15 +801,13 @@ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0)); EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(1)); - // Even though the inputs didn't change and AST can be reused, we need to + // Even though the inputs didn't change, we need to // report the diagnostics, as they were not reported previously. std::atomic SeenDiags(false); updateWithDiags(S, FooCpp, Contents, WantDiagnostics::Auto, [&](std::vector) { SeenDiags = true; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); ASSERT_TRUE(SeenDiags); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(1)); - EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0)); // Subsequent request does not get any diagnostics callback because the same // diags have previously been reported and the inputs didn't change. diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -84,8 +84,9 @@ auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr); - auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI), - Diags.take(), Preamble); + auto AST = + ParsedAST::build(testPath(Filename), Inputs, std::move(CI), + /*ProduceDiagnostics=*/true, Diags.take(), Preamble); if (!AST.hasValue()) { ADD_FAILURE() << "Failed to build code:\n" << Code; llvm_unreachable("Failed to build TestTU!");