[clangd] Surface errors from command-line parsing

[ilya-biryukov]

Summary

Those errors are exposed at the first character of a file,
for a lack of a better place.

Previously, all errors were stored inside the AST and report
accordingly. However, errors in command-line argument parsing could
result in failure to produce the AST, so we need an alternative ways to
report those errors.

We take the following approach in this patch:

• buildCompilerInvocation() now requires an explicit DiagnosticConsumer.
• TUScheduler and TestTU now collect the diagnostics produced when parsing command line arguments. If pasing of the AST failed, diagnostics are reported via a new ParsingCallbacks::onFailedAST method. If parsing of the AST succeeded, any errors produced during command-line parsing are stored alongside the AST inside the ParsedAST instance and reported as previously by calling the ParsingCallbacks::onMainAST method;
• The client code that uses ClangdServer's DiagnosticConsumer does not need to change, it will receive new diagnostics in the onDiagnosticsReady() callback

Errors produced when parsing command-line arguments are collected using
the same StoreDiags class that is used to collect all other errors. They
are recognized by their location being invalid. IIUC, the location is
invalid as there is no source manager at this point, it is created at a
later stage.

Although technically we might also get diagnostics that mention the
command-line arguments FileID with after the source manager was created
(and they have valid source locations), we choose to not handle those
and they are dropped as not coming from the main file. AFAICT, those
diagnostics should always be notes, therefore it's safe to drop them
without loosing too much information.

[kadircet]

Thanks! This looks really useful when we can't build an AST due to unknown compiler commands, but I am not sure about how useful it is to surface command-line parsing errors once we are able to build an AST.
Because people most likely won't care about these errors once clangd is working for them, and even get annoyed since most of the developers has little control over how their build system generates their compile
commands, therefore it will be really hard to get rid of those errors showing up in all the files they want to edit.

So I suggest only surfacing these if we are not able to create a compiler invocation or build an ast at all. WDYT?

[ilya-biryukov]

In D66759#1646509, @kadircet wrote:

Thanks! This looks really useful when we can't build an AST due to unknown compiler commands, but I am not sure about how useful it is to surface command-line parsing errors once we are able to build an AST.
Because people most likely won't care about these errors once clangd is working for them, and even get annoyed since most of the developers has little control over how their build system generates their compile
commands, therefore it will be really hard to get rid of those errors showing up in all the files they want to edit.

How common is this? Do we have any particular examples?

So I suggest only surfacing these if we are not able to create a compiler invocation or build an ast at all. WDYT?

My worry comes from the fact that I don't think we can guarantee that clangd is working for the users after we encountered those errors.
In particular, after D66731, we will be getting a compiler invocation and building the AST in many more cases. However, the AST might be completely bogus, e.g. because we did not recognize actually important flags.
Surfacing this error to the user is useful, because that would explain why clangd is rusty, gives weird results, etc.

In turn, the users will report bugs and we'll be able to collect data on whether this is noisy or everyone is doing compile_commands.json wrong and we should do something about it or whether it's actually very rare in practice and turns out to be a useful indicator for the users.
Either result seems plausible.
I'm all in for hiding annoying errors, but I don't know what the failure modes are in that particular case and I'd be tempted to first see whether surfacing the errors would actually end up producing those annoying errors and what the errors are.

[kadircet]

In D66759#1646599, @ilya-biryukov wrote:

How common is this? Do we have any particular examples?

Well, I don't have any data to prove, but my experience has been so far; most of the people don't touch Makefiles apart from adding their source files to the
CMakeLists within their project directory, and only some of them are comfortable with providing additional flags. For example having a new warning flag, or
a flag that has been recently renamed. Then your compiler won't complain during builds, but you will get those bogus errors in clangd.

My worry comes from the fact that I don't think we can guarantee that clangd is working for the users after we encountered those errors.
In particular, after D66731, we will be getting a compiler invocation and building the AST in many more cases. However, the AST might be completely bogus, e.g. because we did not recognize actually important flags.
Surfacing this error to the user is useful, because that would explain why clangd is rusty, gives weird results, etc.

In turn, the users will report bugs and we'll be able to collect data on whether this is noisy or everyone is doing compile_commands.json wrong and we should do something about it or whether it's actually very rare in practice and turns out to be a useful indicator for the users.
Either result seems plausible.
I'm all in for hiding annoying errors, but I don't know what the failure modes are in that particular case and I'd be tempted to first see whether surfacing the errors would actually end up producing those annoying errors and what the errors are.

OK, I was leaning on "not showing up if we could build an AST" side because I thought that in those cases even if we have errors while building CompilerInvocation
these would most of the time be some small things that would not affect how AST was generated in a sense that users would care. Therefore we would end up annoying
users most of the time and be really useful only in some small portion of the cases.

