[ClangImporter] Fall back to Swift class names when resolving @Class

Christopher Rogers' (good) work in 49fd5ac caught places where
the Swift compiler was allowing a @Class to resolve to a Swift class
even if that class had a conflicting Objective-C name, or wasn't
intended to be exposed to Objective-C at all. Unfortunately, this
broke source compatibility in projects where people were relying on
this. Restore that functionality, but only as a fallback; matching the
Objective-C name is better than matching the Swift name.

rdar://problem/56681046

Author: jrose-apple

lib/ClangImporter/ClangImporter.cpp

@@ -3729,6 +3729,28 @@ EffectiveClangContext ClangImporter::Implementation::getEffectiveClangContext(
         /// FIXME: Other type declarations should also be okay?
       }
     }
+
+    // For source compatibility reasons, fall back to the Swift name.
+    //
+    // This is how people worked around not being able to import-as-member onto
+    // Swift types by their ObjC name before the above code to handle ObjCAttr
+    // was added.
+    if (name != nominal->getName())
+      clangName = exportName(nominal->getName());
+
+    lookupResult.clear();
+    lookupResult.setLookupName(clangName);
+    // FIXME: This loop is duplicated from above, but doesn't obviously factor
+    // out in a nice way.
+    if (sema.LookupName(lookupResult, /*Scope=*/nullptr)) {
+      // FIXME: Filter based on access path? C++ access control?
+      for (auto clangDecl : lookupResult) {
+        if (auto objcClass = dyn_cast<clang::ObjCInterfaceDecl>(clangDecl))
+          return EffectiveClangContext(objcClass);
+
+        /// FIXME: Other type declarations should also be okay?
+      }
+    }
   }
 
   return EffectiveClangContext();

lib/ClangImporter/ImportDecl.cpp

