[pull] master from llvm:master

clang-tools-extra/clangd/ClangdServer.cpp

@@ -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);
   }

clang-tools-extra/clangd/ClangdUnit.cpp

@@ -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);
 }

clang-tools-extra/clangd/ClangdUnit.h

@@ -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 @@ class ParsedAST {
   /// 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);
 

clang-tools-extra/clangd/CodeComplete.cpp

@@ -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());
 

clang-tools-extra/clangd/Compiler.cpp

@@ -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);

clang-tools-extra/clangd/Compiler.h

@@ -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.

clang-tools-extra/clangd/Diagnostics.cpp

@@ -16,11 +16,13 @@
 #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

clang-tools-extra/clangd/TUScheduler.cpp

@@ -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,15 +403,21 @@ 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.
       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();
@@ -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;
     }

clang-tools-extra/clangd/TUScheduler.h

@@ -10,8 +10,10 @@
 #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 @@ class ParsingCallbacks {
   /// 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) {}
 };