[ClangImporter] Take isCompatibilityAlias() into account in interface printing (#16625)

If the Clang declrations are *types*, canonical declaration in Swift is
imported for newest version of Swift. In interface generation, if the
declaration is versioned and it's imported as a member in either or both
version of Swift, we have to take compatibility typealias into account.

* Fixed 'ClangModuleUnit::getTopLevelDecls' to take isCompatibilityAlias() into account
* Fixed bugs in ClangImporter where member-to-member versioned types aren't properly imported.
  * Fixed 'SwiftDeclConverter::importFullName' to check equality of getEffectiveContext()
  * Fixed 'importer::addEntryToLookupTable' to check equality of getEffectiveContext()
    (moved 'ClangImporter::Implementation::forEachDistinctName' to 'NameImporter')

lib/ClangImporter/ClangImporter.cpp

@@ -2455,6 +2455,15 @@ void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
         results.push_back(extension);
     }
 
+    auto findEnclosingExtension = [](Decl *importedDecl) -> ExtensionDecl * {
+      for (auto importedDC = importedDecl->getDeclContext();
+           !importedDC->isModuleContext();
+           importedDC = importedDC->getParent()) {
+        if (auto ext = dyn_cast<ExtensionDecl>(importedDC))
+          return ext;
+      }
+      return nullptr;
+    };
     // Retrieve all of the globals that will be mapped to members.
 
     // FIXME: Since we don't represent Clang submodules as Swift
@@ -2462,21 +2471,33 @@ void ClangModuleUnit::getTopLevelDecls(SmallVectorImpl<Decl*> &results) const {
     llvm::SmallPtrSet<ExtensionDecl *, 8> knownExtensions;
     for (auto entry : lookupTable->allGlobalsAsMembers()) {
       auto decl = entry.get<clang::NamedDecl *>();
-      auto importedDecl =
-          owner.importDecl(decl, owner.CurrentVersion);
+      auto importedDecl = owner.importDecl(decl, owner.CurrentVersion);
       if (!importedDecl) continue;
 
       // Find the enclosing extension, if there is one.
-      ExtensionDecl *ext = nullptr;
-      for (auto importedDC = importedDecl->getDeclContext();
-           !importedDC->isModuleContext();
-           importedDC = importedDC->getParent()) {
-        ext = dyn_cast<ExtensionDecl>(importedDC);
-        if (ext) break;
-      }
-      if (!ext) continue;
+      ExtensionDecl *ext = findEnclosingExtension(importedDecl);
+      if (ext && knownExtensions.insert(ext).second)
+        results.push_back(ext);
 
-      if (knownExtensions.insert(ext).second)
+      // If this is a compatibility typealias, the canonical type declaration
+      // may exist in another extension.
+      auto alias = dyn_cast<TypeAliasDecl>(importedDecl);
+      if (!alias || !alias->isCompatibilityAlias()) continue;
+
+      auto aliasedTy = alias->getUnderlyingTypeLoc().getType();
+      ext = nullptr;
+      importedDecl = nullptr;
+
+      // Note: We can't use getAnyGeneric() here because `aliasedTy`
+      // might be typealias.
+      if (auto Ty = dyn_cast<NameAliasType>(aliasedTy.getPointer()))
+        importedDecl = Ty->getDecl();
+      else if (auto Ty = dyn_cast<AnyGenericType>(aliasedTy.getPointer()))
+        importedDecl = Ty->getDecl();
+      if (!importedDecl) continue;
+
+      ext = findEnclosingExtension(importedDecl);
+      if (ext && knownExtensions.insert(ext).second)
         results.push_back(ext);
     }
   }

lib/ClangImporter/ImportDecl.cpp

@@ -386,35 +386,6 @@ static bool isNSDictionaryMethod(const clang::ObjCMethodDecl *MD,
   return true;
 }
 
