[clangd] Surface errors from command-line parsing
authorIlya Biryukov <ibiryukov@google.com>
Wed, 28 Aug 2019 09:24:55 +0000 (09:24 +0000)
committerIlya Biryukov <ibiryukov@google.com>
Wed, 28 Aug 2019 09:24:55 +0000 (09:24 +0000)
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.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: nridge, javed.absar, MaskRay, jkorous, arphaman, cfe-commits, gribozavr

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66759

llvm-svn: 370177

15 files changed:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdUnit.cpp
clang-tools-extra/clangd/ClangdUnit.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp

index 460883b84391fd99d6d7293f951f50cc5c58b828..c4ecc5286ba9e6e8ba4e0e426988a4bd751ba39b 100644 (file)
@@ -80,6 +80,11 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
     });
   }
 
+  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);
   }
index f85cac200da38a3259c7a009e6c27275f9aad1fb..fd5202f37517377fdacec4c3cae9c02cd6101519 100644 (file)
@@ -292,7 +292,8 @@ void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
 }
 
 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,
@@ -459,10 +460,15 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
   // 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));
@@ -646,6 +652,7 @@ buildPreamble(PathRef FileName, CompilerInvocation &CI,
 
 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");
@@ -661,7 +668,8 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
   }
 
   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);
 }
index f5b18f97387f9de22b552db29490e2a4a7cebc07..3af34b019648deb89a9869ad71ad95cef75e5261 100644 (file)
@@ -25,6 +25,7 @@
 #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>
@@ -76,10 +77,11 @@ public:
   /// 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);
@@ -174,6 +176,7 @@ buildPreamble(PathRef FileName, CompilerInvocation &CI,
 /// 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);
 
index b5304dbffe74a6cafcbecb0dc4b3876e091831a0..045320fc543f34ab9fcb85d3205c364115b3f86b 100644 (file)
@@ -1053,7 +1053,9 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   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;
@@ -1084,12 +1086,11 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   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());
 
index 7080e20e879e76ac64604c3709191e39f0d51afd..e0801433319076726f98489cde26c30cd39525ec 100644 (file)
@@ -41,7 +41,8 @@ void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 }
 
 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());
@@ -52,12 +53,8 @@ buildCompilerInvocation(const ParseInputs &Inputs) {
     // 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);
index c24ea3546c5c46978046f374a1adcd32811aecbd..689514ab4801c8787d027128f378094d8781e3ad 100644 (file)
@@ -52,7 +52,8 @@ struct ParseInputs {
 
 /// 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.
index 7f1ab06db9d1d3096fb2f073a21ee87e20180ef5..c9e1ed6bc6872cdf8427db957556b2b7e5b01b29 100644 (file)
 #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"
@@ -393,6 +395,9 @@ int getSeverity(DiagnosticsEngine::Level L) {
 }
 
 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)) {
@@ -448,7 +453,6 @@ void StoreDiags::BeginSourceFile(const LangOptions &Opts,
 }
 
 void StoreDiags::EndSourceFile() {
-  flushLastDiag();
   LangOpts = None;
 }
 
@@ -467,10 +471,46 @@ static void writeCodeToFixMessage(llvm::raw_ostream &OS, llvm::StringRef Code) {
     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;
@@ -480,18 +520,13 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
   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;
   };
 
@@ -564,7 +599,6 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
     LastDiag = Diag();
     LastDiag->ID = Info.getID();
     FillDiagBase(*LastDiag);
-    LastDiagWasAdjusted = false;
     if (!InsideMainFile)
       LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
 
@@ -617,6 +651,7 @@ void StoreDiags::flushLastDiag() {
     vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
   }
   LastDiag.reset();
+  LastDiagWasAdjusted = false;
 }
 
 } // namespace clangd
index a09bf3f6a43d77b6cf150a9f2c96840311c5cf20..7052feceb35b9ec4d085ae90285ca375026e8985 100644 (file)
@@ -44,6 +44,7 @@
 #include "TUScheduler.h"
 #include "Cancellation.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "GlobalCompilationDatabase.h"
 #include "Logger.h"
 #include "Trace.h"
@@ -365,6 +366,14 @@ ASTWorker::~ASTWorker() {
 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
@@ -394,8 +403,11 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
         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.
@@ -403,6 +415,9 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
       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();
@@ -468,7 +483,8 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
     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;
@@ -481,22 +497,22 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
       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));
@@ -513,14 +529,16 @@ void ASTWorker::runWithAST(
     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;
     }
index d6f530a751d428952f1d928508f92a4bfa942016..e02250d6e6f7ac82676f4ddf8931309434cbc745 100644 (file)
 #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"
@@ -125,6 +127,11 @@ public:
   /// 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) {}
 };
index 6d2360e62ad002a9c0cd6403683fff6fd486e9c2..b58236ef7d8a7c80efc3d247130d0ed1aec1d823 100644 (file)
@@ -369,11 +369,11 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
   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)
index 7fe57025dc770445237b5571db8d1ea23fe2cc1a..430a056c1ea1bda036647d4a0316f7af573cdf91 100644 (file)
@@ -10,6 +10,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -252,12 +253,13 @@ TEST(ClangdUnitTest, CanBuildInvocationWithUnknownArgs) {
   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
index f1f304f935e0c7e0f38df200401e1ab1733e009a..f7b5ecafb8adc12b146a6c741ed901bd168ed535 100644 (file)
@@ -9,6 +9,7 @@
 #include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
+#include "Compiler.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -280,7 +281,8 @@ TEST(FileIndexTest, RebuildWithPreamble) {
   )cpp";
 
   // Rebuild the file.
-  auto CI = buildCompilerInvocation(PI);
+  IgnoreDiagnostics IgnoreDiags;
+  auto CI = buildCompilerInvocation(PI, IgnoreDiags);
 
   FileIndex Index;
   bool IndexUpdated = false;
index da701309e35cdbfa193b5f1060bb850fc82f8127..d07312ca5884fe49d74889aedf5ca874ae2ed0ba 100644 (file)
@@ -46,7 +46,7 @@ private:
     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;
index b605c940360f5d95188f96563f935d3f36d590c3..274c07f99cd7019929f9674fa841359d0eb0a3d2 100644 (file)
@@ -14,6 +14,9 @@
 #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"
@@ -28,6 +31,9 @@ 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;
 
@@ -60,12 +66,22 @@ protected:
   /// 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)(
@@ -720,6 +736,53 @@ TEST_F(TUSchedulerTests, TUStatus) {
                   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
index 0c1727eccad6cf2057dc7a058804a73590341c36..75393f1415b17fd75daae39ad4dacd46fa544964 100644 (file)
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "TestTU.h"
+#include "Compiler.h"
+#include "Diagnostics.h"
 #include "TestFS.h"
 #include "index/FileIndex.h"
 #include "index/MemIndex.h"
@@ -59,14 +61,16 @@ ParsedAST TestTU::build() const {
   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!");