[Importer] While pretty-printing decls for diagnostics, don't emit diagnostics.

DougGregor · DougGregor · commit e316a8ef9080 · 2021-03-19T09:33:55.000-07:00
The diagnostics system doesn't allow a diagnostic to be emitted while
another diagnostic is in flight. Doing so will cause an assertion in
the diagnostics machinery.

There's a longstanding cycle here when diagnostics emission
pretty-prints declarations that are imported from a Clang module, and
the Clang Importer emits a diagnostic. Squash this cycle forcefully,
dropping the diagnostic that the Clang importer would emit.
diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h
@@ -740,6 +740,10 @@ namespace swift {
     /// Path to diagnostic documentation directory.
     std::string diagnosticDocumentationPath = "";
 
+    /// Whether we are actively pretty-printing a declaration as part of
+    /// diagnostics.
+    bool IsPrettyPrintingDecl = false;
+
     friend class InFlightDiagnostic;
     friend class DiagnosticTransaction;
     friend class CompoundDiagnosticTransaction;
@@ -796,6 +800,8 @@ namespace swift {
       return diagnosticDocumentationPath;
     }
 
+    bool isPrettyPrintingDecl() const { return IsPrettyPrintingDecl; }
+
     void setLocalization(std::string locale, std::string path) {
       assert(!locale.empty());
       assert(!path.empty());
diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp
@@ -1039,6 +1039,8 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
           // Pretty-print the declaration we've picked.
           llvm::raw_svector_ostream out(buffer);
           TrackingPrinter printer(entries, out, bufferAccessLevel);
+          llvm::SaveAndRestore<bool> isPrettyPrinting(
+              IsPrettyPrintingDecl, true);
           ppDecl->print(
               printer,
               PrintOptions::printForDiagnostics(
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -1366,7 +1366,7 @@ bool ClangImporter::Implementation::importHeader(
   // to correct. The fix would be explicitly importing on the command line.
   if (implicitImport && !allParsedDecls.empty() &&
     BridgingHeaderExplicitlyRequested) {
-    SwiftContext.Diags.diagnose(
+    diagnose(
       diagLoc, diag::implicit_bridging_header_imported_from_module,
       llvm::sys::path::filename(headerName), adapter->getName());
   }
@@ -1401,8 +1401,7 @@ bool ClangImporter::Implementation::importHeader(
 
   // FIXME: What do we do if there was already an error?
   if (!hadError && clangDiags.hasErrorOccurred()) {
-    SwiftContext.Diags.diagnose(diagLoc, diag::bridging_header_error,
-                                headerName);
+    diagnose(diagLoc, diag::bridging_header_error, headerName);
     return true;
   }
 
@@ -1450,8 +1449,7 @@ bool ClangImporter::importBridgingHeader(StringRef header, ModuleDecl *adapter,
   clang::FileManager &fileManager = Impl.Instance->getFileManager();
   auto headerFile = fileManager.getFile(header, /*OpenFile=*/true);
   if (!headerFile) {
-    Impl.SwiftContext.Diags.diagnose(diagLoc, diag::bridging_header_missing,
-                                     header);
+    Impl.diagnose(diagLoc, diag::bridging_header_missing, header);
     return true;
   }
 
@@ -1519,8 +1517,7 @@ std::string ClangImporter::getBridgingHeaderContents(StringRef headerPath,
 
   success |= !rewriteInstance.getDiagnostics().hasErrorOccurred();
   if (!success) {
-    Impl.SwiftContext.Diags.diagnose({},
-                                     diag::could_not_rewrite_bridging_header);
+    Impl.diagnose({}, diag::could_not_rewrite_bridging_header);
     return "";
   }
 
@@ -1607,9 +1604,8 @@ ClangImporter::emitBridgingPCH(StringRef headerPath,
   emitInstance->ExecuteAction(*action);
 
   if (emitInstance->getDiagnostics().hasErrorOccurred()) {
-    Impl.SwiftContext.Diags.diagnose({},
-                                     diag::bridging_header_pch_error,
-                                     outputPCHPath, headerPath);
+    Impl.diagnose({}, diag::bridging_header_pch_error,
+                  outputPCHPath, headerPath);
     return true;
   }
   return false;
@@ -1668,9 +1664,7 @@ bool ClangImporter::emitPrecompiledModule(StringRef moduleMapPath,
   emitInstance->ExecuteAction(*action);
 
   if (emitInstance->getDiagnostics().hasErrorOccurred()) {
-    Impl.SwiftContext.Diags.diagnose({},
-                                     diag::emit_pcm_error,
-                                     outputPath, moduleMapPath);
+    Impl.diagnose({}, diag::emit_pcm_error, outputPath, moduleMapPath);
     return true;
   }
   return false;
@@ -1693,7 +1687,7 @@ bool ClangImporter::dumpPrecompiledModule(StringRef modulePath,
   dumpInstance->ExecuteAction(*action);
 
   if (dumpInstance->getDiagnostics().hasErrorOccurred()) {
-    Impl.SwiftContext.Diags.diagnose({}, diag::dump_pcm_error, modulePath);
+    Impl.diagnose({}, diag::dump_pcm_error, modulePath);
     return true;
   }
   return false;
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -3985,18 +3985,15 @@ namespace {
 
         if (dc->getSelfProtocolDecl() && !selfIdx) {
           // FIXME: source location...
-          Impl.SwiftContext.Diags.diagnose({}, diag::swift_name_protocol_static,
-                                           /*isInit=*/false);
-          Impl.SwiftContext.Diags.diagnose({}, diag::note_while_importing,
-                                           decl->getName());
+          Impl.diagnose({}, diag::swift_name_protocol_static, /*isInit=*/false);
+          Impl.diagnose({}, diag::note_while_importing, decl->getName());
           return nullptr;
         }
 
         if (!decl->hasPrototype()) {
           // FIXME: source location...
-          Impl.SwiftContext.Diags.diagnose({}, diag::swift_name_no_prototype);
-          Impl.SwiftContext.Diags.diagnose({}, diag::note_while_importing,
-                                           decl->getName());
+          Impl.diagnose({}, diag::swift_name_no_prototype);
+          Impl.diagnose({}, diag::note_while_importing, decl->getName());
           return nullptr;
         }
 
@@ -6302,10 +6299,8 @@ Decl *SwiftDeclConverter::importGlobalAsInitializer(
   // Check for some invalid imports
   if (dc->getSelfProtocolDecl()) {
     // FIXME: clang source location
-    Impl.SwiftContext.Diags.diagnose({}, diag::swift_name_protocol_static,
-                                     /*isInit=*/true);
-    Impl.SwiftContext.Diags.diagnose({}, diag::note_while_importing,
-                                     decl->getName());
+    Impl.diagnose({}, diag::swift_name_protocol_static, /*isInit=*/true);
+    Impl.diagnose({}, diag::note_while_importing, decl->getName());
     return nullptr;
   }
 
@@ -8217,9 +8212,8 @@ void ClangImporter::Implementation::importAttributes(
           getBufferImporterForDiagnostics();
         SourceLoc attrLoc = bufferImporter.resolveSourceLocation(
           clangSrcMgr, swiftAttr->getLocation());
-        SwiftContext.Diags.diagnose(
-              attrLoc, diag::clang_swift_attr_without_at,
-              swiftAttr->getAttribute());
+        diagnose(attrLoc, diag::clang_swift_attr_without_at,
+                 swiftAttr->getAttribute());
       }
       continue;
     }
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -2450,12 +2450,12 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     SourceLoc methodLoc =
         bufferImporter.resolveSourceLocation(srcMgr, clangDecl->getLocation());
     if (methodLoc.isValid()) {
-      SwiftContext.Diags.diagnose(methodLoc, diag::invalid_swift_name_method,
+      diagnose(methodLoc, diag::invalid_swift_name_method,
                                   swiftParams.size() < argNames.size(),
                                   swiftParams.size(), argNames.size());
       ModuleDecl *parentModule = dc->getParentModule();
       if (parentModule != ImportedHeaderUnit->getParentModule()) {
-        SwiftContext.Diags.diagnose(
+        diagnose(
             methodLoc, diag::unresolvable_clang_decl_is_a_framework_bug,
             parentModule->getName().str());
       }
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -747,6 +747,25 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
                       bool fullyQualified,
                       llvm::raw_ostream &os);
 
+  /// Emit a diagnostic, taking care not to interrupt a diagnostic that's
+  /// already in flight.
+  template<typename ...Args>
+  void diagnose(Args &&...args) {
+    // If we're in the middle of pretty-printing, suppress diagnostics.
+    if (SwiftContext.Diags.isPrettyPrintingDecl()) {
+      return;
+    }
+
+    SwiftContext.Diags.diagnose(std::forward<Args>(args)...);
+  }
+
+  /// Emit a diagnostic, taking care not to interrupt a diagnostic that's
+  /// already in flight.
+  template<typename ...Args>
+  void diagnose(SourceLoc loc, Args &&...args) {
+    diagnose(loc, std::forward<Args>(args)...);
+  }
+
   /// Import the given Clang identifier into Swift.
   ///
   /// \param identifier The Clang identifier to map into Swift.