-void ClangImporter::Implementation::forEachDistinctName(
-    const clang::NamedDecl *decl,
-    llvm::function_ref<bool(ImportedName, ImportNameVersion)> action) {
-  using ImportNameKey = std::pair<DeclName, EffectiveClangContext>;
-  SmallVector<ImportNameKey, 8> seenNames;
-
-  ImportedName newName = importFullName(decl, CurrentVersion);
-  ImportNameKey key(newName, newName.getEffectiveContext());
-  if (action(newName, CurrentVersion))
-    seenNames.push_back(key);
-
-  CurrentVersion.forEachOtherImportNameVersion(
-      [&](ImportNameVersion nameVersion) {
-    // Check to see if the name is different.
-    ImportedName newName = importFullName(decl, nameVersion);
-    ImportNameKey key(newName, newName.getEffectiveContext());
-    bool seen = llvm::any_of(seenNames,
-                             [&key](const ImportNameKey &existing) -> bool {
-      if (key.first != existing.first)
-        return false;
-      return key.second.equalsWithoutResolving(existing.second);
-    });
-    if (seen)
-      return;
-    if (action(newName, nameVersion))
-      seenNames.push_back(key);
-  });
-}
-
 // Build the init(rawValue:) initializer for an imported NS_ENUM.
 //   enum NSSomeEnum: RawType {
 //     init?(rawValue: RawType) {
