[clangd] Get rid of WantDiagnostics::Yes

[kadircet]

Summary

This is an extension clangd provides in LSP layer and to embedders of
ClangdServer. Currently there are no users of WantDiagnostics::Yes apart from
our tests, as from a practical point of view it is quite similar to
WantDiagnostics::Auto.

This patch gets rid of WantDiagnostics::Yes to get rid of some logic in
TUScheduler that handles it.

Tests are updated using the facts that an update is executed if:

• there's a read following it, or
• it is the last update in the request queue.

[kadircet]

ping :)

[sammccall]

Sorry for the long delay here...

clang-tools-extra/clangd/ClangdLSPServer.cpp

670

This cuts against the assertion in the description that there are no uses!

And I believe the debounce test demonstrates that that we will in fact debounce an initial request that's Auto, right?
So isn't this going to add 0.5sec to time-before-first-diagnostics in all cases?

[kadircet]

clang-tools-extra/clangd/ClangdLSPServer.cpp

670

ah right, let's reconsider this after having a special initial AST build (one without preamble serialization/deserialization and clang-tidy) to reduce first diagnostic latency.

[sammccall]

clang-tools-extra/clangd/ClangdLSPServer.cpp

670

Yeah, 0.5s isn't a killer so we *could* sequence it the other way.
But it'd be a bit unfortunate as the temporary regression would probably make it into the 11 release, so since this is "just" tech debt cleanup it'd be nice to hold off.