But again this isn't backed by any data, following your approach of turning it on and changing it later on if we annoy people also sounds OK to me. Also as discussed offline
I misunderstood the scope of the diags generated while building compiler invocations. Since this only surfaces errors, it should be mostly OK.

clang-tools-extra/clangd/ClangdUnit.cpp

465 ↗

(On Diff #217202)

could you preserve the previous order by inserting CompilerInvocationDiags here instead of the ones in AST ?

clang-tools-extra/clangd/Diagnostics.cpp

399 ↗

(On Diff #217202)

I suppose we can get rid of the one in StoreDiags::EndSourceFile now ?

[ilya-biryukov]

Just to clarify: we only want to surface errors, not warnings from command-line to avoid too much nosie; I'm totally on board with not spamming users with "unknown compiler warning" errors at the start of each file.
We would only show things like unknown argument "-mdisable-fp-elim". Given that clang understands most flags from other compilers (to be a good drop-in replacement), we should end up showing errors when things actually can be significantly broken.

And we will now show this errors only in cases when we could not build the AST at all before (as CompilerInvocation was null), so overall we should not be producing too much new noise

clang-tools-extra/clangd/ClangdUnit.cpp

465 ↗

(On Diff #217202)

Do you think we should add CompilerInvocatioDiags at the end, rather than at the start of the diagnostics list?
Why would that be better?

Mostly trying to keep the chronological order here: command-line diagnostics came first, followed by preamble, followed by AST diagnostics.

[kadircet]

clang-tools-extra/clangd/ClangdUnit.cpp

465 ↗

(On Diff #217202)

I totally agree that your current ordering makes more sense. I just wasn't sure why it was done in the opposite way before(first AST, then preamble), if it doesn't cause any test breakages we should be good though.

clang-tools-extra/clangd/Diagnostics.cpp

603 ↗

(On Diff #217202)

I suppose this one is also not required anymore, as you clear it during flush

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

755 ↗

(On Diff #217202)

as discussed offline, could you also add a test to make sure this does not fire on "-Wunknown-warning-flag" ?

[ilya-biryukov]

• Do not flushDiag() on EndSourceFile
• Do not reset WasAdjusted outside flusLastDiag()
• Add a test that unknown warnings do not produce diagnostics

[ilya-biryukov]

clang-tools-extra/clangd/ClangdUnit.cpp

465 ↗

(On Diff #217202)

The intention was to do less copies: normally there are less diagnostics in preamble than in the main file, so we would add the main file diagnostics first and later prepend the preamble diagnostics.

Performance-wise it does not matter and the new version reads better as it only appends and the order of items is natural.

clang-tools-extra/clangd/Diagnostics.cpp

399 ↗

(On Diff #217202)

I've removed the call to flushDiag() from the function, but kept the function to reset the LangOpts to None for symmetry with setting them on BeginSourceFile.
Don't think this matters in practice, looks a bit better aesthetically (we "clean up" when the file goes out of scope).

[ilya-biryukov]

• Remove accidental change

[kadircet]

LGTM, thanks!

clang-tools-extra/trunk/clangd/ClangdServer.cpp

void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
 
     Publish([&]() {
       DiagConsumer.onDiagnosticsReady(Path, std::move(Diagnostics));
       if (SemanticHighlighting)
         DiagConsumer.onHighlightingsReady(Path, std::move(Highlightings));
     });
   }
 
+  void onFailedAST(PathRef Path, std::vector<Diag> Diags,
+                   PublishFn Publish) override {
+    Publish([&]() { DiagConsumer.onDiagnosticsReady(Path, Diags); });
+  }
+
   void onFileUpdated(PathRef File, const TUStatus &Status) override {
     DiagConsumer.onFileUpdated(File, Status);
   }
 
 private:
   FileIndex *FIndex;
   DiagnosticsConsumer &DiagConsumer;
   bool SemanticHighlighting;

clang-tools-extra/trunk/clangd/ClangdUnit.h

 #include "index/Index.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 
 namespace vfs {

 
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
   /// it is reused during parsing.
   static llvm::Optional<ParsedAST>
   build(std::unique_ptr<clang::CompilerInvocation> CI,
+        llvm::ArrayRef<Diag> CompilerInvocationDiags,
         std::shared_ptr<const PreambleData> Preamble,
         std::unique_ptr<llvm::MemoryBuffer> Buffer,
-        IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, const SymbolIndex *Index,
-        const ParseOptions &Opts);
+        llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
+        const SymbolIndex *Index, const ParseOptions &Opts);
 
   ParsedAST(ParsedAST &&Other);
   ParsedAST &operator=(ParsedAST &&Other);
 
   ~ParsedAST();
 
   /// Note that the returned ast will not contain decls from the preamble that
   /// were not deserialized during parsing. Clients should expect only decls
buildPreamble(PathRef FileName, CompilerInvocation &CI,
               const ParseInputs &Inputs, bool StoreInMemory,
               PreambleParsedCallback PreambleCallback);
 
 /// Build an AST from provided user inputs. This function does not check if
 /// preamble can be reused, as this function expects that \p Preamble is the
 /// result of calling buildPreamble.
 llvm::Optional<ParsedAST>
 buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
+         llvm::ArrayRef<Diag> CompilerInvocationDiags,
          const ParseInputs &Inputs,
          std::shared_ptr<const PreambleData> Preamble);
 
 /// Get the beginning SourceLocation at a specified \p Pos.
 /// May be invalid if Pos is, or if there's no identifier.
 SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
                                         const Position &Pos, const FileID FID);
 
 /// For testing/debugging purposes. Note that this method deserializes all
 /// unserialized Decls, so use with care.
 void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS);
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H

clang-tools-extra/trunk/clangd/ClangdUnit.cpp

 
 } // namespace
 
 void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
   AST.getASTContext().getTranslationUnitDecl()->dump(OS, true);
 }
 
 llvm::Optional<ParsedAST>
-ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
+ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
+                 llvm::ArrayRef<Diag> CompilerInvocationDiags,
                  std::shared_ptr<const PreambleData> Preamble,
                  std::unique_ptr<llvm::MemoryBuffer> Buffer,
                  llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
                  const SymbolIndex *Index, const ParseOptions &Opts) {
   assert(CI);
   // Command-line parsing sets DisableFree to true by default, but we don't want
   // to leak memory in clangd.
   CI->getFrontendOpts().DisableFree = false;
ParsedAST::build(std::unique_ptr<clang::CompilerInvocation> CI,
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
   // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
   // However Action->EndSourceFile() would destroy the ASTContext!
   // So just inform the preprocessor of EOF, while keeping everything alive.
   Clang->getPreprocessor().EndSourceFile();
 
-  std::vector<Diag> Diags = ASTDiags.take(CTContext.getPointer());
+  std::vector<Diag> Diags = CompilerInvocationDiags;
   // Add diagnostics from the preamble, if any.
   if (Preamble)
-    Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
+    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(std::move(Preamble), std::move(Clang), std::move(Action),
                    std::move(Tokens), std::move(ParsedDecls), std::move(Diags),
                    std::move(Includes), std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
 
 ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
buildPreamble(PathRef FileName, CompilerInvocation &CI,
   } else {
     elog("Could not build a preamble for file {0}", FileName);
     return nullptr;
   }
 }
 
 llvm::Optional<ParsedAST>
 buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
+         llvm::ArrayRef<Diag> CompilerInvocationDiags,
          const ParseInputs &Inputs,
          std::shared_ptr<const PreambleData> Preamble) {
   trace::Span Tracer("BuildAST");
   SPAN_ATTACH(Tracer, "File", FileName);
 
   auto VFS = Inputs.FS;
   if (Preamble && Preamble->StatCache)
     VFS = Preamble->StatCache->getConsumingFS(std::move(VFS));
   if (VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
     log("Couldn't set working directory when building the preamble.");
     // We proceed anyway, our lit-tests rely on results for non-existing working
     // dirs.
   }
 
   return ParsedAST::build(
-      std::make_unique<CompilerInvocation>(*Invocation), Preamble,
+      std::make_unique<CompilerInvocation>(*Invocation),
+      CompilerInvocationDiags, Preamble,
       llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName),
       std::move(VFS), Inputs.Index, Inputs.Opts);
 }
 
 SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
                                         const Position &Pos, const FileID FID) {
   const ASTContext &AST = Unit.getASTContext();
   const SourceManager &SourceMgr = AST.getSourceManager();

clang-tools-extra/trunk/clangd/CodeComplete.cpp

bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = Input.VFS;
   if (Input.Preamble && Input.Preamble->StatCache)
     VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
   ParseInputs ParseInput;
   ParseInput.CompileCommand = Input.Command;
   ParseInput.FS = VFS;
   ParseInput.Contents = Input.Contents;
   ParseInput.Opts = ParseOptions();
-  auto CI = buildCompilerInvocation(ParseInput);
+
+  IgnoreDiagnostics IgnoreDiags;
+  auto CI = buildCompilerInvocation(ParseInput, IgnoreDiags);
   if (!CI) {
     elog("Couldn't create CompilerInvocation");
     return false;
   }
   auto &FrontendOpts = CI->getFrontendOpts();
   FrontendOpts.SkipFunctionBodies = true;
   // Disable typo correction in Sema.
   CI->getLangOpts()->SpellChecking = false;
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   // However, if we're completing *inside* the preamble section of the draft,
   // overriding the preamble will break sema completion. Fortunately we can just
   // skip all includes in this case; these completions are really simple.
   PreambleBounds PreambleRegion =
       ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
   bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
   // the remapped buffers do not get freed.
-  IgnoreDiagnostics DummyDiagsConsumer;
   auto Clang = prepareCompilerInstance(
       std::move(CI),
       (Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble
                                                 : nullptr,
-      std::move(ContentsBuffer), std::move(VFS), DummyDiagsConsumer);
+      std::move(ContentsBuffer), std::move(VFS), IgnoreDiags);
   Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
   Clang->setCodeCompletionConsumer(Consumer.release());
 
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     log("BeginSourceFile() failed when running codeComplete for {0}",
         Input.FileName);
     return false;

clang-tools-extra/trunk/clangd/Compiler.h

struct ParseInputs {
   std::string Contents;
   // Used to recover from diagnostics (e.g. find missing includes for symbol).
   const SymbolIndex *Index = nullptr;
   ParseOptions Opts;
 };
 
 /// Builds compiler invocation that could be used to build AST or preamble.
 std::unique_ptr<CompilerInvocation>
-buildCompilerInvocation(const ParseInputs &Inputs);
+buildCompilerInvocation(const ParseInputs &Inputs,
+                        clang::DiagnosticConsumer &D);
 
 /// Creates a compiler instance, configured so that:
 ///   - Contents of the parsed file are remapped to \p MainFile.
 ///   - Preamble is overriden to use PCH passed to this function. It means the
 ///     changes to the preamble headers or files included in the preamble are
 ///     not visible to this compiler instance.
 ///   - llvm::vfs::FileSystem is used for all underlying file accesses. The
 ///     actual vfs used by the compiler may be an overlay over the passed vfs.

clang-tools-extra/trunk/clangd/Compiler.cpp

 }
 
 void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                                          const clang::Diagnostic &Info) {
   IgnoreDiagnostics::log(DiagLevel, Info);
 }
 
 std::unique_ptr<CompilerInvocation>