@@ -4537,54 +4537,77 @@ namespace {
       const auto &languageVersion =
           Impl.SwiftContext.LangOpts.EffectiveLanguageVersion;
 
-      auto isMatch = [&](const T *singleResult, bool baseNameMatches) -> bool {
+      auto isMatch = [&](const T *singleResult, bool baseNameMatches,
+                         bool allowObjCMismatch) -> bool {
         const DeclAttributes &attrs = singleResult->getAttrs();
 
         // Skip versioned variants.
         if (attrs.isUnavailableInSwiftVersion(languageVersion))
           return false;
 
-        // Skip if type not exposed to Objective-C.
-        // If the base name doesn't match, then a matching
-        // custom name in an @objc attribute is required.
-        if (baseNameMatches && !singleResult->isObjC())
-          return false;
-
-        // If Clang decl has a custom Swift name, then we know that
-        // `name` is the base name we're looking for.
-        if (hasKnownSwiftName)
-          return baseNameMatches;
+        // If Clang decl has a custom Swift name, then we know that the name we
+        // did direct lookup for is correct.
+        // 'allowObjCMismatch' shouldn't exist, but we need it for source
+        // compatibility where a previous version of the compiler didn't check
+        // @objc-ness at all.
+        if (hasKnownSwiftName || allowObjCMismatch) {
+          assert(baseNameMatches);
+          return allowObjCMismatch || singleResult->isObjC();
+        }
 
         // Skip if a different name is used for Objective-C.
         if (auto objcAttr = attrs.getAttribute<ObjCAttr>())
           if (auto objcName = objcAttr->getName())
             return objcName->getSimpleName() == name;
 
-        return baseNameMatches;
+        return baseNameMatches && singleResult->isObjC();
       };
 
       // First look at Swift types with the same name.
-      SmallVector<ValueDecl *, 4> results;
-      overlay->lookupValue(name, NLKind::QualifiedLookup, results);
+      SmallVector<ValueDecl *, 4> swiftDeclsByName;
+      overlay->lookupValue(name, NLKind::QualifiedLookup, swiftDeclsByName);
       T *found = nullptr;
-      for (auto result : results) {
+      for (auto result : swiftDeclsByName) {
         if (auto singleResult = dyn_cast<T>(result)) {
-          if (isMatch(singleResult, /*baseNameMatches=*/true)) {
+          if (isMatch(singleResult, /*baseNameMatches=*/true,
+                      /*allowObjCMismatch=*/false)) {
             if (found)
               return nullptr;
             found = singleResult;
           }
         }
       }
 
-      if (!found && !hasKnownSwiftName) {
+      if (!found && hasKnownSwiftName)
+        return nullptr;
+
+      if (!found) {
         // Try harder to find a match looking at just custom Objective-C names.
-        SmallVector<Decl *, 64> results;
-        overlay->getTopLevelDecls(results);
-        for (auto result : results) {
+        SmallVector<Decl *, 64> allTopLevelDecls;
+        overlay->getTopLevelDecls(allTopLevelDecls);
+        for (auto result : allTopLevelDecls) {
           if (auto singleResult = dyn_cast<T>(result)) {
             // The base name _could_ match but it's irrelevant here.
-            if (isMatch(singleResult, /*baseNameMatches=*/false)) {
+            if (isMatch(singleResult, /*baseNameMatches=*/false,
+                        /*allowObjCMismatch=*/false)) {
+              if (found)
+                return nullptr;
+              found = singleResult;
+            }
+          }
+        }
+      }
+
+      if (!found) {
+        // Go back to the first list and find classes with matching Swift names
+        // *even if the ObjC name doesn't match.*
+        // This shouldn't be allowed but we need it for source compatibility;
+        // people used `@class SwiftNameOfClass` as a workaround for not
+        // having the previous loop, and it "worked".
+        for (auto result : swiftDeclsByName) {
+          if (auto singleResult = dyn_cast<T>(result)) {
+            if (isMatch(singleResult, /*baseNameMatches=*/true,
+                        /*allowObjCMismatch=*/true)) {
               if (found)
                 return nullptr;
               found = singleResult;

test/ClangImporter/MixedSource/Inputs/import-as-member-swift.h

@@ -1,5 +1,14 @@
 @class Outer;
-
-struct Nested {
-  int value;
+struct NestedInOuter {
+  int a;
 } __attribute((swift_name("Outer.Nested")));
+
+@class OuterByObjCName_ObjC;
+struct NestedInOuterByObjCName {
+  int b;
+} __attribute((swift_name("OuterByObjCName_ObjC.Nested")));
+
+@class OuterBySwiftName_Swift;
+struct NestedInOuterBySwiftName {
+  int c;
+} __attribute((swift_name("OuterBySwiftName_Swift.Nested")));

test/ClangImporter/MixedSource/Inputs/mixed-framework/Mixed.framework/Headers/Mixed.h

@@ -124,6 +124,9 @@ void consumeObjCForwardNativeTypeHasDifferentCustomNameClass(ObjCForwardNativeTy
 @class ForwardNativeTypeIsNonObjCClass;
 void consumeForwardNativeTypeIsNonObjCClass(ForwardNativeTypeIsNonObjCClass *_Nonnull obj);
 
+@class ForwardNativeTypeIsUnambiguouslyNonObjCClass;
+void consumeForwardNativeTypeIsUnambiguouslyNonObjCClass(ForwardNativeTypeIsUnambiguouslyNonObjCClass *_Nonnull obj);
+
 SWIFT_CLASS("BOGUS")
 @interface BogusClass
 @end

test/ClangImporter/MixedSource/Inputs/mixed-framework/Mixed.swift

@@ -98,3 +98,7 @@ public class ForwardNativeTypeIsNonObjCClass {
 public class SwiftForwardNativeTypeIsNonObjCClass {
     public init() {}
 }
+
+public class ForwardNativeTypeIsUnambiguouslyNonObjCClass {
+    public init() {}
+}

test/ClangImporter/MixedSource/import-as-member-swift.swift

@@ -2,4 +2,12 @@
 
 @objc internal class Outer {}
 
-_ = Outer.Nested()
+@objc(OuterByObjCName_ObjC)
+internal class OuterByObjCName_Swift {}
+
+@objc(OuterBySwiftName_ObjC)
+internal class OuterBySwiftName_Swift {}
+
+_ = Outer.Nested(a: 1)
+_ = OuterByObjCName_Swift.Nested(b: 2)
+_ = OuterBySwiftName_Swift.Nested(c: 3)

test/ClangImporter/MixedSource/import-mixed-framework.swift

@@ -60,3 +60,5 @@ consumeObjCForwardNativeTypeHasDifferentCustomNameClass(SwiftForwardNativeTypeHa
 
 consumeForwardNativeTypeIsNonObjCClass(SwiftForwardNativeTypeIsNonObjCClass())
 consumeForwardNativeTypeIsNonObjCClass(ForwardNativeTypeIsNonObjCClass()) // expected-error {{cannot convert value of type 'ForwardNativeTypeIsNonObjCClass' to expected argument type 'SwiftForwardNativeTypeIsNonObjCClass'}}
+
+consumeForwardNativeTypeIsUnambiguouslyNonObjCClass(ForwardNativeTypeIsUnambiguouslyNonObjCClass())