clang-tools-extra/clangd/ClangdLSPServer.cpp

 
 void ClangdLSPServer::onDocumentDidOpen(
     const DidOpenTextDocumentParams &Params) {
   PathRef File = Params.textDocument.uri.file();
 
   const std::string &Contents = Params.textDocument.text;
 
   auto Version = DraftMgr.addDraft(File, Params.textDocument.version, Contents);
-  Server->addDocument(File, Contents, encodeVersion(Version),
-                      WantDiagnostics::Yes);
+  Server->addDocument(File, Contents, encodeVersion(Version));
 }
 
 void ClangdLSPServer::onDocumentDidChange(
     const DidChangeTextDocumentParams &Params) {
   auto WantDiags = WantDiagnostics::Auto;
   if (Params.wantDiagnostics.hasValue())
-    WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes
+    WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Auto
                                                   : WantDiagnostics::No;
 
   PathRef File = Params.textDocument.uri.file();
   llvm::Expected<DraftStore::Draft> Draft = DraftMgr.updateDraft(
       File, Params.textDocument.version, Params.contentChanges);
   if (!Draft) {
     // If this fails, we are most likely going to be not in sync anymore with
     // the client.  It is better to remove the draft and let further operations

clang-tools-extra/clangd/ClangdServer.h

public:
   /// diagnostics etc built from it.
   void addDocument(PathRef File, StringRef Contents,
                    llvm::StringRef Version = "null",
                    WantDiagnostics WD = WantDiagnostics::Auto,
                    bool ForceRebuild = false);
 
   /// Remove \p File from list of tracked files, schedule a request to free
   /// resources associated with it. Pending diagnostics for closed files may not
-  /// be delivered, even if requested with WantDiags::Auto or WantDiags::Yes.
+  /// be delivered, even if requested with WantDiags::Auto.
   /// An empty set of diagnostics will be delivered, with Version = "".
   void removeDocument(PathRef File);
 
   /// Run code completion for \p File at \p Pos.
   /// Request is processed asynchronously.
   ///
   /// This method should only be called for currently tracked files. However, it
   /// is safe to call removeDocument for \p File after this method returns, even

clang-tools-extra/clangd/Protocol.h

struct DidChangeTextDocumentParams {
   /// The document that did change. The version number points
   /// to the version after all provided content changes have
   /// been applied.
   VersionedTextDocumentIdentifier textDocument;
 
   /// The actual content changes.
   std::vector<TextDocumentContentChangeEvent> contentChanges;
 
-  /// Forces diagnostics to be generated, or to not be generated, for this
-  /// version of the file. If not set, diagnostics are eventually consistent:
+  /// Can be used to disalbe diagnostic generation for this version of the file.
+  /// If not provided or set to true, diagnostics are eventually consistent:
   /// either they will be provided for this version or some subsequent one.
   /// This is a clangd extension.
   llvm::Optional<bool> wantDiagnostics;
 
   /// Force a complete rebuild of the file, ignoring all cached state. Slow!
   /// This is useful to defeat clangd's assumption that missing headers will
   /// stay missing.
   /// This is a clangd extension.

clang-tools-extra/clangd/TUScheduler.h

struct InputsAndPreamble {
   llvm::StringRef Contents;
   const tooling::CompileCommand &Command;
   // This can be nullptr if no preamble is available.
   const PreambleData *Preamble;
 };
 
 /// Determines whether diagnostics should be generated for a file snapshot.
 enum class WantDiagnostics {
-  Yes,  /// Diagnostics must be generated for this snapshot.
   No,   /// Diagnostics must not be generated for this snapshot.
   Auto, /// Diagnostics must be generated for this snapshot or a subsequent one,
         /// within a bounded amount of time.
 };
 
 /// Configuration of the AST retention policy. This only covers retention of
 /// *idle* ASTs. If queue has operations requiring the AST, they might be
 /// kept in memory.

clang-tools-extra/clangd/TUScheduler.cpp

 // diagnostics for update requests. Read requests will be served with ASTs build
 // with stale preambles, unless the read is picky and requires a compatible
 // preamble. In such cases it will block until new preamble is built.
 //
 // ASTWorker owns a PreambleThread for building preambles. If the preamble gets
 // invalidated by an update request, a new build will be requested on
 // PreambleThread. Since PreambleThread only receives requests for newer
 // versions of the file, in case of multiple requests it will only build the
-// last one and skip requests in between. Unless client force requested
-// diagnostics(WantDiagnostics::Yes).
+// last one and skip requests in between.
 //
 // When a new preamble is built, a "golden" AST is immediately built from that
 // version of the file. This ensures diagnostics get updated even if the queue
 // is full.
 //
 // Some read requests might just need preamble. Since preambles can be read
 // concurrently, ASTWorker runs these requests on their own thread. These
 // requests will receive latest build preamble, which might possibly be stale.
void update(std::unique_ptr<CompilerInvocation> CI, ParseInputs PI,
     if (RunSync) {
       build(std::move(Req));
       Status.update([](TUStatus &Status) {
         Status.PreambleActivity = PreambleAction::Idle;
       });
       return;
     }
     {
-      std::unique_lock<std::mutex> Lock(Mutex);
-      // If NextReq was requested with WantDiagnostics::Yes we cannot just drop
-      // that on the floor. Block until we start building it. This won't
-      // dead-lock as we are blocking the caller thread, while builds continue
-      // on preamble thread.
-      ReqCV.wait(Lock, [this] {
-        return !NextReq || NextReq->WantDiags != WantDiagnostics::Yes;
-      });
+      std::lock_guard<std::mutex> Lock(Mutex);
       NextReq = std::move(Req);
     }
     // Let the worker thread know there's a request, notify_one is safe as there
     // should be a single worker thread waiting on it.
     ReqCV.notify_all();
   }
 
   void run() {
for (auto I = Requests.begin(), E = Requests.end(); I != E; ++I) {
     }
     // Cancelled reads are moved to the front of the queue and run immediately.
     if (I->UpdateType == None) {
       Request R = std::move(*I);
       Requests.erase(I);
       Requests.push_front(std::move(R));
       return Deadline::zero();
     }
-    // Cancelled updates are downgraded to auto-diagnostics, and may be elided.
-    if (I->UpdateType == WantDiagnostics::Yes)
-      I->UpdateType = WantDiagnostics::Auto;
   }
 
   while (shouldSkipHeadLocked()) {
     vlog("ASTWorker skipping {0} for {1}", Requests.front().Name, FileName);
     Requests.pop_front();
   }
   assert(!Requests.empty() && "skipped the whole queue");
   // Some updates aren't dead yet, but never end up being used.
   // e.g. the first keystroke is live until obsoleted by the second.
-  // We debounce "maybe-unused" writes, sleeping in case they become dead.
-  // But don't delay reads (including updates where diagnostics are needed).
-  for (const auto &R : Requests)
-    if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes)
+  // We debounce "maybe-unused" writes, sleeping in case they become dead. But
+  // don't delay reads.
+  for (const auto &R : Requests) {
+    if (R.UpdateType == llvm::None)
       return Deadline::zero();
+  }
   // Front request needs to be debounced, so determine when we're ready.
   Deadline D(Requests.front().AddTime + UpdateDebounce.compute(RebuildTimes));
   return D;
 }
 
 // Returns true if Requests.front() is a dead update that can be skipped.
 bool ASTWorker::shouldSkipHeadLocked() const {
   assert(!Requests.empty());
   auto Next = Requests.begin();
   auto UpdateType = Next->UpdateType;
   if (!UpdateType) // Only skip updates.
     return false;
   ++Next;
   // An update is live if its AST might still be read.
   // That is, if it's not immediately followed by another update.
   if (Next == Requests.end() || !Next->UpdateType)
     return false;
   // The other way an update can be live is if its diagnostics might be used.
   switch (*UpdateType) {
-  case WantDiagnostics::Yes:
-    return false; // Always used.
   case WantDiagnostics::No:
     return true; // Always dead.
   case WantDiagnostics::Auto:
     // Used unless followed by an update that generates diagnostics.
-    for (; Next != Requests.end(); ++Next)
-      if (Next->UpdateType == WantDiagnostics::Yes ||
-          Next->UpdateType == WantDiagnostics::Auto)
+    for (; Next != Requests.end(); ++Next) {
+      if (Next->UpdateType == WantDiagnostics::Auto)
         return true; // Prefer later diagnostics.
+    }
     return false;
   }
   llvm_unreachable("Unknown WantDiagnostics");
 }
 
 bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
   auto WaitUntilASTWorkerIsIdle = [&] {
     std::unique_lock<std::mutex> Lock(Mutex);

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

TEST(CompletionTest, DocumentationFromChangedFileCrash) {
 
   Annotations Source(R"cpp(
     #include "foo.h"
     int func() {
       // This makes sure we have func from header in the AST.
     }
     int a = fun^
   )cpp");
-  Server.addDocument(FooCpp, Source.code(), "null", WantDiagnostics::Yes);
+  Server.addDocument(FooCpp, Source.code(), "null");
   // We need to wait for preamble to build.
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   // Change the header file. Completion will reuse the old preamble!
   FS.Files[FooH] = R"cpp(
     int func();
   )cpp";
 

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

 }
 
 TEST_F(TUSchedulerTests, WantDiagnostics) {
   std::atomic<int> CallbackCount(0);
   {
     // To avoid a racy test, don't allow tasks to actually run on the worker
     // thread until we've scheduled them all.
     Notification Ready;
+    Notification ScheduledFirstUpdate;
     TUScheduler S(CDB, optsForTest(), captureDiags());
     auto Path = testPath("foo.cpp");
-    updateWithDiags(S, Path, "", WantDiagnostics::Yes,
-                    [&](std::vector<Diag>) { Ready.wait(); });
-    updateWithDiags(S, Path, "request diags", WantDiagnostics::Yes,
-                    [&](std::vector<Diag>) { ++CallbackCount; });
+    updateWithDiags(S, Path, "", WantDiagnostics::Auto, [&](std::vector<Diag>) {
+      ScheduledFirstUpdate.notify();
+      Ready.wait();
+    });
+    ScheduledFirstUpdate.wait();
     updateWithDiags(S, Path, "auto (clobbered)", WantDiagnostics::Auto,
                     [&](std::vector<Diag>) {
                       ADD_FAILURE()
                           << "auto should have been cancelled by auto";
                     });
     updateWithDiags(S, Path, "request no diags", WantDiagnostics::No,
                     [&](std::vector<Diag>) {
                       ADD_FAILURE() << "no diags should not be called back";
                     });
     updateWithDiags(S, Path, "auto (produces)", WantDiagnostics::Auto,
                     [&](std::vector<Diag>) { ++CallbackCount; });
     Ready.notify();
 
     ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   }
-  EXPECT_EQ(2, CallbackCount);
+  EXPECT_EQ(1, CallbackCount);
 }
 
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic<int> CallbackCount(0);
   {
     auto Opts = optsForTest();
     Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1));
     TUScheduler S(CDB, Opts, captureDiags());