-buildCompilerInvocation(const ParseInputs &Inputs) {
+buildCompilerInvocation(const ParseInputs &Inputs,
+                        clang::DiagnosticConsumer &D) {
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());
 
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
     log("Couldn't set working directory when creating compiler invocation.");
     // We proceed anyway, our lit-tests rely on results for non-existing working
     // dirs.
   }
 
-  // FIXME(ibiryukov): store diagnostics from CommandLine when we start
-  // reporting them.
-  IgnoreDiagnostics IgnoreDiagnostics;
   llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
-      CompilerInstance::createDiagnostics(new DiagnosticOptions,
-                                          &IgnoreDiagnostics, false);
+      CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
   std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(
       ArgStrs, CommandLineDiagsEngine, Inputs.FS,
       /*ShouldRecoverOnErrors=*/true);
   if (!CI)
     return nullptr;
   // createInvocationFromCommandLine sets DisableFree.
   CI->getFrontendOpts().DisableFree = false;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;

clang-tools-extra/trunk/clangd/Diagnostics.cpp

 #include "Compiler.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "clang/Basic/AllDiagnostics.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
case DiagnosticsEngine::Error:
     return 1;
   case DiagnosticsEngine::Ignored:
     return 0;
   }
   llvm_unreachable("Unknown diagnostic level!");
 }
 
 std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
