diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -12,6 +12,7 @@
 #include "../clang-tidy/ClangTidyOptions.h"
 #include "CodeComplete.h"
 #include "ConfigProvider.h"
+#include "Diagnostics.h"
 #include "DraftStore.h"
 #include "FeatureModule.h"
 #include "GlobalCompilationDatabase.h"
@@ -40,6 +41,7 @@
 #include <string>
 #include <type_traits>
 #include <utility>
+#include <vector>
 
 namespace clang {
 namespace clangd {
@@ -64,6 +66,8 @@
     virtual ~Callbacks() = default;
 
     /// Called by ClangdServer when \p Diagnostics for \p File are ready.
+    /// These pushed diagnostics might correspond to an older version of the
+    /// file, they do not interfere with "pull-based" ClangdServer::diagnostics.
     /// May be called concurrently for separate files, not for a single file.
     virtual void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
                                     std::vector<Diag> Diagnostics) {}
@@ -345,6 +349,14 @@
   void customAction(PathRef File, llvm::StringRef Name,
                     Callback<InputsAndAST> Action);
 
+  /// Fetches diagnostics for current version of the \p File. This might fail if
+  /// server is busy (building a preamble) and would require a long time to
+  /// prepare diagnostics. If it fails, clients should wait for
+  /// onSemanticsMaybeChanged and then retry.
+  /// These 'pulled' diagnostics do not interfere with the diagnostics 'pushed'
+  /// to Callbacks::onDiagnosticsReady, and clients may use either or both.
+  void diagnostics(PathRef File, Callback<std::vector<Diag>> CB);
+
   /// Returns estimated memory usage and other statistics for each of the
   /// currently open files.
   /// Overall memory usage of clangd may be significantly more than reported
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -9,6 +9,7 @@
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "DumpAST.h"
 #include "FindSymbols.h"
 #include "Format.h"
@@ -81,7 +82,9 @@
     if (FIndex)
       FIndex->updateMain(Path, AST);
 
-    std::vector<Diag> Diagnostics = AST.getDiagnostics();
+    assert(AST.getDiagnostics().hasValue() &&
+           "We issue callback only with fresh preambles");
+    std::vector<Diag> Diagnostics = *AST.getDiagnostics();
     if (ServerCallbacks)
       Publish([&]() {
         ServerCallbacks->onDiagnosticsReady(Path, AST.version(),
@@ -902,6 +905,21 @@
   WorkScheduler->runWithAST(Name, File, std::move(Action));
 }
 
+void ClangdServer::diagnostics(PathRef File, Callback<std::vector<Diag>> CB) {
+  auto Action =
+      [CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
+        if (!InpAST)
+          return CB(InpAST.takeError());
+        if (auto Diags = InpAST->AST.getDiagnostics())
+          return CB(*Diags);
+        // FIXME: Use ServerCancelled error once it is settled in LSP-3.17.
+        return CB(llvm::make_error<LSPError>("server is busy parsing includes",
+                                             ErrorCode::InternalError));
+      };
+
+  WorkScheduler->runWithAST("Diagnostics", File, std::move(Action));
+}
+
 llvm::StringMap<TUScheduler::FileStats> ClangdServer::fileStats() const {
   return WorkScheduler->fileStats();
 }
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
@@ -88,7 +88,9 @@
   /// (These should be const, but RecursiveASTVisitor requires Decl*).
   ArrayRef<Decl *> getLocalTopLevelDecls();
 
-  const std::vector<Diag> &getDiagnostics() const;
+  const llvm::Optional<std::vector<Diag>> &getDiagnostics() const {
+    return Diags;
+  }
 
   /// Returns the estimated size of the AST and the accessory structures, in
   /// bytes. Does not include the size of the preamble.
@@ -120,7 +122,7 @@
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens,
             MainFileMacros Macros, std::vector<Decl *> LocalTopLevelDecls,
-            std::vector<Diag> Diags, IncludeStructure Includes,
+            llvm::Optional<std::vector<Diag>> Diags, IncludeStructure Includes,
             CanonicalIncludes CanonIncludes);
 
   std::string Version;
@@ -142,8 +144,8 @@
 
   /// All macro definitions and expansions in the main file.
   MainFileMacros Macros;
-  // Data, stored after parsing.
-  std::vector<Diag> Diags;
+  // Data, stored after parsing. None if AST was built with a stale preamble.
+  llvm::Optional<std::vector<Diag>> Diags;
   // Top-level decls inside the current file. Not that this does not include
   // top-level decls from the preamble.
   std::vector<Decl *> LocalTopLevelDecls;
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
@@ -264,9 +264,11 @@
   StoreDiags ASTDiags;
 
   llvm::Optional<PreamblePatch> Patch;
+  bool PreserveDiags = true;
   if (Preamble) {
     Patch = PreamblePatch::create(Filename, Inputs, *Preamble);
     Patch->apply(*CI);
+    PreserveDiags = Patch->preserveDiagnostics();
   }
   auto Clang = prepareCompilerInstance(
       std::move(CI), PreamblePCH,
@@ -441,14 +443,20 @@
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
 
-  std::vector<Diag> 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<Diag> D = ASTDiags.take(CTContext.getPointer());
-    Diags.insert(Diags.end(), D.begin(), D.end());
+  llvm::Optional<std::vector<Diag>> Diags;
+  // FIXME: Also skip generation of diagnostics alltogether to speed up ast
+  // builds when we are patching a stale preamble.
+  if (PreserveDiags) {
+    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<Diag> 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),
@@ -493,14 +501,12 @@
 
 const MainFileMacros &ParsedAST::getMacros() const { return Macros; }
 
-const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; }
-
 std::size_t ParsedAST::getUsedBytes() const {
   auto &AST = getASTContext();
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
   // Message and Fixes inside each diagnostic.
-  std::size_t Total =
-      clangd::getUsedBytes(LocalTopLevelDecls) + clangd::getUsedBytes(Diags);
+  std::size_t Total = clangd::getUsedBytes(LocalTopLevelDecls) +
+                      (Diags ? clangd::getUsedBytes(*Diags) : 0);
 
   // FIXME: the rest of the function is almost a direct copy-paste from
   // libclang's clang_getCXTUResourceUsage. We could share the implementation.
@@ -541,8 +547,8 @@
                      std::unique_ptr<FrontendAction> Action,
                      syntax::TokenBuffer Tokens, MainFileMacros Macros,
                      std::vector<Decl *> LocalTopLevelDecls,
-                     std::vector<Diag> Diags, IncludeStructure Includes,
-                     CanonicalIncludes CanonIncludes)
+                     llvm::Optional<std::vector<Diag>> Diags,
+                     IncludeStructure Includes, CanonicalIncludes CanonIncludes)
     : Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Tokens(std::move(Tokens)),
       Macros(std::move(Macros)), Diags(std::move(Diags)),
diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h
--- a/clang-tools-extra/clangd/Preamble.h
+++ b/clang-tools-extra/clangd/Preamble.h
@@ -126,6 +126,9 @@
   /// Returns textual patch contents.
   llvm::StringRef text() const { return PatchContents; }
 
+  /// Whether diagnostics generated using this patch are trustable.
+  bool preserveDiagnostics() const { return PatchContents.empty(); }
+
 private:
   PreamblePatch() = default;
   std::string PatchContents;
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -181,7 +181,7 @@
       elog("Failed to build AST");
       return false;
     }