@@ -2175,7 +2146,9 @@ namespace {
         if (canonicalVersion != getActiveSwiftVersion()) {
           auto activeName = Impl.importFullName(D, getActiveSwiftVersion());
           if (activeName &&
-              activeName.getDeclName() == canonicalName.getDeclName()) {
+              activeName.getDeclName() == canonicalName.getDeclName() &&
+              activeName.getEffectiveContext().equalsWithoutResolving(
+                  canonicalName.getEffectiveContext())) {
             return ImportedName();
           }
         }
@@ -2192,7 +2165,9 @@ namespace {
       if (!alternateName)
         return ImportedName();
 
-      if (alternateName.getDeclName() == canonicalName.getDeclName()) {
+      if (alternateName.getDeclName() == canonicalName.getDeclName() &&
+          alternateName.getEffectiveContext().equalsWithoutResolving(
+              canonicalName.getEffectiveContext())) {
         if (getVersion() == getActiveSwiftVersion()) {
           assert(canonicalVersion != getActiveSwiftVersion());
           return alternateName;

lib/ClangImporter/ImportName.cpp

@@ -1779,6 +1779,40 @@ ImportedName NameImporter::importName(const clang::NamedDecl *decl,
   return res;
 }
 
+bool NameImporter::forEachDistinctImportName(
+    const clang::NamedDecl *decl, ImportNameVersion activeVersion,
+    llvm::function_ref<bool(ImportedName, ImportNameVersion)> action) {
+  using ImportNameKey = std::pair<DeclName, EffectiveClangContext>;
+  SmallVector<ImportNameKey, 8> seenNames;
+
+  ImportedName newName = importName(decl, activeVersion);
+  if (!newName)
+    return true;
+  ImportNameKey key(newName.getDeclName(), newName.getEffectiveContext());
+  if (action(newName, activeVersion))
+    seenNames.push_back(key);
+
+  activeVersion.forEachOtherImportNameVersion(
+      [&](ImportNameVersion nameVersion) {
+        // Check to see if the name is different.
+        ImportedName newName = importName(decl, nameVersion);
+        if (!newName)
+          return;
+        ImportNameKey key(newName.getDeclName(), newName.getEffectiveContext());
+
+        bool seen = llvm::any_of(
+            seenNames, [&key](const ImportNameKey &existing) -> bool {
+              return key.first == existing.first &&
+                     key.second.equalsWithoutResolving(existing.second);
+            });
+        if (seen)
+          return;
+        if (action(newName, nameVersion))
+          seenNames.push_back(key);
+      });
+  return false;
+}
+
 const InheritedNameSet *NameImporter::getAllPropertyNames(
                           clang::ObjCInterfaceDecl *classDecl,
                           bool forInstance) {
@@ -1846,4 +1880,3 @@ const InheritedNameSet *NameImporter::getAllPropertyNames(
 
   return known->second.get();
 }
-

lib/ClangImporter/ImportName.h

@@ -334,6 +334,28 @@ public:
                           clang::DeclarationName preferredName =
                             clang::DeclarationName());
 
+  /// Attempts to import the name of \p decl with each possible
+  /// ImportNameVersion. \p action will be called with each unique name.
+  ///
+  /// In this case, "unique" means either the full name is distinct or the
+  /// effective context is distinct. This method does not attempt to handle
+  /// "unresolved" contexts in any special way---if one name references a
+  /// particular Clang declaration and the other has an unresolved context that
+  /// will eventually reference that declaration, the contexts will still be
+  /// considered distinct.
+  ///
+  /// If \p action returns false, the current name will \e not be added to the
+  /// set of seen names.
+  ///
+  /// The active name for \p activeVerion is always first, followed by the
+  /// other names in the order of
+  /// ImportNameVersion::forEachOtherImportNameVersion.
+  ///
+  /// Returns \c true if it fails to import name for the active version.
+  bool forEachDistinctImportName(
+      const clang::NamedDecl *decl, ImportNameVersion activeVersion,
+      llvm::function_ref<bool(ImportedName, ImportNameVersion)> action);
+
   /// Imports the name of the given Clang macro into Swift.
   Identifier importMacroName(const clang::IdentifierInfo *clangIdentifier,
                              const clang::MacroInfo *macro);

lib/ClangImporter/ImporterImpl.h

@@ -1352,7 +1352,9 @@ public:
   void forEachDistinctName(
       const clang::NamedDecl *decl,
       llvm::function_ref<bool(importer::ImportedName,
-                              importer::ImportNameVersion)> action);
+                              importer::ImportNameVersion)> action) {
+    getNameImporter().forEachDistinctImportName(decl, CurrentVersion, action);
+  }
 
   /// Dump the Swift-specific name lookup tables we generate.
   void dumpSwiftLookupTables();

lib/ClangImporter/SwiftLookupTable.cpp

@@ -1633,37 +1633,30 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
   // If we have a name to import as, add this entry to the table.
   auto currentVersion =
       ImportNameVersion::fromOptions(nameImporter.getLangOpts());
-  if (auto importedName = nameImporter.importName(named, currentVersion)) {
-    SmallPtrSet<DeclName, 8> distinctNames;
-    distinctNames.insert(importedName.getDeclName());
-    table.addEntry(importedName.getDeclName(), named,
-                   importedName.getEffectiveContext());
+  auto failed = nameImporter.forEachDistinctImportName(
+      named, currentVersion,
+      [&](ImportedName importedName, ImportNameVersion version) {
+        table.addEntry(importedName.getDeclName(), named,
+                       importedName.getEffectiveContext());
 
-    // Also add the subscript entry, if needed.
-    if (importedName.isSubscriptAccessor())
-      table.addEntry(DeclName(nameImporter.getContext(),
-                              DeclBaseName::createSubscript(),
-                              ArrayRef<Identifier>()),
-                     named, importedName.getEffectiveContext());
+        // Also add the subscript entry, if needed.
+        if (version == currentVersion && importedName.isSubscriptAccessor()) {
+          table.addEntry(DeclName(nameImporter.getContext(),
+                                  DeclBaseName::createSubscript(),
+                                  {Identifier()}),
+                         named, importedName.getEffectiveContext());
+        }
 
-    currentVersion.forEachOtherImportNameVersion(
-        [&](ImportNameVersion alternateVersion) {
-      auto alternateName = nameImporter.importName(named, alternateVersion);
-      if (!alternateName)
+        return true;
+      });
+  if (failed) {
+    if (auto category = dyn_cast<clang::ObjCCategoryDecl>(named)) {
+      // If the category is invalid, don't add it.
+      if (category->isInvalidDecl())
         return;
-      // FIXME: What if the DeclNames are the same but the contexts are
-      // different?
-      if (distinctNames.insert(alternateName.getDeclName()).second) {
-        table.addEntry(alternateName.getDeclName(), named,
-                       alternateName.getEffectiveContext());
-      }
-    });
-  } else if (auto category = dyn_cast<clang::ObjCCategoryDecl>(named)) {
-    // If the category is invalid, don't add it.
-    if (category->isInvalidDecl())
-      return;
 
-    table.addCategory(category);
+      table.addCategory(category);
+    }
   }
 
   // Walk the members of any context that can have nested members.
@@ -1864,4 +1857,3 @@ SwiftNameLookupExtension::createExtensionReader(
   // Return the new reader.
   return std::move(tableReader);
 }
-

test/APINotes/Inputs/custom-modules/ObsoletedAPINotesTest.apinotes

@@ -2,9 +2,13 @@ Name: M
 Classes:
 - Name: FooID
   SwiftName: Foo_ID
+- Name: BarSub
+  SwiftName: BarContainerCanonical.Sub
 
 SwiftVersions:
 - Version: 4
   Classes:
   - Name: FooID
     SwiftName: FooID
+  - Name: BarSub
+    SwiftName: BarContainerOld.Sub

test/APINotes/Inputs/custom-modules/ObsoletedAPINotesTest.h

@@ -1,3 +1,10 @@
 @import Foundation;
 @interface FooID: NSObject
 @end
+
+@interface BarContainerOld
+@end
+@interface BarContainerCanonical
+@end
+@interface BarSub
+@end

test/APINotes/obsoleted.swift

@@ -6,3 +6,6 @@ import ObsoletedAPINotesTest
 
 let _: FooID // expected-error{{'FooID' has been renamed to 'Foo_ID'}}
 let _: Foo_ID
+
+let _: BarContainerOld.Sub // expected-error{{'Sub' has been renamed to 'BarContainerCanonical.Sub'}}
+let _: BarContainerCanonical.Sub

test/SourceKit/InterfaceGen/Inputs/mock-sdk/APINotesTests.framework/Headers/APINotesTests.apinotes

@@ -0,0 +1,47 @@
+Name: APINotesTests
+Classes:
+- Name:      GlobalToMember_Class_Payload
+  SwiftName: GlobalToMember_Class_Container.Payload
+- Name:      MemberToGlobal_Class_Payload
+  SwiftName: MemberToGlobal_Class_Payload
+- Name:      MemberToMember_Class_Payload
+  SwiftName: MemberToMember_Class_Swift4.PayloadFor4
+- Name:      MemberToMember_SameName_Class_Payload
+  SwiftName: MemberToMember_SameName_Class_Swift4.Payload
+- Name:      MemberToMember_SameContainer_Class_Payload
+  SwiftName: MemberToMember_SameContainer_Class_Container.PayloadFor4
+Typedefs:
+- Name:      GlobalToMember_Typedef_Payload
+  SwiftName: GlobalToMember_Typedef_Container.Payload
+- Name:      MemberToGlobal_Typedef_Payload
+  SwiftName: MemberToGlobal_Typedef_Payload
+- Name:      MemberToMember_Typedef_Payload
+  SwiftName: MemberToMember_Typedef_Swift4.PayloadFor4
+- Name:      MemberToMember_SameName_Typedef_Payload
+  SwiftName: MemberToMember_SameName_Typedef_Swift4.Payload
+- Name:      MemberToMember_SameContainer_Typedef_Payload
+  SwiftName: MemberToMember_SameContainer_Typedef_Container.PayloadFor4
+SwiftVersions:
+- Version: 3
+  Classes:
+  - Name:      GlobalToMember_Class_Payload
+    SwiftName: GlobalToMember_Class_Payload
+  - Name:      MemberToGlobal_Class_Payload
+    SwiftName: MemberToGlobal_Class_Container.Payload
+  - Name:      MemberToMember_Class_Payload
+    SwiftName: MemberToMember_Class_Swift3.PayloadFor3
+  - Name:      MemberToMember_SameContainer_Class_Payload
+    SwiftName: MemberToMember_SameContainer_Class_Container.PayloadFor3
+  - Name:      MemberToMember_SameName_Class_Payload
+    SwiftName: MemberToMember_SameName_Class_Swift3.Payload
+  Typedefs:
+  - Name:      GlobalToMember_Typedef_Payload
+    SwiftName: GlobalToMember_Typedef_Payload
+  - Name:      MemberToGlobal_Typedef_Payload
+    SwiftName: MemberToGlobal_Typedef_Container.Payload
+  - Name:      MemberToMember_Typedef_Payload
+    SwiftName: MemberToMember_Typedef_Swift3.PayloadFor3
+  - Name:      MemberToMember_SameContainer_Typedef_Payload
+    SwiftName: MemberToMember_SameContainer_Typedef_Container.PayloadFor3
+  - Name:      MemberToMember_SameName_Typedef_Payload
+    SwiftName: MemberToMember_SameName_Typedef_Swift3.Payload

test/SourceKit/InterfaceGen/Inputs/mock-sdk/APINotesTests.framework/Headers/APINotesTests.h

@@ -0,0 +1,3 @@
+
+#import "Foo.h"
+#import "Decls.h"

test/SourceKit/InterfaceGen/Inputs/mock-sdk/APINotesTests.framework/Headers/Decls.h

@@ -0,0 +1,82 @@
+@import Foundation;
+
+// ===-------------------------------------------------------------------------
+// class Payload
+// ===-------------------------------------------------------------------------
+
+// 3: Payload
+// 4: Namespace.Payload
+@interface GlobalToMember_Class_Container : NSObject
+@end
+@interface GlobalToMember_Class_Payload : NSObject
+@end
+
+// 3: Namespace.Payload
+// 4: Payload
+@interface MemberToGlobal_Class_Container : NSObject
+@end
+@interface MemberToGlobal_Class_Payload: NSObject
+@end
+
+// 3: Namespace_Swift3.PayloadFor3
+// 4: Namespace_Swift4.PayloadFor4
+@interface MemberToMember_Class_Swift3 : NSObject
+@end
+@interface MemberToMember_Class_Swift4 : NSObject
+@end
+@interface MemberToMember_Class_Payload : NSObject
+@end
+
+// 3: Namespace.PayloadFor3
+// 4: Namespace.PayloadFor4
+@interface MemberToMember_SameContainer_Class_Container : NSObject
+@end
+@interface MemberToMember_SameContainer_Class_Payload : NSObject
+@end
+
+// 3: Namespace_Swift3.Payload
+// 4: Namespace_Swift4.Payload
+@interface MemberToMember_SameName_Class_Swift3 : NSObject
+@end
+@interface MemberToMember_SameName_Class_Swift4 : NSObject
+@end
+@interface MemberToMember_SameName_Class_Payload : NSObject
+@end
+
+// ===-------------------------------------------------------------------------
+// typealias Payload
+// ===-------------------------------------------------------------------------
+
+// 3: Payload
+// 4: Namespace.Payload
+@interface GlobalToMember_Typedef_Container : NSObject
+@end
+typedef Foo* GlobalToMember_Typedef_Payload;
+
+// 3: Namespace.Payload
+// 4: Payload
+@interface MemberToGlobal_Typedef_Container : NSObject
+@end
+typedef Foo* MemberToGlobal_Typedef_Payload;
+
+// 3: Namespace_Swift3.PayloadFor3
+// 4: Namespace_Swift4.PayloadFor4
+@interface MemberToMember_Typedef_Swift3 : NSObject
+@end
+@interface MemberToMember_Typedef_Swift4 : NSObject
+@end
+typedef Foo* MemberToMember_Typedef_Payload;
+
+// 3: Namespace.PayloadFor3
+// 4: Namespace.PayloadFor4
+@interface MemberToMember_SameContainer_Typedef_Container : NSObject
+@end
+typedef Foo* MemberToMember_SameContainer_Typedef_Payload;
+
+// 3: Namespace_Swift3.Payload
+// 4: Namespace_Swift4.Payload
+@interface MemberToMember_SameName_Typedef_Swift3 : NSObject
+@end
+@interface MemberToMember_SameName_Typedef_Swift4 : NSObject
+@end
+typedef Foo* MemberToMember_SameName_Typedef_Payload;

test/SourceKit/InterfaceGen/Inputs/mock-sdk/APINotesTests.framework/Headers/Foo.h

@@ -0,0 +1,4 @@
+@import Foundation;
+
+@interface Foo : NSObject
+@end

test/SourceKit/InterfaceGen/Inputs/mock-sdk/APINotesTests.framework/Modules/module.modulemap

@@ -0,0 +1,4 @@
+framework module APINotesTests {
+  umbrella header "APINotesTests.h"
+  export *
+}

test/SourceKit/InterfaceGen/gen_clang_module.swift

@@ -90,3 +90,10 @@ var x: FooClassBase
 // CHECK-IMPORT: 	  source.lang.swift.ref.module ()
 // CHECK-IMPORT-NEXT: FooHelper{{$}}
 // CHECK-IMPORT-NEXT: FooHelper{{$}}
+
+// RUN: %sourcekitd-test -req=interface-gen -module APINotesTests -- -swift-version 3 -F %S/Inputs/mock-sdk \
+// RUN:         %mcp_opt -target %target-triple %clang-importer-sdk-nosource > %t.apinotes_swift3.response
+// RUN: diff -u %s.apinotes_swift3.response %t.apinotes_swift3.response
+// RUN: %sourcekitd-test -req=interface-gen -module APINotesTests -- -swift-version 4 -F %S/Inputs/mock-sdk \
+// RUN:         %mcp_opt -target %target-triple %clang-importer-sdk-nosource > %t.apinotes_swift4.response
+// RUN: diff -u %s.apinotes_swift4.response %t.apinotes_swift4.response