+  // Do not forget to emit a pending diagnostic if there is one.
+  flushLastDiag();
+
   // Fill in name/source now that we have all the context needed to map them.
   for (auto &Diag : Output) {
     if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
       // Warnings controlled by -Wfoo are better recognized by that name.
       StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
       if (!Warning.empty()) {
         Diag.Name = ("-W" + Warning).str();
       } else {

 }
 
 void StoreDiags::BeginSourceFile(const LangOptions &Opts,
                                  const Preprocessor *) {
   LangOpts = Opts;
 }
 
 void StoreDiags::EndSourceFile() {
-  flushLastDiag();
   LangOpts = None;
 }
 
 /// Sanitizes a piece for presenting it in a synthesized fix message. Ensures
 /// the result is not too large and does not contain newlines.
 static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) {
   constexpr unsigned MaxLen = 50;
 
   // Only show the first line if there are many.
   llvm::StringRef R = Code.split('\n').first;
   // Shorten the message if it's too long.
   R = R.take_front(MaxLen);
 
   OS << R;
   if (R.size() != Code.size())
     OS << "…";
 }
 
+/// Fills \p D with all information, except the location-related bits.
+/// Also note that ID and Name are not part of clangd::DiagBase and should be
+/// set elsewhere.
+static void fillNonLocationData(DiagnosticsEngine::Level DiagLevel,
+                                const clang::Diagnostic &Info,
+                                clangd::DiagBase &D) {
+  llvm::SmallString<64> Message;
+  Info.FormatDiagnostic(Message);
+
+  D.Message = Message.str();
+  D.Severity = DiagLevel;
+  D.Category = DiagnosticIDs::getCategoryNameFromID(
+                   DiagnosticIDs::getCategoryNumberForDiag(Info.getID()))
+                   .str();
+}
+
 void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                                   const clang::Diagnostic &Info) {
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 
+  if (Info.getLocation().isInvalid()) {
+    // Handle diagnostics coming from command-line arguments. The source manager
+    // is *not* available at this point, so we cannot use it.
+    if (DiagLevel < DiagnosticsEngine::Level::Error) {
+      IgnoreDiagnostics::log(DiagLevel, Info);
+      return; // non-errors add too much noise, do not show them.
+    }
+
+    flushLastDiag();
+
+    LastDiag = Diag();
+    LastDiag->ID = Info.getID();
+    fillNonLocationData(DiagLevel, Info, *LastDiag);
+    LastDiag->InsideMainFile = true;
+    // Put it at the start of the main file, for a lack of a better place.
+    LastDiag->Range.start = Position{0, 0};
+    LastDiag->Range.end = Position{0, 0};
+    return;
+  }
+
   if (!LangOpts || !Info.hasSourceManager()) {
     IgnoreDiagnostics::log(DiagLevel, Info);
     return;
   }
 
   bool InsideMainFile = isInsideMainFile(Info);
   SourceManager &SM = Info.getSourceManager();
 
   auto FillDiagBase = [&](DiagBase &D) {
-    D.Range = diagnosticRange(Info, *LangOpts);
-    llvm::SmallString<64> Message;
-    Info.FormatDiagnostic(Message);
-    D.Message = Message.str();
+    fillNonLocationData(DiagLevel, Info, D);
+
     D.InsideMainFile = InsideMainFile;
+    D.Range = diagnosticRange(Info, *LangOpts);
     D.File = SM.getFilename(Info.getLocation());
     D.AbsFile = getCanonicalPath(
         SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
-    D.Severity = DiagLevel;
-    D.Category = DiagnosticIDs::getCategoryNameFromID(
-                     DiagnosticIDs::getCategoryNumberForDiag(Info.getID()))
-                     .str();
     return D;
   };
 
   auto AddFix = [&](bool SyntheticMessage) -> bool {
     assert(!Info.getFixItHints().empty() &&
            "diagnostic does not have attached fix-its");
     if (!InsideMainFile)
       return false;
if (Adjuster) {
         return;
       }
     }
     LastPrimaryDiagnosticWasSuppressed = false;
 
     LastDiag = Diag();
     LastDiag->ID = Info.getID();
     FillDiagBase(*LastDiag);
-    LastDiagWasAdjusted = false;
     if (!InsideMainFile)
       LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
 
     if (!Info.getFixItHints().empty())
       AddFix(true /* try to invent a message instead of repeating the diag */);
     if (Fixer) {
       auto ExtraFixes = Fixer(DiagLevel, Info);
       LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
if (mentionsMainFile(*LastDiag) &&
       (!LastDiagWasAdjusted ||
        // Only report the first diagnostic coming from each particular header.
        IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
     Output.push_back(std::move(*LastDiag));
   } else {
     vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
   }
   LastDiag.reset();
+  LastDiagWasAdjusted = false;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/TUScheduler.h

 //===--- TUScheduler.h -------------------------------------------*-C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 
 #include "ClangdUnit.h"
+#include "Diagnostics.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
+#include "Path.h"
 #include "Threading.h"
 #include "index/CanonicalIncludes.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <future>
 
public:
   ///     Highlights = computeHighlights();
   ///     Publish([&] { notifyHighlights(Path, Highlights); });
   ///   }
   /// This guarantees that clients will see results in the correct sequence if
   /// the file is concurrently closed and/or reopened. (The lambda passed to
   /// Publish() may never run in this case).
   virtual void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) {}
 
+  /// Called whenever the AST fails to build. \p Diags will have the diagnostics
+  /// that led to failure.
+  virtual void onFailedAST(PathRef Path, std::vector<Diag> Diags,
+                           PublishFn Publish) {}
+
   /// Called whenever the TU status is updated.
   virtual void onFileUpdated(PathRef File, const TUStatus &Status) {}
 };
 
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
 /// and scheduling tasks.

clang-tools-extra/trunk/clangd/TUScheduler.cpp

 // - If subsequent updates come before rebuild was started, we replace the
 //   pending inputs and reset the timer.
 // - If any reads of the AST are scheduled, we start building the AST
 //   immediately.
 
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "Trace.h"
 #include "index/CanonicalIncludes.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
#ifndef NDEBUG
   assert(Done && "handle was not destroyed");
   assert(Requests.empty() && "unprocessed requests when destroying ASTWorker");
 #endif
 }
 
 void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
   llvm::StringRef TaskName = "Update";
   auto Task = [=]() mutable {
+    auto RunPublish = [&](llvm::function_ref<void()> Publish) {
+      // Ensure we only publish results from the worker if the file was not
+      // removed, making sure there are not race conditions.
+      std::lock_guard<std::mutex> Lock(PublishMu);
+      if (CanPublishResults)
+        Publish();
+    };
+
     // Get the actual command as `Inputs` does not have a command.
     // FIXME: some build systems like Bazel will take time to preparing
     // environment to build the file, it would be nice if we could emit a
     // "PreparingBuild" status to inform users, it is non-trivial given the
     // current implementation.
     if (auto Cmd = CDB.getCompileCommand(FileName))
       Inputs.CompileCommand = *Cmd;
     else
auto Task = [=]() mutable {
     }
     RanASTCallback = false;
     emitTUStatus({TUAction::BuildingPreamble, TaskName});
     log("Updating file {0} with command {1}\n[{2}]\n{3}", FileName,
         Inputs.CompileCommand.Heuristic,
         Inputs.CompileCommand.Directory,
         llvm::join(Inputs.CompileCommand.CommandLine, " "));
     // Rebuild the preamble and the AST.
+    StoreDiags CompilerInvocationDiagConsumer;
     std::unique_ptr<CompilerInvocation> Invocation =
-        buildCompilerInvocation(Inputs);
+        buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+    std::vector<Diag> CompilerInvocationDiags =
+        CompilerInvocationDiagConsumer.take();
     if (!Invocation) {
       elog("Could not build CompilerInvocation for file {0}", FileName);
       // Remove the old AST if it's still in cache.
       IdleASTs.take(this);
       TUStatus::BuildDetails Details;
       Details.BuildFailed = true;
       emitTUStatus({TUAction::BuildingPreamble, TaskName}, &Details);
+      // Report the diagnostics we collected when parsing the command line.
+      Callbacks.onFailedAST(FileName, std::move(CompilerInvocationDiags),
+                            RunPublish);
       // Make sure anyone waiting for the preamble gets notified it could not
       // be built.
       PreambleWasBuilt.notify();
       return;
     }
 
     std::shared_ptr<const PreambleData> OldPreamble =
         getPossiblyStalePreamble();
if (WantDiags == WantDiagnostics::No)
       if (!CanPublishResults)
         return;
     }
 
     // Get the AST for diagnostics.
     llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
     if (!AST) {
       llvm::Optional<ParsedAST> NewAST =
-          buildAST(FileName, std::move(Invocation), Inputs, NewPreamble);
+          buildAST(FileName, std::move(Invocation), CompilerInvocationDiags,
+                   Inputs, NewPreamble);
       AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
       if (!(*AST)) { // buildAST fails.
         TUStatus::BuildDetails Details;
         Details.BuildFailed = true;
         emitTUStatus({TUAction::BuildingFile, TaskName}, &Details);
       }
     } else {
       // We are reusing the AST.
       TUStatus::BuildDetails Details;
       Details.ReuseAST = true;
       emitTUStatus({TUAction::BuildingFile, TaskName}, &Details);
     }
+
     // 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.
     // Note *AST can still be null if buildAST fails.
     if (*AST) {
       trace::Span Span("Running main AST callback");
-      auto RunPublish = [&](llvm::function_ref<void()> Publish) {
-        // Ensure we only publish results from the worker if the file was not
-        // removed, making sure there are not race conditions.
-        std::lock_guard<std::mutex> Lock(PublishMu);
-        if (CanPublishResults)
-          Publish();
-      };
 
       Callbacks.onMainAST(FileName, **AST, RunPublish);
       RanASTCallback = true;
+    } else {
+      // Failed to build the AST, at least report diagnostics from the command
+      // line if there were any.
+      // FIXME: we might have got more errors while trying to build the AST,
+      //        surface them too.
+      Callbacks.onFailedAST(FileName, CompilerInvocationDiags, RunPublish);
     }
     // Stash the AST in the cache for further use.
     IdleASTs.put(this, std::move(*AST));
   };
   startTask(TaskName, std::move(Task), WantDiags);
 }
 
 void ASTWorker::runWithAST(
     llvm::StringRef Name,
     llvm::unique_function<void(llvm::Expected<InputsAndAST>)> Action) {
   auto Task = [=, Action = std::move(Action)]() mutable {
     if (isCancelled())
       return Action(llvm::make_error<CancelledError>());
     llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
     auto CurrentInputs = getCurrentFileInputs();
     if (!AST) {
-      std::unique_ptr<CompilerInvocation> Invocation =
-          buildCompilerInvocation(*CurrentInputs);
+      StoreDiags CompilerInvocationDiagConsumer;
+      std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
+          *CurrentInputs, CompilerInvocationDiagConsumer);
       // Try rebuilding the AST.
       llvm::Optional<ParsedAST> NewAST =
           Invocation
               ? buildAST(FileName,
                          std::make_unique<CompilerInvocation>(*Invocation),
-                         *CurrentInputs, getPossiblyStalePreamble())
+                         CompilerInvocationDiagConsumer.take(), *CurrentInputs,
+                         getPossiblyStalePreamble())
               : None;
       AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
     }
     // Make sure we put the AST back into the LRU cache.
     auto _ = llvm::make_scope_exit(
         [&AST, this]() { IdleASTs.put(this, std::move(*AST)); });
     // Run the user-provided action.
     if (!*AST)