-    ErrCount += showErrors(llvm::makeArrayRef(AST->getDiagnostics())
+    ErrCount += showErrors(llvm::makeArrayRef(*AST->getDiagnostics())
                                .drop_front(Preamble->Diags.size()));
 
     if (Opts.BuildDynamicSymbolIndex) {
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -135,7 +135,7 @@
   auto TU = TestTU::withCode(Test.code());
   TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
@@ -173,14 +173,14 @@
 TEST(DiagnosticsTest, FlagsMatter) {
   Annotations Test("[[void]] main() {} // error-ok");
   auto TU = TestTU::withCode(Test.code());
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 WithFix(Fix(Test.range(), "int",
                                             "change 'void' to 'int'")))));
   // Same code built as C gets different diagnostics.
   TU.Filename = "Plain.c";
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(AllOf(
           Diag(Test.range(), "return type of 'main' is not 'int'"),
           WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
@@ -192,7 +192,7 @@
   )cpp");
 
   auto TU = TestTU::withCode(Test.code());
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(::testing::AllOf(
                   Diag(Test.range(), "'not-found.h' file not found"),
                   DiagSource(Diag::Clang), DiagName("pp_file_not_found"))));
@@ -209,7 +209,7 @@
                                        "hicpp-uppercase-literal-suffix");
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Test.range(),
                "floating point literal has suffix 'f', which is not uppercase"),
