[NFC] Add ClangImporter header diagnostic helper

beccadax · beccadax · commit d2d786a76a5c · 2021-11-19T11:34:01.000-08:00
diff --git a/lib/ClangImporter/ClangDiagnosticConsumer.cpp b/lib/ClangImporter/ClangDiagnosticConsumer.cpp
@@ -121,21 +121,13 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
     return;
   }
 
-  const ASTContext &ctx = ImporterImpl.SwiftContext;
-  ClangSourceBufferImporter &bufferImporter =
-      ImporterImpl.getBufferImporterForDiagnostics();
-
   if (clangDiag.getID() == clang::diag::err_module_not_built &&
       CurrentImport && clangDiag.getArgStdStr(0) == CurrentImport->getName()) {
-    SourceLoc loc = DiagLoc;
-    if (clangDiag.getLocation().isValid()) {
-      loc = bufferImporter.resolveSourceLocation(clangDiag.getSourceManager(),
-                                                 clangDiag.getLocation());
-    }
-
-    ctx.Diags.diagnose(loc, diag::clang_cannot_build_module,
-                       ctx.LangOpts.EnableObjCInterop,
-                       CurrentImport->getName());
+    HeaderLoc loc(clangDiag.getLocation(), DiagLoc,
+                  &clangDiag.getSourceManager());
+    ImporterImpl.diagnose(loc, diag::clang_cannot_build_module,
+                          ImporterImpl.SwiftContext.LangOpts.EnableObjCInterop,
+                          CurrentImport->getName());
     return;
   }
 
@@ -146,10 +138,9 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
     DiagnosticConsumer::HandleDiagnostic(clangDiagLevel, clangDiag);
 
   // FIXME: Map over source ranges in the diagnostic.