TEST_F(TUSchedulerTests, Cancellation) {
   //   U0
   //   U1(WantDiags=Yes) <-- cancelled
   //    R1               <-- cancelled
   //   U2(WantDiags=Yes) <-- cancelled
   //    R2A              <-- cancelled
   //    R2B
   //   U3(WantDiags=Yes)
   //    R3               <-- cancelled
-  std::vector<std::string> DiagsSeen, ReadsSeen, ReadsCanceled;
+  std::vector<std::string> DiagsSeen, ReadsSeen, ReadsCancelled;
   {
     Notification Proceed; // Ensure we schedule everything.
     TUScheduler S(CDB, optsForTest(), captureDiags());
     auto Path = testPath("foo.cpp");
     // Helper to schedule a named update and return a function to cancel it.
     auto Update = [&](std::string ID) -> Canceler {
       auto T = cancelableTask();
       WithContext C(std::move(T.first));
       updateWithDiags(
-          S, Path, "//" + ID, WantDiagnostics::Yes,
+          S, Path, "//" + ID, WantDiagnostics::Auto,
           [&, ID](std::vector<Diag> Diags) { DiagsSeen.push_back(ID); });
       return std::move(T.second);
     };
     // Helper to schedule a named read and return a function to cancel it.
     auto Read = [&](std::string ID) -> Canceler {
       auto T = cancelableTask();
       WithContext C(std::move(T.first));
       S.runWithAST(ID, Path, [&, ID](llvm::Expected<InputsAndAST> E) {
         if (auto Err = E.takeError()) {
           if (Err.isA<CancelledError>()) {
-            ReadsCanceled.push_back(ID);
+            ReadsCancelled.push_back(ID);
             consumeError(std::move(Err));
           } else {
             ADD_FAILURE() << "Non-cancelled error for " << ID << ": "
                           << llvm::toString(std::move(Err));
           }
         } else {
           ReadsSeen.push_back(ID);
         }
       });
       return std::move(T.second);
     };
 
-    updateWithCallback(S, Path, "", WantDiagnostics::Yes,
-                       [&]() { Proceed.wait(); });
+    Notification ScheduledUpdate;
+    updateWithCallback(S, Path, "", WantDiagnostics::Auto, [&]() {
+      ScheduledUpdate.notify();
+      Proceed.wait();
+    });
+    ScheduledUpdate.wait();
     // The second parens indicate cancellation, where present.
     Update("U1")();
     Read("R1")();
     Update("U2")();
     Read("R2A")();
     Read("R2B");
     Update("U3");
     Read("R3")();
     Proceed.notify();
 
     ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   }
   EXPECT_THAT(DiagsSeen, ElementsAre("U2", "U3"))
       << "U1 and all dependent reads were cancelled. "
-         "U2 has a dependent read R2A. "
+         "U2 has a dependent read R2B. "
          "U3 was not cancelled.";
   EXPECT_THAT(ReadsSeen, ElementsAre("R2B"))
       << "All reads other than R2B were cancelled";
-  EXPECT_THAT(ReadsCanceled, ElementsAre("R1", "R2A", "R3"))
+  EXPECT_THAT(ReadsCancelled, ElementsAre("R1", "R2A", "R3"))
       << "All reads other than R2B were cancelled";
 }
 
 TEST_F(TUSchedulerTests, InvalidationNoCrash) {
   auto Path = testPath("foo.cpp");
   TUScheduler S(CDB, optsForTest(), captureDiags());
 
   Notification StartedRunning;

 }
 
 TEST_F(TUSchedulerTests, Invalidation) {
   auto Path = testPath("foo.cpp");
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::atomic<int> Builds(0), Actions(0);
 
   Notification Start;
-  updateWithDiags(S, Path, "a", WantDiagnostics::Yes, [&](std::vector<Diag>) {
+  updateWithDiags(S, Path, "a", WantDiagnostics::Auto, [&](std::vector<Diag>) {
     ++Builds;
     Start.wait();
   });
   S.runWithAST(
       "invalidatable", Path,
       [&](llvm::Expected<InputsAndAST> AST) {
         ++Actions;
         EXPECT_FALSE(bool(AST));
TEST_F(TUSchedulerTests, EvictedAST) {
   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,
+  updateWithCallback(S, Foo, SourceContents, WantDiagnostics::Auto,
                      [&BuiltASTCounter]() { ++BuiltASTCounter; });
   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,
+  updateWithCallback(S, Bar, SourceContents, WantDiagnostics::Auto,
                      [&BuiltASTCounter]() { ++BuiltASTCounter; });
-  updateWithCallback(S, Baz, SourceContents, WantDiagnostics::Yes,
+  updateWithCallback(S, Baz, SourceContents, WantDiagnostics::Auto,
                      [&BuiltASTCounter]() { ++BuiltASTCounter; });
   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));
 
   // Check only the last two ASTs are retained.
   ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz));
 
   // Access the old file again.
-  updateWithCallback(S, Foo, OtherSourceContents, WantDiagnostics::Yes,
+  updateWithCallback(S, Foo, OtherSourceContents, WantDiagnostics::Auto,
                      [&BuiltASTCounter]() { ++BuiltASTCounter; });
   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.
std::string SourceContents = R"cpp(
       #include "foo.h"
       int b = a;
     )cpp";
 
   // Return value indicates if the updated callback was received.
   auto DoUpdate = [&](std::string Contents) -> bool {
     std::atomic<bool> Updated(false);
     Updated = false;
-    updateWithDiags(S, Source, Contents, WantDiagnostics::Yes,
+    updateWithDiags(S, Source, Contents, WantDiagnostics::Auto,
                     [&Updated](std::vector<Diag>) { Updated = true; });
     bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(10));
     if (!UpdateFinished)
       ADD_FAILURE() << "Updated has not finished in one second. Threading bug?";
     return Updated;
   };
 
   // Test that subsequent updates with the same inputs do not cause rebuilds.
auto SourceContents = R"cpp(
     )cpp";
 
   ParseInputs Inputs = getInputs(Source, SourceContents);
   std::atomic<size_t> DiagCount(0);
 
   // Update the source contents, which should trigger an initial build with
   // the header file missing.
   updateWithDiags(
-      S, Source, Inputs, WantDiagnostics::Yes,
+      S, Source, Inputs, WantDiagnostics::Auto,
       [&DiagCount](std::vector<Diag> Diags) {
         ++DiagCount;
         EXPECT_THAT(Diags,
                     ElementsAre(Field(&Diag::Message, "'foo.h' file not found"),
                                 Field(&Diag::Message,
                                       "use of undeclared identifier 'b'")));
       });
   S.blockUntilIdle(timeoutSeconds(10));
 
   FSProvider.Files[HeaderB] = "int b;";
   FSProvider.Timestamps[HeaderB] = time_t(1);
 
   // The addition of the missing header file triggers a rebuild, no errors.
-  updateWithDiags(
-      S, Source, Inputs, WantDiagnostics::Yes,
+  updateWithDiags(S, Source, Inputs, WantDiagnostics::Auto,
                   [&DiagCount](std::vector<Diag> Diags) {
                     ++DiagCount;
                     EXPECT_THAT(Diags, IsEmpty());
                   });
 
   // Ensure previous assertions are done before we touch the FS again.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   // Add the high-priority header file, which should reintroduce the error.
   FSProvider.Files[HeaderA] = "int a;";
   FSProvider.Timestamps[HeaderA] = time_t(1);
 
   // This isn't detected: we don't stat a/foo.h to validate the preamble.
-  updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+  updateWithDiags(S, Source, Inputs, WantDiagnostics::Auto,
                   [&DiagCount](std::vector<Diag> Diags) {
                     ++DiagCount;
                     ADD_FAILURE()
                         << "Didn't expect new diagnostics when adding a/foo.h";
                   });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
 
   // Forcing the reload should should cause a rebuild.
   Inputs.ForceRebuild = true;
-  updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
-                  [&DiagCount](std::vector<Diag> Diags) {
-                    ++DiagCount;
-                    ElementsAre(Field(&Diag::Message,
-                                      "use of undeclared identifier 'b'"));
-                  });
+  updateWithDiags(
+      S, Source, Inputs, WantDiagnostics::Auto,
+        ElementsAre(Field(&Diag::Message, "use of undeclared identifier 'b'"));
 
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
   EXPECT_EQ(DiagCount, 3U);
 }
 
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   trace::TestTracer Tracer;
   TUScheduler S(CDB, optsForTest(), captureDiags());
TEST_F(TUSchedulerTests, CommandLineErrors) {
   CDB.ExtraClangFlags = {"-fsome-unknown-flag"};
 
   // (!) 'Ready' must live longer than TUScheduler.
   Notification Ready;
 
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector<Diag> Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-                  WantDiagnostics::Yes, [&](std::vector<Diag> D) {
+                  WantDiagnostics::Auto, [&](std::vector<Diag> D) {
                     Diagnostics = std::move(D);
                     Ready.notify();
                   });
   Ready.wait();
 
   EXPECT_THAT(
       Diagnostics,
       ElementsAre(AllOf(
           Field(&Diag::ID, Eq(diag::err_drv_unknown_argument)),
           Field(&Diag::Name, Eq("drv_unknown_argument")),
           Field(&Diag::Message, "unknown argument: '-fsome-unknown-flag'"))));
 }
 
 TEST_F(TUSchedulerTests, CommandLineWarnings) {
   // We should not see warnings from command-line parsing.
   CDB.ExtraClangFlags = {"-Wsome-unknown-warning"};
 
   // (!) 'Ready' must live longer than TUScheduler.
   Notification Ready;
 
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector<Diag> Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-                  WantDiagnostics::Yes, [&](std::vector<Diag> D) {
+                  WantDiagnostics::Auto, [&](std::vector<Diag> D) {
                     Diagnostics = std::move(D);
                     Ready.notify();
                   });
   Ready.wait();
 
   EXPECT_THAT(Diagnostics, IsEmpty());
 }
 

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

TEST(LocateSymbol, WithPreamble) {
   EXPECT_THAT(
       cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
       ElementsAre(Sym("foo", FooWithoutHeader.range())));
 
   // Reset test environment.
   runAddDocument(Server, FooCpp, FooWithHeader.code());
   // Both preamble and AST are built in this request.
   Server.addDocument(FooCpp, FooWithoutHeader.code(), "null",
-                     WantDiagnostics::Yes);
+                     WantDiagnostics::Auto);
   // Use the AST being built in above request.
   EXPECT_THAT(
       cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
       ElementsAre(Sym("foo", FooWithoutHeader.range())));
 }
 
 TEST(LocateSymbol, NearbyTokenSmoke) {
   auto T = Annotations(R"cpp(