@@ -229,7 +229,7 @@
   // The check doesn't handle template instantiations which ends up emitting
   // duplicated messages, verify that we deduplicate them.
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Test.range(),
                "floating point literal has suffix 'f', which is not uppercase"),
@@ -254,7 +254,7 @@
                                        "modernize-deprecated-headers,"
                                        "modernize-use-trailing-return-type");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(
           AllOf(Diag(Test.range("deprecated"),
                      "inclusion of deprecated C++ header 'assert.h'; consider "
@@ -296,7 +296,7 @@
   TU.AdditionalFiles["a.h"] = TU.AdditionalFiles["b.h"] = "";
   TU.ClangTidyProvider = addTidyChecks("llvm-include-order");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       Contains(AllOf(Diag(Test.range(), "#includes are not sorted properly"),
                      DiagSource(Diag::ClangTidy),
                      DiagName("llvm-include-order"))));
@@ -314,7 +314,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.HeaderCode = Header.code().str();
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(AllOf(
           Diag(Main.range(), "in template: base specifier must name a class"),
           WithNote(Diag(Header.range(), "error occurred here"),
@@ -340,7 +340,7 @@
     }
     }
   )cpp";
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(),
                        "in template: "
@@ -368,7 +368,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("modernize-loop-convert");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "use range-based for loop instead"),
           DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"))));
@@ -384,14 +384,14 @@
   )cpp");
   auto TU = TestTU::withCode(Main.code());
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"),
                   Diag(Main.range("ret"),
                        "void function 'x' should not return a value")));
   Config Cfg;
   Cfg.Diagnostics.Suppress.insert("return-type");
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(Diag(Main.range(),
                                "use of undeclared identifier 'unknown'")));
 }
@@ -413,7 +413,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("bugprone-integer-division");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "result of integer division used in a floating "
                              "point context; possible loss of precision"),
@@ -431,7 +431,7 @@
   TU.ClangTidyProvider =
       addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(::testing::AllOf(
           Diag(Main.range(), "result of integer division used in a floating "
                              "point context; possible loss of precision"),
@@ -450,7 +450,7 @@
   )cpp");
   TestTU TU = TestTU::withCode(Source.code());
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(WithFix(Fix(
           Source.range(),
           "somereallyreallyreallyreallyreallyreallyreallyreallylongidentifier",
@@ -466,7 +466,7 @@
     }
   )cpp");
   TU.Code = std::string(Source.code());
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(WithFix(
                   Fix(Source.range(), "ident", "change 'ide\\…' to 'ident'"))));
 }
@@ -481,7 +481,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider =
       addTidyChecks("bugprone-integer-division", "bugprone-integer-division");
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
 TEST(DiagnosticTest, ClangTidyNoLiteralDataInMacroToken) {
@@ -496,7 +496,7 @@
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("bugprone-bad-signal-to-kill-thread");
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+  EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
 }
 
 TEST(DiagnosticTest, ElseAfterReturnRange) {
@@ -513,7 +513,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.ClangTidyProvider = addTidyChecks("llvm-else-after-return");
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(Diag(Main.range(), "do not use 'else' after 'return'")));
 }
 