clang-tools-extra/trunk/clangd/index/Background.cpp

llvm::StringMap<ShardVersion> ShardVersionsSnapshot;
     ShardVersionsSnapshot = ShardVersions;
   }
 
   vlog("Indexing {0} (digest:={1})", Cmd.Filename, llvm::toHex(Hash));
   ParseInputs Inputs;
   Inputs.FS = std::move(FS);
   Inputs.FS->setCurrentWorkingDirectory(Cmd.Directory);
   Inputs.CompileCommand = std::move(Cmd);
-  auto CI = buildCompilerInvocation(Inputs);
+  IgnoreDiagnostics IgnoreDiags;
+  auto CI = buildCompilerInvocation(Inputs, IgnoreDiags);
   if (!CI)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "Couldn't build compiler invocation");
-  IgnoreDiagnostics IgnoreDiags;
   auto Clang = prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr,
                                        std::move(*Buf), Inputs.FS, IgnoreDiags);
   if (!Clang)
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "Couldn't build compiler instance");
 
   SymbolCollector::Options IndexOpts;
   // Creates a filter to not collect index results from files with unchanged

clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

 //===-- ClangdUnitTests.cpp - ClangdUnit tests ------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock-matchers.h"

 }
 
 TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
   // Unknown flags should not prevent a build of compiler invocation.
   ParseInputs Inputs;
   Inputs.FS = buildTestFS({{testPath("foo.cpp"), "void test() {}"}});
   Inputs.CompileCommand.CommandLine = {"clang", "-fsome-unknown-flag",
                                        testPath("foo.cpp")};