-  auto emitDiag =
-      [&ctx, &bufferImporter](clang::FullSourceLoc clangNoteLoc,
-                              clang::DiagnosticsEngine::Level clangDiagLevel,
-                              StringRef message) {
+  auto emitDiag = [this](clang::FullSourceLoc clangNoteLoc,
+                         clang::DiagnosticsEngine::Level clangDiagLevel,
+                         StringRef message) {
     decltype(diag::error_from_clang) diagKind;
     switch (clangDiagLevel) {
     case clang::DiagnosticsEngine::Ignored:
@@ -170,11 +161,9 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
       break;
     }
 
-    SourceLoc noteLoc;
-    if (clangNoteLoc.isValid())
-      noteLoc = bufferImporter.resolveSourceLocation(clangNoteLoc.getManager(),
-                                                     clangNoteLoc);
-    ctx.Diags.diagnose(noteLoc, diagKind, message);
+    HeaderLoc noteLoc(clangNoteLoc, SourceLoc(),
+              clangNoteLoc.hasManager() ? &clangNoteLoc.getManager() : nullptr);
+    ImporterImpl.diagnose(noteLoc, diagKind, message);
   };
 
   llvm::SmallString<128> message;
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -8607,12 +8607,7 @@ void ClangImporter::Implementation::importAttributes(
         if (SeenMainActorAttr) {
           // Cannot add main actor annotation twice. We'll keep the first
           // one and raise a warning about the duplicate.
-          auto &clangSrcMgr = getClangASTContext().getSourceManager();
-          ClangSourceBufferImporter &bufferImporter =
-              getBufferImporterForDiagnostics();
-          SourceLoc attrLoc = bufferImporter.resolveSourceLocation(
-              clangSrcMgr, swiftAttr->getLocation());
-
+          HeaderLoc attrLoc(swiftAttr->getLocation());
           diagnose(attrLoc, diag::import_multiple_mainactor_attr,
                    swiftAttr->getAttribute(),
                    SeenMainActorAttr.getValue()->getAttribute());
@@ -8668,11 +8663,7 @@ void ClangImporter::Implementation::importAttributes(
 
       if (hadError) {
         // Complain about the unhandled attribute or modifier.
-        auto &clangSrcMgr = getClangASTContext().getSourceManager();
-        ClangSourceBufferImporter &bufferImporter =
-          getBufferImporterForDiagnostics();
-        SourceLoc attrLoc = bufferImporter.resolveSourceLocation(
-          clangSrcMgr, swiftAttr->getLocation());
+        HeaderLoc attrLoc(swiftAttr->getLocation());
         diagnose(attrLoc, diag::clang_swift_attr_unhandled,
                  swiftAttr->getAttribute());
       }
@@ -9246,27 +9237,21 @@ ClangImporter::Implementation::importDeclForDeclContext(
   if (!contextDeclsWarnedAbout.insert(contextDecl).second)
     return nullptr;
 
-  auto convertLoc = [&](clang::SourceLocation clangLoc) {
-    return getBufferImporterForDiagnostics()
-      .resolveSourceLocation(getClangASTContext().getSourceManager(),
-                             clangLoc);
-  };
-
   auto getDeclName = [](const clang::Decl *D) -> StringRef {
     if (auto ND = dyn_cast<clang::NamedDecl>(D))
       return ND->getName();
     return "<anonymous>";
   };
 
-  SourceLoc loc = convertLoc(importingDecl->getLocation());
+  HeaderLoc loc(importingDecl->getLocation());
   diagnose(loc, diag::swift_name_circular_context_import,
            writtenName, getDeclName(importingDecl));
 
   // Diagnose other decls involved in the cycle.
   for (auto entry : make_range(contextDeclsBeingImported.rbegin(), iter)) {
     auto otherDecl = std::get<0>(entry);
     auto otherWrittenName = std::get<1>(entry);
-    diagnose(convertLoc(otherDecl->getLocation()),
+    diagnose(HeaderLoc(otherDecl->getLocation()),
              diag::swift_name_circular_context_import_other,
              otherWrittenName, getDeclName(otherDecl));
   }
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -2579,20 +2579,15 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
 
   if (importedName.hasCustomName() && argNames.size() != swiftParams.size()) {
     // Note carefully: we're emitting a warning in the /Clang/ buffer.
-    auto &srcMgr = getClangASTContext().getSourceManager();
-    ClangSourceBufferImporter &bufferImporter =
-        getBufferImporterForDiagnostics();
-    SourceLoc methodLoc =
-        bufferImporter.resolveSourceLocation(srcMgr, clangDecl->getLocation());
-    if (methodLoc.isValid()) {
+    if (clangDecl->getLocation().isValid()) {
+      HeaderLoc methodLoc(clangDecl->getLocation());
       diagnose(methodLoc, diag::invalid_swift_name_method,
-                                  swiftParams.size() < argNames.size(),
-                                  swiftParams.size(), argNames.size());
+               swiftParams.size() < argNames.size(),
+               swiftParams.size(), argNames.size());
       ModuleDecl *parentModule = dc->getParentModule();
       if (parentModule != ImportedHeaderUnit->getParentModule()) {
-        diagnose(
-            methodLoc, diag::unresolvable_clang_decl_is_a_framework_bug,
-            parentModule->getName().str());
+        diagnose(methodLoc, diag::unresolvable_clang_decl_is_a_framework_bug,
+                 parentModule->getName().str());
       }
     }
     return {Type(), false};
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -317,6 +317,19 @@ class ImportedType {
   explicit operator bool() const { return type.getPointer() != nullptr; }
 };
 
+/// Wraps a Clang source location with additional optional information used to
+/// resolve it for diagnostics.
+struct HeaderLoc {
+  clang::SourceLocation clangLoc;
+  SourceLoc fallbackLoc;
+  const clang::SourceManager *sourceMgr;
+
+  explicit HeaderLoc(clang::SourceLocation clangLoc,
+                     SourceLoc fallbackLoc = SourceLoc(),
+                     const clang::SourceManager *sourceMgr = nullptr)
+    : clangLoc(clangLoc), fallbackLoc(fallbackLoc), sourceMgr(sourceMgr) {}
+};
+
 /// Implementation of the Clang importer.
 class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation 
   : public LazyMemberLoader,
@@ -778,6 +791,31 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
     SwiftContext.Diags.diagnose(loc, std::forward<Args>(args)...);
   }
 
+  /// Emit a diagnostic at a clang source location, falling back to a Swift
+  /// location if the clang one is invalid.
+  ///
+  /// The diagnostic will appear in the header file rather than in a generated
+  /// interface. Use this to diagnose issues with declarations that are not
+  /// imported or that are not reflected in a generated interface.
+  template<typename ...Args>
+  void diagnose(HeaderLoc loc, Args &&...args) {
+    // If we're in the middle of pretty-printing, suppress diagnostics.
+    if (SwiftContext.Diags.isPrettyPrintingDecl()) {
+      return;
+    }
+
+    auto swiftLoc = loc.fallbackLoc;
+    if (loc.clangLoc.isValid()) {
+      auto &clangSrcMgr = loc.sourceMgr ? *loc.sourceMgr
+                        : getClangASTContext().getSourceManager();
+      auto &bufferImporter = getBufferImporterForDiagnostics();
+      swiftLoc = bufferImporter.resolveSourceLocation(clangSrcMgr,
+                                                      loc.clangLoc);
+    }
+
+    SwiftContext.Diags.diagnose(swiftLoc, std::forward<Args>(args)...);
+  }
+
   /// Import the given Clang identifier into Swift.
   ///
   /// \param identifier The Clang identifier to map into Swift.