@@ -532,7 +532,7 @@
     #endif
     )cpp");
   EXPECT_THAT(
-      TestTU::withCode(Test.code()).build().getDiagnostics(),
+      *TestTU::withCode(Test.code()).build().getDiagnostics(),
       ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
@@ -542,7 +542,7 @@
   )cpp");
   TU.ExtraArgs.push_back("-Xclang");
   TU.ExtraArgs.push_back("-verify");
-  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
 }
 
 // Recursive main-file include is diagnosed, and doesn't crash.
@@ -552,7 +552,7 @@
     int symbol;
   )cpp");
   TU.Filename = "foo.h";
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
   EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
 }
@@ -565,7 +565,7 @@
     int symbol;
   )cpp");
   TU.Filename = "foo.h";
-  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
   EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
 }
 
@@ -581,7 +581,7 @@
   )cpp");
   TU.Filename = "foo.h";
   // FIXME: should be no errors here.
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
   EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
 }
@@ -598,7 +598,7 @@
       return $bar[[TEN]];
     }
     )cpp");
-  EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
+  EXPECT_THAT(*TestTU::withCode(Test.code()).build().getDiagnostics(),
               ElementsAre(Diag(Test.range("foo"),
                                "cannot initialize return object of type "
                                "'int *' with an rvalue of type 'int'"),
@@ -614,7 +614,7 @@
     [[Define]](main) // error-ok
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 Not(WithFix(_)))));
 }
@@ -625,7 +625,7 @@
   llvm::InitializeAllTargetInfos(); // As in ClangdMain
   auto TU = TestTU::withCode("void fn() { __asm { cmp cl,64 } }");
   TU.ExtraArgs = {"-fms-extensions"};
-  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
 }
 
 TEST(DiagnosticsTest, ToLSP) {
@@ -783,7 +783,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAreArray(
           {AllOf(Diag(Test.range("nested"),
                       "incomplete type 'ns::X' named in nested name specifier"),
@@ -868,7 +868,7 @@
       MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab());
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Test.range("base"), "base class has incomplete type"),
                   Diag(Test.range("access"),
@@ -901,7 +901,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(
           AllOf(Diag(Test.range("unqualified1"), "unknown type name 'X'"),
                 DiagName("unknown_typename"),
@@ -946,7 +946,7 @@
        SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
                   Diag(Test.range("unqualified"), "unknown type name 'X'"),
                   DiagName("unknown_typename"),
@@ -967,7 +967,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
 }
 
@@ -1002,7 +1002,7 @@
   TU.ExternalIndex = Index.get();
 
   auto Parsed = TU.build();
-  for (const auto &D : Parsed.getDiagnostics()) {
+  for (const auto &D : *Parsed.getDiagnostics()) {
     if (D.Fixes.size() != 1) {
       ADD_FAILURE() << "D.Fixes.size() != 1";
       continue;
@@ -1027,7 +1027,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(AllOf(
           Diag(Test.range(), "no member named 'scope' in namespace 'ns'"),
           DiagName("no_member"),
@@ -1055,7 +1055,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       UnorderedElementsAre(
           AllOf(
               Diag(Test.range("q1"), "use of undeclared identifier 'clangd'; "
@@ -1098,7 +1098,7 @@
       SymbolWithHeader{"a::X", "unittest:///x.h", "\"x.h\""});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
                   Diag(Test.range(), "no type named 'X' in namespace 'a'"),
                   DiagName("typename_nested_not_found"),
@@ -1124,7 +1124,7 @@
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
-      TU.build().getDiagnostics(),
+      *TU.build().getDiagnostics(),
       ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
@@ -1135,7 +1135,7 @@
   Annotations Header("[[no_type_spec]]; // error-ok");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
                   Diag(Main.range(), "in included file: C++ requires a "
                                      "type specifier for all declarations"),
@@ -1149,7 +1149,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", "#include \"b.h\""},
                         {"b.h", "no_type_spec; // error-ok"}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: C++ requires a "
                                      "type specifier for all declarations")));
@@ -1163,7 +1163,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", "no_type_spec; // error-ok"},
                         {"b.h", "no_type_spec; // error-ok"}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range("a"), "in included file: C++ requires a type "
                                         "specifier for all declarations"),
@@ -1180,7 +1180,7 @@
   TU.AdditionalFiles = {
       {"a.h", "#include \"b.h\"\n"},
       {"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(Diag(Main.range(),
                                         "in included file: C++ requires a type "
                                         "specifier for all declarations")));
@@ -1198,7 +1198,7 @@
       {"a.h", "#include \"c.h\"\n"},
       {"b.h", "#include \"c.h\"\n"},
       {"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: C++ requires a "
                                      "type specifier for all declarations")));
@@ -1227,7 +1227,7 @@
       no_type_spec_9;
       no_type_spec_10;
       #endif)cpp"}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: C++ requires a "
                                      "type specifier for all declarations")));
@@ -1242,7 +1242,7 @@
     int x = 5/0;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
                   Diag(Main.range(), "in included file: C++ requires "
                                      "a type specifier for all declarations"),
@@ -1260,7 +1260,7 @@
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
   // promote warnings to errors.
   TU.ExtraArgs = {"-Werror", "-Wunused"};
-  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
 }
 
 TEST(DiagsInHeaders, FromNonWrittenSources) {
@@ -1273,7 +1273,7 @@
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
   TU.ExtraArgs = {"-DFOO=NOOO"};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
                   Diag(Main.range(),
                        "in included file: use of undeclared identifier 'NOOO'"),
@@ -1291,7 +1291,7 @@
   X;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: use of undeclared "
                                      "identifier 'foo'; did you mean 'fo'?")));
