Merge pull request #6175 from jrose-apple/bad-property-redeclaration

[ClangImporter] Ignore redeclared properties with different types

rdar://problem/29422993

Author: jrose-apple

lib/AST/ASTVerifier.cpp

@@ -1735,6 +1735,58 @@ struct ASTNodeBase {};
         var->getType().print(Out);
         abort();
       }
+
+      Type typeForAccessors =
+          var->getInterfaceType()->getReferenceStorageReferent();
+      typeForAccessors =
+          ArchetypeBuilder::mapTypeIntoContext(var->getDeclContext(),
+                                               typeForAccessors);
+      if (const FuncDecl *getter = var->getGetter()) {
+        if (getter->getParameterLists().back()->size() != 0) {
+          Out << "property getter has parameters\n";
+          abort();
+        }
+        Type getterResultType = getter->getResultInterfaceType();
+        getterResultType =
+            ArchetypeBuilder::mapTypeIntoContext(var->getDeclContext(),
+                                                 getterResultType);
+        if (!getterResultType->isEqual(typeForAccessors)) {
+          Out << "property and getter have mismatched types: '";
+          typeForAccessors.print(Out);
+          Out << "' vs. '";
+          getterResultType.print(Out);
+          Out << "'\n";
+          abort();
+        }
+      }
+
+      if (const FuncDecl *setter = var->getSetter()) {
+        if (!setter->getResultInterfaceType()->isVoid()) {
+          Out << "property setter has non-Void result type\n";
+          abort();
+        }
+        if (setter->getParameterLists().back()->size() == 0) {
+          Out << "property setter has no parameters\n";
+          abort();
+        }
+        if (setter->getParameterLists().back()->size() != 1) {
+          Out << "property setter has 2+ parameters\n";
+          abort();
+        }
+        const ParamDecl *param = setter->getParameterLists().back()->get(0);
+        Type paramType = param->getInterfaceType();
+        paramType = ArchetypeBuilder::mapTypeIntoContext(var->getDeclContext(),
+                                                         paramType);
+        if (!paramType->isEqual(typeForAccessors)) {
+          Out << "property and setter param have mismatched types: '";
+          typeForAccessors.print(Out);
+          Out << "' vs. '";
+          paramType.print(Out);
+          Out << "'\n";
+          abort();
+        }
+      }
+
       verifyCheckedBase(var);
     }
 

lib/ClangImporter/ImportDecl.cpp

@@ -4023,6 +4023,16 @@ namespace {
       if (!setter)
         return;
 
+      // Check that the redeclared property's setter uses the same type as the
+      // original property. Objective-C can get away with the types being
+      // different (usually in something like nullability), but for Swift it's
+      // an AST invariant that's assumed and asserted elsewhere. If the type is
+      // different, just drop the setter, and leave the property as get-only.
+      assert(setter->getParameterLists().back()->size() == 1);
+      const ParamDecl *param = setter->getParameterLists().back()->get(0);
+      if (!param->getInterfaceType()->isEqual(original->getInterfaceType()))
+        return;
+
       original->setComputedSetter(setter);
     }
 

lib/Sema/TypeCheckDecl.cpp

@@ -4524,7 +4524,12 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     auto typeRepr = func->getBodyResultTypeLoc().getTypeRepr();
     if (!typeRepr)
       return false;
-      
+
+    // 'Self' on a property accessor is not dynamic 'Self'...even on a read-only
+    // property. We could implement it as such in the future.
+    if (func->isAccessor())
+      return false;
+
     return checkDynamicSelfReturn(func, typeRepr, 0);
   }
 

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RPFirst.h

@@ -0,0 +1,12 @@
+@interface RPFoo
+@property (readonly, nonnull) int *nonnullToNullable;
+@property (readonly, nullable) int *nullableToNonnull;
+@property (readonly, nonnull) id typeChangeMoreSpecific;
+@property (readonly, nonnull) RPFoo *typeChangeMoreGeneral;
+
+@property (readonly, nonnull) id accessorRedeclaredAsNullable;
+- (nullable id)accessorRedeclaredAsNullable;
+
+- (nullable id)accessorDeclaredFirstAsNullable;
+@property (readonly, nonnull) id accessorDeclaredFirstAsNullable;
+@end

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RPSecond.h

