[sourcekit] Attempt to provide diagnostics when compilation fails for any reason

Refactors the diagnostic code to be run whenever a compilation
notification has been started and there are diagnostics available in the
consumer. This allows us to capture diagnostics on all exit paths, and
specifically when code-completion fails because of invalid arguments.

Note: the editor.open code path still doesn't report invalid arguments
because it fails before even trying to create an AST.

Author: benlangmuir

test/SourceKit/CompileNotifications/arg-parsing.swift

@@ -7,8 +7,12 @@
 // ARG_PARSE_1: {
 // ARG_PARSE_1:   key.notification: source.notification.compile-did-finish
 // ARG_PARSE_1:   key.diagnostics: [
-// FIXME: we should pass through the error from parsing the arguments
-// ARG_PARSE_1-NEXT: ]
+// ARG_PARSE_1:     {
+// ARG_PARSE_1:       key.filepath: "<unknown>",
+// ARG_PARSE_1:       key.severity: source.diagnostic.severity.error,
+// ARG_PARSE_1:       key.description: "unknown argument: '-no-such-arg'"
+// ARG_PARSE_1:     }
+// ARG_PARSE_1:   ]
 // ARG_PARSE_1:   key.compileid: [[CID1]]
 // ARG_PARSE_1: }
 // ARG_PARSE_1-NOT: compile-will-start

tools/SourceKit/include/SourceKit/Support/Tracing.h

@@ -14,6 +14,7 @@
 #define LLVM_SOURCEKIT_SUPPORT_TRACING_H
 
 #include "SourceKit/Core/LLVM.h"
+#include "SourceKit/Core/LangSupport.h"
 #include "SourceKit/Support/UIdent.h"
 #include "swift/Basic/OptionSet.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -91,8 +92,11 @@ void unregisterConsumer(TraceConsumer *Consumer);
 
 // Class that utilizes the RAII idiom for the operations being traced
 class TracedOperation final {
+  using DiagnosticProvider = std::function<void(SmallVectorImpl<DiagnosticEntryInfo> &)>;
+
   OperationKind OpKind;
   llvm::Optional<uint64_t> OpId;
+  llvm::Optional<DiagnosticProvider> DiagProvider;
   bool Enabled;
 
 public:
@@ -116,13 +120,20 @@ class TracedOperation final {
     OpId = startOperation(OpKind, Inv, OpArgs);
   }
 
-  void finish(ArrayRef<DiagnosticEntryInfo> Diagnostics = llvm::None) {
+  void finish() {
     if (OpId.hasValue()) {
+      SmallVector<DiagnosticEntryInfo, 8> Diagnostics;
+      if (DiagProvider.hasValue())
+        (*DiagProvider)(Diagnostics);
       operationFinished(OpId.getValue(), OpKind, Diagnostics);
       OpId.reset();
     }
   }
 
+  void setDiagnosticProvider(DiagnosticProvider &&DiagProvider) {
+    assert(!this->DiagProvider.hasValue());
+    this->DiagProvider = std::move(DiagProvider);
+  }
 };
 
 } // namespace sourcekitd

tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp

@@ -794,24 +794,27 @@ ASTUnitRef ASTProducer::createASTUnit(SwiftASTManager::Implementation &MgrImpl,
   for (auto &Content : Contents)
     Stamps.push_back(Content.Stamp);
 
-  trace::TracedOperation TracedOp(trace::OperationKind::PerformSema);
-  trace::SwiftInvocation TraceInfo;
-  if (TracedOp.enabled()) {
-    trace::initTraceInfo(TraceInfo, InvokRef->Impl.Opts.PrimaryFile,
-                         InvokRef->Impl.Opts.Args);
-  }
-
   ASTUnitRef ASTRef = new ASTUnit(++ASTUnitGeneration, MgrImpl.Stats);
   for (auto &Content : Contents) {
     if (Content.Snapshot)
       ASTRef->Impl.Snapshots.push_back(Content.Snapshot);
   }
   auto &CompIns = ASTRef->Impl.CompInst;
   auto &Consumer = ASTRef->Impl.CollectDiagConsumer;
-
   // Display diagnostics to stderr.
   CompIns.addDiagnosticConsumer(&Consumer);
 
+  trace::TracedOperation TracedOp(trace::OperationKind::PerformSema);
+  trace::SwiftInvocation TraceInfo;
+  if (TracedOp.enabled()) {
+    trace::initTraceInfo(TraceInfo, InvokRef->Impl.Opts.PrimaryFile,
+                         InvokRef->Impl.Opts.Args);
+    TracedOp.setDiagnosticProvider(
+        [&Consumer](SmallVectorImpl<DiagnosticEntryInfo> &diags) {
+          Consumer.getAllDiagnostics(diags);
+        });
+  }
+
   CompilerInvocation Invocation;
   InvokRef->Impl.Opts.applyToSubstitutingInputs(
       Invocation, convertFileContentsToInputs(Contents));
@@ -875,12 +878,6 @@ ASTUnitRef ASTProducer::createASTUnit(SwiftASTManager::Implementation &MgrImpl,
   // TypeResolver is set before.
   ASTRef->Impl.TypeResolver = createLazyResolver(CompIns.getASTContext());
 
-  if (TracedOp.enabled()) {
-    SmallVector<DiagnosticEntryInfo, 8> Diagnostics;
-    Consumer.getAllDiagnostics(Diagnostics);
-    TracedOp.finish(Diagnostics);
-  }
-
   return ASTRef;
 }
 

tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp

@@ -136,6 +136,10 @@ static bool swiftCodeCompleteImpl(SwiftLangSupport &Lang,
     CI.addDiagnosticConsumer(&TraceDiags);
     trace::SwiftInvocation SwiftArgs;
     trace::initTraceInfo(SwiftArgs, InputFile->getBufferIdentifier(), Args);
+    TracedOp.setDiagnosticProvider(
+        [&TraceDiags](SmallVectorImpl<DiagnosticEntryInfo> &diags) {
+          TraceDiags.getAllDiagnostics(diags);
+        });
     TracedOp.start(SwiftArgs,
                    {std::make_pair("OriginalOffset", std::to_string(Offset)),
                     std::make_pair("Offset",
@@ -191,12 +195,6 @@ static bool swiftCodeCompleteImpl(SwiftLangSupport &Lang,
   CI.performSema();
   SwiftConsumer.clearContext();
 
-  if (TracedOp.enabled()) {
-    SmallVector<DiagnosticEntryInfo, 8> Diagnostics;
-    TraceDiags.getAllDiagnostics(Diagnostics);
-    TracedOp.finish(Diagnostics);
-  }
-
   return true;
 }