Merge pull request #29561 from CodaFi/an-air-of-refinement

[ClangImporter] Redeclarations as Refinements

Author: CodaFi

lib/ClangImporter/ImportDecl.cpp

@@ -2208,6 +2208,50 @@ namespace {
     return {nullptr, foundMethod};
   }
 
+  // Attempt to identify the redeclaration of a property.
+  //
+  // Note that this function does not perform any additional member loading and
+  // is therefore subject to the relativistic effects of module import order.
+  // That is, suppose that a Clang Module and an Overlay module are in play.
+  // Depending on which module loads members first, a redeclaration point may
+  // or may not be identifiable.
+  VarDecl *
+  identifyPropertyRedeclarationPoint(ClangImporter::Implementation &Impl,
+                                     const clang::ObjCPropertyDecl *decl,
+                                     ClassDecl *subject, Identifier name) {
+    llvm::SetVector<Decl *> lookup;
+    // First, pull in all available members of the base class so we can catch
+    // redeclarations of APIs that are refined for Swift.
+    auto currentMembers = subject->getCurrentMembersWithoutLoading();
+    lookup.insert(currentMembers.begin(), currentMembers.end());
+
+    // Now pull in any just-imported members from the overrides table.
+    auto foundNames = Impl.MembersForNominal.find(subject);
+    if (foundNames != Impl.MembersForNominal.end()) {
+      auto foundDecls = foundNames->second.find(name);
+      if (foundDecls != foundNames->second.end()) {
+        lookup.insert(foundDecls->second.begin(), foundDecls->second.end());
+      }
+    }
+
+    for (auto *result : lookup) {
+      auto *var = dyn_cast<VarDecl>(result);
+      if (!var)
+        continue;
+
+      if (var->isInstanceMember() != decl->isInstanceProperty())
+        continue;
+
+      // If the selectors of the getter match in Objective-C, we have a
+      // redeclaration.
+      if (var->getObjCGetterSelector() ==
+          Impl.importSelector(decl->getGetterName())) {
+        return var;
+      }
+    }
+    return nullptr;
+  }
+
   /// Convert Clang declarations into the corresponding Swift
   /// declarations.
   class SwiftDeclConverter
@@ -5094,12 +5138,21 @@ namespace {
               overrideContext->getSelfNominalTypeDecl()
                 == dc->getSelfNominalTypeDecl()) {
             // We've encountered a redeclaration of the property.
-            // HACK: Just update the original declaration instead of importing a
-            // second property.
             handlePropertyRedeclaration(overridden, decl);
             return nullptr;
           }
         }
+
+        // Try searching the class for a property redeclaration. We can use
+        // the redeclaration to refine the already-imported property with a
+        // setter and also cut off any double-importing behavior.
+        auto *redecl
+            = identifyPropertyRedeclarationPoint(Impl, decl,
+                                                 dc->getSelfClassDecl(), name);
+        if (redecl) {
+          handlePropertyRedeclaration(redecl, decl);
+          return nullptr;
+        }
       }
 
       auto importedType = Impl.importPropertyType(decl, isInSystemModule(dc));

test/ClangImporter/Inputs/frameworks/CategoryOverrides.framework/Headers/CategoryOverrides.h

@@ -32,3 +32,19 @@ typedef enum {
 @interface Refinery : Base
 @property (nonatomic, readonly) RefinedSugar sugar /*NS_REFINED_FOR_SWIFT*/ __attribute__((swift_private));
 @end
+
+@protocol NullableProtocol
+@property (nonatomic, readonly, nullable) Base *requirement;
+@end
+
+@protocol NonNullProtocol <NullableProtocol>
+@property (nonatomic, readonly, nonnull) Base *requirement;
+@end
+
+@protocol ReadonlyProtocol
+@property (nonatomic, readonly) int answer;
+@end
+
+@protocol ReadwriteProtocol <ReadonlyProtocol>
+@property (nonatomic, readwrite) int answer;
+@end

test/ClangImporter/Inputs/frameworks/CategoryOverrides.framework/PrivateHeaders/Private.h

@@ -18,3 +18,9 @@
 @interface Refinery ()
 @property (nonatomic, readwrite) RefinedSugar sugar;
 @end
+
+@interface MyBaseClass () <NonNullProtocol>
+@end
+
+@interface MyDerivedClass () <ReadwriteProtocol>
+@end

test/ClangImporter/objc_redeclared_properties_categories.swift

@@ -11,7 +11,6 @@ import CategoryOverrides
 
 // Nail down some emergent behaviors of the Clang Importer's override checking:
 
-
 // A category declared in a (private) header can happen to double-import a property
 // and a function with the same name - both before and after omit-needless-words -
 // as long as they have different contextual types.
@@ -78,3 +77,20 @@ func takesARefinery(_ x: Refinery) {
   // CHECK: cannot assign to property: 'sugar' is a get-only property
   x.sugar = .caster
 }
+
+func nullabilityRefinementProto(_ x: MyBaseClass) {
+  // CHECK-PUBLIC: has no member 'requirement'
+  // CHECK-PRIVATE-NOT: has no member 'requirement'
+  // CHECK-PRIVATE-NOT: value of optional type 'Base?'
+  let _ : Base = x.requirement
+}
+
+func readwriteRefinementProto(_ x: MyDerivedClass) {
+  // CHECK-PUBLIC: has no member 'answer'
+  // CHECK-PRIVATE-NOT: has no member 'answer'
+  if x.answer == 0 {
+    // CHECK-PUBLIC: has no member 'answer'
+    // CHECK-PRIVATE-NOT: has no member 'answer'
+    x.answer = 42
+  }
+}