[ClangImporter] Diagnose bad swift_name attributes better

1. Set the diagnostic location to where the attribute was written (or
   to the Clang decl's source, if the attribute came from API notes)

2. Add a note to contact the owners of the framework to make it clear
   that the client of the framework didn't do anything wrong.

rdar://problem/52736145

Author: jrose-apple

include/swift/AST/DiagnosticsClangImporter.def

@@ -86,6 +86,10 @@ WARNING(unresolvable_clang_decl,none,
         "imported declaration '%0' could not be mapped to '%1'",
         (StringRef, StringRef))
 
+NOTE(unresolvable_clang_decl_is_a_framework_bug,none,
+     "please report this issue to the owners of '%0'",
+     (StringRef))
+
 WARNING(implicit_bridging_header_imported_from_module,none,
         "implicit import of bridging header '%0' via module %1 "
         "is deprecated and will be removed in a later version of Swift",

lib/ClangImporter/ClangImporter.cpp

@@ -1000,6 +1000,7 @@ ClangImporter::create(ASTContext &ctx, const ClangImporterOptions &importerOpts,
       std::make_shared<SwiftNameLookupExtension>(
           importer->Impl.BridgingHeaderLookupTable,
           importer->Impl.LookupTables, importer->Impl.SwiftContext,
+          importer->Impl.getBufferImporterForDiagnostics(),
           importer->Impl.platformAvailability,
           importer->Impl.InferImportAsMember));
 
@@ -1330,7 +1331,8 @@ bool ClangImporter::Implementation::importHeader(
   }
 
   // Finalize the lookup table, which may fail.
-  finalizeLookupTable(*BridgingHeaderLookupTable, getNameImporter());
+  finalizeLookupTable(*BridgingHeaderLookupTable, getNameImporter(),
+                      getBufferImporterForDiagnostics());
 
   // FIXME: What do we do if there was already an error?
   if (!hadError && clangDiags.hasErrorOccurred()) {
@@ -1844,7 +1846,6 @@ ClangImporter::Implementation::Implementation(
     ASTContext &ctx, const ClangImporterOptions &opts,
     DWARFImporterDelegate *dwarfImporterDelegate)
     : SwiftContext(ctx),
-      BuffersForDiagnostics(ctx.SourceMgr),
       ImportForwardDeclarations(opts.ImportForwardDeclarations),
       InferImportAsMember(opts.InferImportAsMember),
       DisableSwiftBridgeAttr(opts.DisableSwiftBridgeAttr),
@@ -1853,6 +1854,7 @@ ClangImporter::Implementation::Implementation(
       IsReadingBridgingPCH(false),
       CurrentVersion(ImportNameVersion::fromOptions(ctx.LangOpts)),
       BridgingHeaderLookupTable(new SwiftLookupTable(nullptr)),
+      BuffersForDiagnostics(ctx.SourceMgr),
       platformAvailability(ctx.LangOpts), nameImporter(),
       DWARFImporter(dwarfImporterDelegate) {}
 

lib/ClangImporter/ImporterImpl.h

@@ -1380,15 +1380,18 @@ class SwiftNameLookupExtension : public clang::ModuleFileExtension {
   std::unique_ptr<SwiftLookupTable> &pchLookupTable;
   LookupTableMap &lookupTables;
   ASTContext &swiftCtx;
+  ClangSourceBufferImporter &buffersForDiagnostics;
   const PlatformAvailability &availability;
   const bool inferImportAsMember;
 
 public:
   SwiftNameLookupExtension(std::unique_ptr<SwiftLookupTable> &pchLookupTable,
                            LookupTableMap &tables, ASTContext &ctx,
+                           ClangSourceBufferImporter &buffersForDiagnostics,
                            const PlatformAvailability &avail, bool inferIAM)
       : pchLookupTable(pchLookupTable), lookupTables(tables), swiftCtx(ctx),
-        availability(avail), inferImportAsMember(inferIAM) {}
+        buffersForDiagnostics(buffersForDiagnostics), availability(avail),
+        inferImportAsMember(inferIAM) {}
 
   clang::ModuleFileExtensionMetadata getExtensionMetadata() const override;
   llvm::hash_code hashExtension(llvm::hash_code code) const override;

lib/ClangImporter/SwiftLookupTable.cpp

@@ -67,15 +67,19 @@ class SwiftLookupTableWriter : public clang::ModuleFileExtensionWriter {
   clang::ASTWriter &Writer;
 
   ASTContext &swiftCtx;
+  importer::ClangSourceBufferImporter &buffersForDiagnostics;
   const PlatformAvailability &availability;
   const bool inferImportAsMember;
 
 public:
-  SwiftLookupTableWriter(clang::ModuleFileExtension *extension,
-                         clang::ASTWriter &writer, ASTContext &ctx,
-                         const PlatformAvailability &avail, bool inferIAM)
-      : ModuleFileExtensionWriter(extension), Writer(writer), swiftCtx(ctx),
-        availability(avail), inferImportAsMember(inferIAM) {}
+  SwiftLookupTableWriter(
+      clang::ModuleFileExtension *extension, clang::ASTWriter &writer,
+      ASTContext &ctx,
+      importer::ClangSourceBufferImporter &buffersForDiagnostics,
+      const PlatformAvailability &avail, bool inferIAM)
+    : ModuleFileExtensionWriter(extension), Writer(writer), swiftCtx(ctx),
+      buffersForDiagnostics(buffersForDiagnostics), availability(avail),
+      inferImportAsMember(inferIAM) {}
 
   void writeExtensionContents(clang::Sema &sema,
                               llvm::BitstreamWriter &stream) override;
@@ -1788,8 +1792,9 @@ void importer::addMacrosToLookupTable(SwiftLookupTable &table,
   }
 }
 
-void importer::finalizeLookupTable(SwiftLookupTable &table,
-                                   NameImporter &nameImporter) {
+void importer::finalizeLookupTable(
+    SwiftLookupTable &table, NameImporter &nameImporter,
+    ClangSourceBufferImporter &buffersForDiagnostics) {
   // Resolve any unresolved entries.
   SmallVector<SwiftLookupTable::SingleEntry, 4> unresolved;
   if (table.resolveUnresolvedEntries(unresolved)) {
@@ -1799,9 +1804,22 @@ void importer::finalizeLookupTable(SwiftLookupTable &table,
       auto swiftName = decl->getAttr<clang::SwiftNameAttr>();
 
       if (swiftName) {
-        nameImporter.getContext().Diags.diagnose(
-            SourceLoc(), diag::unresolvable_clang_decl, decl->getNameAsString(),
-            swiftName->getName());
+        clang::SourceLocation diagLoc = swiftName->getLocation();
+        if (!diagLoc.isValid())
+          diagLoc = decl->getLocation();
+        SourceLoc swiftSourceLoc = buffersForDiagnostics.resolveSourceLocation(
+            nameImporter.getClangContext().getSourceManager(), diagLoc);
+
+        DiagnosticEngine &swiftDiags = nameImporter.getContext().Diags;
+        swiftDiags.diagnose(swiftSourceLoc, diag::unresolvable_clang_decl,
+                            decl->getNameAsString(), swiftName->getName());
+        StringRef moduleName =
+            nameImporter.getClangContext().getLangOpts().CurrentModule;
+        if (!moduleName.empty()) {
+          swiftDiags.diagnose(swiftSourceLoc,
+                              diag::unresolvable_clang_decl_is_a_framework_bug,
+                              moduleName);
+        }
       }
     }
   }
@@ -1842,14 +1860,15 @@ void SwiftLookupTableWriter::populateTable(SwiftLookupTable &table,
   addMacrosToLookupTable(table, nameImporter);
 
   // Finalize the lookup table, which may fail.
-  finalizeLookupTable(table, nameImporter);
+  finalizeLookupTable(table, nameImporter, buffersForDiagnostics);
 };
 
 std::unique_ptr<clang::ModuleFileExtensionWriter>
 SwiftNameLookupExtension::createExtensionWriter(clang::ASTWriter &writer) {
-  return std::unique_ptr<clang::ModuleFileExtensionWriter>(
-      new SwiftLookupTableWriter(this, writer, swiftCtx, availability,
-                                 inferImportAsMember));
+  return llvm::make_unique<SwiftLookupTableWriter>(this, writer, swiftCtx,
+                                                   buffersForDiagnostics,
+                                                   availability,
+                                                   inferImportAsMember);
 }
 
 std::unique_ptr<clang::ModuleFileExtensionReader>

lib/ClangImporter/SwiftLookupTable.h

@@ -520,6 +520,7 @@ class SwiftLookupTable {
 };
 
 namespace importer {
+class ClangSourceBufferImporter;
 class NameImporter;
 
 /// Add the given named declaration as an entry to the given Swift name
@@ -533,7 +534,8 @@ void addMacrosToLookupTable(SwiftLookupTable &table, NameImporter &);
 
 /// Finalize a lookup table, handling any as-yet-unresolved entries
 /// and emitting diagnostics if necessary.
-void finalizeLookupTable(SwiftLookupTable &table, NameImporter &);
+void finalizeLookupTable(SwiftLookupTable &table, NameImporter &,
+                         ClangSourceBufferImporter &buffersForDiagnostics);
 }
 }
 

test/ClangImporter/Inputs/custom-modules/ImportAsMember.apinotes

@@ -12,6 +12,8 @@ Functions:
 Typedefs:
 - Name: IAMStruct1APINoteType
   SwiftName: Struct1.NewApiNoteType
+- Name: IAMBadInnerIntAPINotes
+  SwiftName: IAMNonexistent.Inner2
 SwiftVersions:
 - Version: 3
   Globals:

test/ClangImporter/Inputs/custom-modules/ImportAsMember.h

@@ -68,4 +68,12 @@ struct IAMMultipleNested {
 typedef int MNInnerInt __attribute__((swift_name("IAMMultipleNested.Inner")));
 typedef float MNInnerFloat __attribute__((swift_name("IAMMultipleNested.Inner")));
 
+typedef int IAMBadInnerInt
+    __attribute__((swift_name("IAMNonexistent.Inner")));
+// CHECK: ImportAsMember.h:[[@LINE-1]]:{{[0-9]+}}: warning: imported declaration 'IAMBadInnerInt' could not be mapped to 'IAMNonexistent.Inner'
+// CHECK: ImportAsMember.h:[[@LINE-2]]:{{[0-9]+}}: note: please report this issue to the owners of 'ImportAsMember'
+typedef int IAMBadInnerIntAPINotes;
+// CHECK: ImportAsMember.h:[[@LINE-1]]:{{[0-9]+}}: warning: imported declaration 'IAMBadInnerIntAPINotes' could not be mapped to 'IAMNonexistent.Inner2'
+// CHECK: ImportAsMember.h:[[@LINE-2]]:{{[0-9]+}}: note: please report this issue to the owners of 'ImportAsMember'
+
 #endif // IMPORT_AS_MEMBER_H

test/ClangImporter/import-as-member.swift

@@ -1,3 +1,4 @@
+// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -typecheck -F %S/Inputs/frameworks -I %S/Inputs/custom-modules %s 2>&1 | %FileCheck %S/Inputs/custom-modules/ImportAsMember.h
 // RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-objc-interop -typecheck -F %S/Inputs/frameworks -I %S/Inputs/custom-modules %s -verify
 
 import ImportAsMember