@@ -1308,7 +1308,7 @@
   X(foo);)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
-  EXPECT_THAT(TU.build().getDiagnostics(),
+  EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(), "in included file: use of undeclared "
                                      "identifier 'foo'; did you mean 'fo'?")));
@@ -1320,7 +1320,7 @@
   TU.AdditionalFiles = {{"a.h", "void main();"}};
   // The diagnostic "main must return int" is from the header, we don't attempt
   // to render it in the main file as there is no written location there.
-  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+  EXPECT_THAT(*TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
 TEST(ToLSPDiag, RangeIsInMain) {
diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
--- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -61,7 +61,7 @@
       header "module.h"
     }
 )modulemap";
-  EXPECT_TRUE(TU.build().getDiagnostics().empty());
+  EXPECT_TRUE(TU.build().getDiagnostics()->empty());
 }
 
 TEST(Modules, Diagnostic) {
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
@@ -511,7 +511,7 @@
     auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
                                        std::move(CI), {}, BaselinePreamble);
     ASSERT_TRUE(PatchedAST);
-    EXPECT_TRUE(PatchedAST->getDiagnostics().empty());
+    EXPECT_FALSE(PatchedAST->getDiagnostics());
   }
 
   // Then ensure correctness by making sure includes were seen only once.
@@ -526,7 +526,7 @@
   auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
                                      std::move(CI), {}, BaselinePreamble);
   ASSERT_TRUE(PatchedAST);
-  EXPECT_TRUE(PatchedAST->getDiagnostics().empty());
+  EXPECT_FALSE(PatchedAST->getDiagnostics());
   EXPECT_THAT(Includes,
               ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
                           WithFileName("b.h"), WithFileName("a.h")));
@@ -569,7 +569,7 @@
   auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
                                      {}, EmptyPreamble);
   ASSERT_TRUE(PatchedAST);
