Merge pull request #24390 from DougGregor/property-delegates-private-memberwise-inits

 Prevent less-visible properties with delegates from affecting memberwise initializer

Author: DougGregor

lib/AST/Decl.cpp

@@ -5316,6 +5316,18 @@ bool VarDecl::isMemberwiseInitialized(bool preferDeclaredProperties) const {
   if (isLet() && isParentInitialized())
     return false;
 
+  // Properties with attached delegates that have an access level < internal
+  // but do have an initializer don't participate in the memberwise
+  // initializer, because they would arbitrarily lower the access of the
+  // memberwise initializer.
+  auto origVar = this;
+  if (auto origDelegate = getOriginalDelegatedProperty())
+    origVar = origDelegate;
+  if (origVar->getFormalAccess() < AccessLevel::Internal &&
+      origVar->getAttachedPropertyDelegate() &&
+      origVar->isParentInitialized())
+    return false;
+
   return true;
 }
 

lib/SILGen/SILGenConstructor.cpp

@@ -177,32 +177,31 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
                                       fieldTy.getAddressType());
       InitializationPtr init(new KnownAddressInitialization(slot));
 
-      // An initialized 'let' property has a single value specified by the
-      // initializer - it doesn't come from an argument.
-      if (!field->isStatic() && field->isLet() &&
-          field->getParentInitializer()) {
+      // If it's memberwise initialized, do so now.
+      if (field->isMemberwiseInitialized(/*preferDeclaredProperties=*/false)) {
+        assert(elti != eltEnd &&
+               "number of args does not match number of fields");
+        (void)eltEnd;
+        FullExpr scope(SGF.Cleanups, field->getParentPatternBinding());
+        if (!maybeEmitPropertyDelegateInitFromValue(
+              SGF, Loc, field, std::move(*elti),
+              [&](Expr *expr) {
+                SGF.emitExprInto(expr, init.get());
+              })) {
+          std::move(*elti).forwardInto(SGF, Loc, init.get());
+        }
+        ++elti;
+      } else {
 #ifndef NDEBUG
-        assert(field->getType()->isEqual(field->getParentInitializer()->getType())
-               && "Checked by sema");
+        assert(
+            field->getType()->isEqual(field->getParentInitializer()->getType())
+              && "Checked by sema");
 #endif
 
         // Cleanup after this initialization.
         FullExpr scope(SGF.Cleanups, field->getParentPatternBinding());
         SGF.emitExprInto(field->getParentInitializer(), init.get());
-        continue;
       }
-
-      assert(elti != eltEnd && "number of args does not match number of fields");
-      (void)eltEnd;
-      FullExpr scope(SGF.Cleanups, field->getParentPatternBinding());
-      if (!maybeEmitPropertyDelegateInitFromValue(
-            SGF, Loc, field, std::move(*elti),
-            [&](Expr *expr) {
-              SGF.emitExprInto(expr, init.get());
-            })) {
-        std::move(*elti).forwardInto(SGF, Loc, init.get());
-      }
-      ++elti;
     }
     SGF.B.createReturn(ImplicitReturnLocation::getImplicitReturnLoc(Loc),
                        SGF.emitEmptyTuple(Loc));

lib/Sema/CodeSynthesis.cpp

@@ -1584,6 +1584,9 @@ PropertyDelegateBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
   if (parentPBD->isInitialized(patternNumber) &&
       !parentPBD->isInitializerChecked(patternNumber)) {
     auto &tc = *static_cast<TypeChecker *>(ctx.getLazyResolver());
+    if (!var->hasType())
+      tc.validateDecl(var);
+
     tc.typeCheckPatternBinding(parentPBD, patternNumber);
   }
 

test/SILGen/property_delegates.swift

@@ -103,15 +103,9 @@ func forceHasMemberwiseInit() {
 
 // CHECK: return
 
-// CHECK-LABEL: sil private [ossa] @$s18property_delegates9HasNestedV1yACyxGSayxG_tc33_
-// CHECK: [[STACK_SLOT:%.*]] = alloc_stack $HasNested<T>.PrivateDelegate<Array<T>>
-// CHECK: [[METATYPE:%.*]] = metatype $@thin HasNested<T>.PrivateDelegate<Array<T>>.Type
-// CHECK: [[ARRAY_STACK_SLOT:%.*]] = alloc_stack $Array<T>
-// CHECK: store %0 to [init] [[ARRAY_STACK_SLOT]] : $*Array<T>
-// CHECK: [[INIT:%.*]] = function_ref @$s18property_delegates9HasNestedV15PrivateDelegate{{.*}}initialValue
-// CHECK: [[DELEGATE_INSTANCE:%.*]] = apply [[INIT]]<T, [T]>([[STACK_SLOT]], [[ARRAY_STACK_SLOT]], [[METATYPE]])
-// CHECK: [[DELEGATE_VALUE:%.*]] = load [take] [[STACK_SLOT]] : $*HasNested<T>.PrivateDelegate<Array<T>>
-// CHECK: struct $HasNested<T> ([[DELEGATE_VALUE]] : $HasNested<T>.PrivateDelegate<Array<T>>)
+// CHECK-LABEL: sil hidden [transparent] [ossa] @$s18property_delegates9HasNestedV2$y33_4EEAE04FCFB319F460A84BF3BE29C303LLAC15PrivateDelegateAELLVyx_SayxGGvpfi : $@convention(thin) <T> () -> @owned Array<T> {
+// CHECK: bb0:
+// CHECK: function_ref @$ss27_allocateUninitializedArrayySayxG_BptBwlF
 struct HasNested<T> {
   @_propertyDelegate
   private struct PrivateDelegate<U> {
@@ -125,7 +119,7 @@ struct HasNested<T> {
   private var y: [T] = []
 
   static func blah(y: [T]) -> HasNested<T> {
-    return HasNested<T>(y: y)
+    return HasNested<T>()
   }
 }
 

test/decl/var/property_delegates.swift

@@ -610,6 +610,24 @@ func testDefaultInitializers() {
   _ = NoDefaultInitializerStruct() // expected-error{{missing argument for parameter 'x' in call}}
 }
 
+struct DefaultedPrivateMemberwiseLets {
+  @Wrapper(value: true)
+  private var x: Bool
+
+  @WrapperWithInitialValue
+  var y: Int = 17
+
+  @WrapperWithInitialValue(initialValue: 17)
+  private var z: Int
+}
+
+func testDefaultedPrivateMemberwiseLets() {
+  _ = DefaultedPrivateMemberwiseLets()
+  _ = DefaultedPrivateMemberwiseLets(y: 42)
+  _ = DefaultedPrivateMemberwiseLets(x: Wrapper(value: false)) // expected-error{{incorrect argument label in call (have 'x:', expected 'y:')}}
+}
+
+
 // ---------------------------------------------------------------------------
 // Storage references
 // ---------------------------------------------------------------------------