-  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+  IgnoreDiagnostics IgnoreDiags;
+  EXPECT_NE(buildCompilerInvocation(Inputs, IgnoreDiags), nullptr);
 
   // Unknown forwarded to -cc1 should not a failure either.
   Inputs.CompileCommand.CommandLine = {
       "clang", "-Xclang", "-fsome-unknown-flag", testPath("foo.cpp")};
-  EXPECT_NE(buildCompilerInvocation(Inputs), nullptr);
+  EXPECT_NE(buildCompilerInvocation(Inputs, IgnoreDiags), nullptr);
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp

 //===-- FileIndexTests.cpp  ---------------------------*- C++ -*-----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/Utils.h"
TEST(FileIndexTest, RebuildWithPreamble) {
   PI.Contents = R"cpp(
     #include "foo.h"
     namespace ns_in_source {
       int func_in_source();
     }
   )cpp";
 
   // Rebuild the file.
-  auto CI = buildCompilerInvocation(PI);
+  IgnoreDiagnostics IgnoreDiags;
+  auto CI = buildCompilerInvocation(PI, IgnoreDiags);
 
   FileIndex Index;
   bool IndexUpdated = false;
   buildPreamble(
       FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
       /*StoreInMemory=*/true,
       [&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP,
           const CanonicalIncludes &CanonIncludes) {

clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp

std::unique_ptr<CompilerInstance> setupClang() {
     auto Cmd = CDB.getCompileCommand(MainFile);
     assert(static_cast<bool>(Cmd));
     auto VFS = FS.getFileSystem();
     VFS->setCurrentWorkingDirectory(Cmd->Directory);
 
     ParseInputs PI;
     PI.CompileCommand = *Cmd;
     PI.FS = VFS;
-    auto CI = buildCompilerInvocation(PI);
+    auto CI = buildCompilerInvocation(PI, IgnoreDiags);
     EXPECT_TRUE(static_cast<bool>(CI));
     // The diagnostic options must be set before creating a CompilerInstance.
     CI->getDiagnosticOpts().IgnoreWarnings = true;
     auto Clang = prepareCompilerInstance(
         std::move(CI), /*Preamble=*/nullptr,
         llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile), VFS,
         IgnoreDiags);
 

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

 //===-- TUSchedulerTests.cpp ------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Context.h"
 #include "Diagnostics.h"
 #include "Matchers.h"
 #include "Path.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
+#include "Threading.h"
+#include "clang/Basic/DiagnosticDriver.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
 #include <utility>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
+using ::testing::Eq;
+using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
 MATCHER_P2(TUState, State, ActionName, "") {
   return arg.Action.S == State && arg.Action.Name == ActionName;
 }
 
 class TUSchedulerTests : public ::testing::Test {
protected:
 
   static Key<llvm::unique_function<void(PathRef File, std::vector<Diag>)>>
       DiagsCallbackKey;
 
   /// A diagnostics callback that should be passed to TUScheduler when it's used
   /// in updateWithDiags.
   static std::unique_ptr<ParsingCallbacks> captureDiags() {
     class CaptureDiags : public ParsingCallbacks {
+    public:
       void onMainAST(PathRef File, ParsedAST &AST, PublishFn Publish) override {
-        auto Diags = AST.getDiagnostics();
+        reportDiagnostics(File, AST.getDiagnostics(), Publish);
+      }
+
+      void onFailedAST(PathRef File, std::vector<Diag> Diags,
+                       PublishFn Publish) override {
+        reportDiagnostics(File, Diags, Publish);
+      }
+
+    private:
+      void reportDiagnostics(PathRef File, llvm::ArrayRef<Diag> Diags,
+                             PublishFn Publish) {
         auto D = Context::current().get(DiagsCallbackKey);
         if (!D)
           return;
-
         Publish([&]() {
           const_cast<
               llvm::unique_function<void(PathRef, std::vector<Diag>)> &> (*D)(
               File, std::move(Diags));
         });
       }
     };
     return std::make_unique<CaptureDiags>();
EXPECT_THAT(CaptureTUStatus.allStatus(),
                   TUState(TUAction::BuildingPreamble, "Update"),
                   TUState(TUAction::BuildingFile, "Update"),
 
                   // Statuses of "Definitions" action
                   TUState(TUAction::RunningAction, "Definitions"),
                   TUState(TUAction::Idle, /*No action*/ "")));
 }
 
+TEST_F(TUSchedulerTests, CommandLineErrors) {
+  // We should see errors from command-line parsing inside the main file.
+  CDB.ExtraClangFlags = {"-fsome-unknown-flag"};
+
+  TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+                /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/captureDiags(),
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
+
+  Notification Ready;
+  std::vector<Diag> Diagnostics;
+  updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
+                  WantDiagnostics::Yes, [&](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"};
+
+  TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
+                /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/captureDiags(),
+                /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+                ASTRetentionPolicy());
+
+  Notification Ready;
+  std::vector<Diag> Diagnostics;
+  updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
+                  WantDiagnostics::Yes, [&](std::vector<Diag> D) {
+                    Diagnostics = std::move(D);
+                    Ready.notify();
+                  });
+  Ready.wait();
+
+  EXPECT_THAT(Diagnostics, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/trunk/clangd/unittests/TestTU.cpp

 //===--- TestTU.cpp - Scratch source files for testing --------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TestTU.h"
+#include "Compiler.h"
+#include "Diagnostics.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/Utils.h"
 
 namespace clang {
ParsedAST TestTU::build() const {
   Inputs.Contents = Code;
   Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
     Inputs.Opts.SuggestMissingIncludes = true;
-  auto CI = buildCompilerInvocation(Inputs);
+  StoreDiags Diags;
+  auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
   auto Preamble =
       buildPreamble(FullFilename, *CI,
                     /*OldPreamble=*/nullptr,
                     /*OldCompileCommand=*/Inputs.CompileCommand, Inputs,
                     /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
-  auto AST = buildAST(FullFilename, std::move(CI), Inputs, Preamble);
+  auto AST =
+      buildAST(FullFilename, std::move(CI), Diags.take(), Inputs, Preamble);
   if (!AST.hasValue()) {
     ADD_FAILURE() << "Failed to build code:\n" << Code;
     llvm_unreachable("Failed to build TestTU!");
   }
   return std::move(*AST);
 }
 
 SymbolSlab TestTU::headerSymbols() const {