-  ASSERT_TRUE(PatchedAST->getDiagnostics().empty());
+  ASSERT_FALSE(PatchedAST->getDiagnostics());
 
   // Ensure source location information is correct, including resolved paths.
   EXPECT_THAT(PatchedAST->getIncludeStructure().MainFileIncludes,
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
@@ -274,8 +274,12 @@
 
     auto AST = createPatchedAST("", Modified.code());
     ASSERT_TRUE(AST);
-    EXPECT_THAT(AST->getDiagnostics(),
-                Not(Contains(Field(&Diag::Range, Modified.range()))));
+    std::vector<Range> MacroRefRanges;
+    for (auto &M : AST->getMacros().MacroRefs) {
+      for (auto &O : M.getSecond())
+        MacroRefRanges.push_back(O.Rng);
+    }
+    EXPECT_THAT(MacroRefRanges, Contains(Modified.range()));
   }
 }
 
@@ -298,8 +302,6 @@
 
   auto AST = createPatchedAST(Baseline, Modified.code());
   ASSERT_TRUE(AST);
-  EXPECT_THAT(AST->getDiagnostics(),
-              Not(Contains(Field(&Diag::Range, Modified.range()))));
 }
 
 TEST(PreamblePatchTest, LocateMacroAtWorks) {
@@ -535,6 +537,15 @@
               ExpectedBounds.PreambleEndsAtStartOfLine);
   }
 }
+
+TEST(PreamblePatch, DropsDiagnostics) {
+  llvm::StringLiteral Code = "#define FOO\nx;/* error-ok */";
+  // First check that this code generates diagnostics.
+  EXPECT_THAT(*TestTU::withCode(Code).build().getDiagnostics(),
+              testing::Not(testing::IsEmpty()));
+  // Ensure they are dropeed when a patched preamble is used.
+  EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -581,7 +581,7 @@
   auto TU = TestTU::withCode(Test.code());
   TU.AdditionalFiles["Expand.inc"] = "MACRO\n";
   auto AST = TU.build();
-  EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty());
+  EXPECT_THAT(*AST.getDiagnostics(), ::testing::IsEmpty());
   auto T = makeSelectionTree(Case, AST);
 
   EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());
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
@@ -121,7 +121,7 @@
     class CaptureDiags : public ParsingCallbacks {
     public:
       void onMainAST(PathRef File, ParsedAST &AST, PublishFn Publish) override {
-        reportDiagnostics(File, AST.getDiagnostics(), Publish);
+        reportDiagnostics(File, *AST.getDiagnostics(), Publish);
       }
 
       void onFailedAST(PathRef File, llvm::StringRef Version,
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -78,6 +78,7 @@
 
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
+  // The result will always have getDiagnostics() populated.
   ParsedAST build() const;
   std::shared_ptr<const PreambleData>
   preamble(PreambleParsedCallback PreambleCallback = nullptr) const;
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
@@ -113,6 +113,11 @@
     ADD_FAILURE() << "Failed to build code:\n" << Code;
     llvm_unreachable("Failed to build TestTU!");
   }
+  if (!AST->getDiagnostics()) {
+    ADD_FAILURE() << "TestTU should always build an AST with a fresh Preamble"
+                  << Code;
+    return std::move(*AST);
+  }
   // Check for error diagnostics and report gtest failures (unless expected).
   // This guards against accidental syntax errors silently subverting tests.
   // error-ok is awfully primitive - using clang -verify would be nicer.
@@ -128,7 +133,8 @@
     return false;
   }();
   if (!ErrorOk) {
-    for (const auto &D : AST->getDiagnostics())
+    // We always build AST with a fresh preamble in TestTU.
+    for (const auto &D : *AST->getDiagnostics())
       if (D.Severity >= DiagnosticsEngine::Error) {
         ADD_FAILURE()
             << "TestTU failed to build (suppress with /*error-ok*/): \n"
diff --git a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
--- a/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -398,7 +398,7 @@
 
   // The compiler should produce a diagnostic for hitting the
   // template instantiation depth.
-  ASSERT_TRUE(!AST.getDiagnostics().empty());
+  ASSERT_TRUE(!AST.getDiagnostics()->empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
   // The parent is reported as "S" because "S<0>" is an invalid instantiation.