@@ -0,0 +1,6 @@
+@interface RPFoo ()
+@property (readwrite, nullable) int *nonnullToNullable;
+@property (readwrite, nonnull) int *nullableToNonnull;
+@property (readwrite, nonnull) RPFoo *typeChangeMoreSpecific;
+@property (readwrite, nonnull) id typeChangeMoreGeneral;
+@end

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

@@ -0,0 +1,2 @@
+#import "RPFirst.h"
+#import "RPSecond.h"

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RedeclaredPropertiesSplit.h

@@ -0,0 +1 @@
+#import "RPFirst.h"

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RedeclaredPropertiesSplit2.h

@@ -0,0 +1,2 @@
+#import "RedeclaredPropertiesSplit.h"
+#import "RPSecond.h"

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RedeclaredPropertiesSub.h

@@ -0,0 +1 @@
+#import "RPFirst.h"

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/RedeclaredPropertiesSubPrivate.h

@@ -0,0 +1,2 @@
+#import "RedeclaredPropertiesSub.h"
+#import "RPSecond.h"

test/ClangImporter/Inputs/custom-modules/RedeclaredProperties/module.modulemap

@@ -0,0 +1,22 @@
+module RedeclaredPropertiesTextual {
+  textual header "RPFirst.h"
+  textual header "RPSecond.h"
+}
+
+module RedeclaredProperties {
+  header "RedeclaredProperties.h"
+}
+
+module RedeclaredPropertiesSub {
+  header "RedeclaredPropertiesSub.h"
+  explicit module Private {
+    header "RedeclaredPropertiesSubPrivate.h"
+  }
+}
+
+module RedeclaredPropertiesSplit {
+  header "RedeclaredPropertiesSplit.h"
+}
+module RedeclaredPropertiesSplit2 {
+  header "RedeclaredPropertiesSplit2.h"
+}

test/ClangImporter/objc_redeclared_properties.swift

@@ -0,0 +1,32 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/custom-modules -D ONE_MODULE %s -verify
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/custom-modules -D SUB_MODULE %s -verify
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck -I %S/Inputs/custom-modules -D TWO_MODULES %s -verify
+
+// REQUIRES: objc_interop
+
+#if ONE_MODULE
+import RedeclaredProperties
+#elseif SUB_MODULE
+import RedeclaredPropertiesSub
+import RedeclaredPropertiesSub.Private
+#elseif TWO_MODULES
+import RedeclaredPropertiesSplit
+import RedeclaredPropertiesSplit2
+#endif
+
+func test(obj: RPFoo) {
+  if let _ = obj.nonnullToNullable {} // expected-error {{initializer for conditional binding must have Optional type}}
+  obj.nonnullToNullable = obj // expected-error {{cannot assign to property: 'nonnullToNullable' is a get-only property}}
+
+  if let _ = obj.nullableToNonnull {} // okay
+  obj.nullableToNonnull = obj // expected-error {{cannot assign to property: 'nullableToNonnull' is a get-only property}}
+
+  let _: RPFoo = obj.typeChangeMoreSpecific // expected-error {{cannot convert value of type 'Any' to specified type 'RPFoo'}}
+  obj.typeChangeMoreSpecific = obj // expected-error {{cannot assign to property: 'typeChangeMoreSpecific' is a get-only property}}
+
+  let _: RPFoo = obj.typeChangeMoreGeneral
+  obj.typeChangeMoreGeneral = obj // expected-error {{cannot assign to property: 'typeChangeMoreGeneral' is a get-only property}}
+
+  if let _ = obj.accessorRedeclaredAsNullable {} // expected-error {{initializer for conditional binding must have Optional type}}
+  if let _ = obj.accessorDeclaredFirstAsNullable {} // expected-error {{initializer for conditional binding must have Optional type}}
+}