[SILGenConstructor] InitAccessors: Make sure that accessed fields are initialized before init accessors

lib/SILGen/SILGenConstructor.cpp

@@ -1502,87 +1502,109 @@ void SILGenFunction::emitMemberInitializationViaInitAccessor(
     B.createEndAccess(loc, selfRef.getValue(), /*aborted=*/false);
 }
 
+void SILGenFunction::emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl,
+                                           PatternBindingDecl *field,
+                                           SubstitutionMap substitutions) {
+  assert(!field->isStatic());
+
+  for (auto i : range(field->getNumPatternEntries())) {
+    auto init = field->getExecutableInit(i);
+    if (!init)
+      continue;
+
+    auto *varPattern = field->getPattern(i);
+
+    // Cleanup after this initialization.
+    FullExpr scope(Cleanups, varPattern);
+
+    // Get the type of the initialization result, in terms
+    // of the constructor context's archetypes.
+    auto resultType =
+        getInitializationTypeInContext(field->getDeclContext(), dc, varPattern);
+    AbstractionPattern origType = resultType.first;
+    CanType substType = resultType.second;
+
+    // Figure out what we're initializing.
+    auto memberInit = emitMemberInit(*this, selfDecl, varPattern);
+
+    // This whole conversion thing is about eliminating the
+    // paired orig-to-subst subst-to-orig conversions that
+    // will happen if the storage is at a different abstraction
+    // level than the constructor. When emitApply() is used
+    // to call the stored property initializer, it naturally
+    // wants to convert the result back to the most substituted
+    // abstraction level. To undo this, we use a converting
+    // initialization and rely on the peephole that optimizes
+    // out the redundant conversion.
+    SILType loweredResultTy;
+    SILType loweredSubstTy;
+
+    // A converting initialization isn't necessary if the member is
+    // a property wrapper. Though the initial value can have a
+    // reabstractable type, the result of the initialization is
+    // always the property wrapper type, which is never reabstractable.
+    bool needsConvertingInit = false;
+    auto *singleVar = varPattern->getSingleVar();
+    if (!(singleVar && singleVar->getOriginalWrappedProperty())) {
+      loweredResultTy = getLoweredType(origType, substType);
+      loweredSubstTy = getLoweredType(substType);
+      needsConvertingInit = loweredResultTy != loweredSubstTy;
+    }
+
+    if (needsConvertingInit) {
+      Conversion conversion =
+          Conversion::getSubstToOrig(origType, substType, loweredResultTy);
+
+      ConvertingInitialization convertingInit(conversion,
+                                              SGFContext(memberInit.get()));
+
+      emitAndStoreInitialValueInto(*this, varPattern, field, i, substitutions,
+                                   origType, substType, &convertingInit);
+
+      auto finalValue = convertingInit.finishEmission(
+          *this, varPattern, ManagedValue::forInContext());
+      if (!finalValue.isInContext())
+        finalValue.forwardInto(*this, varPattern, memberInit.get());
+    } else {
+      emitAndStoreInitialValueInto(*this, varPattern, field, i, substitutions,
+                                   origType, substType, memberInit.get());
+    }
+  }
+}
+
 void SILGenFunction::emitMemberInitializers(DeclContext *dc,
                                             VarDecl *selfDecl,
                                             NominalTypeDecl *nominal) {
   auto subs = getSubstitutionsForPropertyInitializer(dc, nominal);
 
+  llvm::SmallPtrSet<PatternBindingDecl *, 4> alreadyInitialized;
   for (auto member : nominal->getImplementationContext()->getAllMembers()) {
     // Find instance pattern binding declarations that have initializers.
     if (auto pbd = dyn_cast<PatternBindingDecl>(member)) {
       if (pbd->isStatic()) continue;
 
+      if (alreadyInitialized.count(pbd))
+        continue;
+
       // Emit default initialization for an init accessor property.
       if (auto *var = pbd->getSingleVar()) {
         if (var->hasInitAccessor()) {
+          auto initAccessor = var->getAccessor(AccessorKind::Init);
+
+          // Make sure that initializations for the accessed properties
+          // are emitted before the init accessor that uses them.
+          for (auto *property : initAccessor->getAccessedProperties()) {
+            auto *PBD = property->getParentPatternBinding();
+            if (alreadyInitialized.insert(PBD).second)
+              emitMemberInitializer(dc, selfDecl, PBD, subs);
+          }
+
           emitMemberInitializationViaInitAccessor(dc, selfDecl, pbd, subs);
           continue;
         }
       }
 
-      for (auto i : range(pbd->getNumPatternEntries())) {
-        auto init = pbd->getExecutableInit(i);
-        if (!init) continue;
-
-        auto *varPattern = pbd->getPattern(i);
-
-        // Cleanup after this initialization.
-        FullExpr scope(Cleanups, varPattern);
-
-        // Get the type of the initialization result, in terms
-        // of the constructor context's archetypes.
-        auto resultType = getInitializationTypeInContext(
-            pbd->getDeclContext(), dc, varPattern);
-        AbstractionPattern origType = resultType.first;
-        CanType substType = resultType.second;
-
-        // Figure out what we're initializing.
-        auto memberInit = emitMemberInit(*this, selfDecl, varPattern);
-
-        // This whole conversion thing is about eliminating the
-        // paired orig-to-subst subst-to-orig conversions that
-        // will happen if the storage is at a different abstraction
-        // level than the constructor. When emitApply() is used
-        // to call the stored property initializer, it naturally
-        // wants to convert the result back to the most substituted
-        // abstraction level. To undo this, we use a converting
-        // initialization and rely on the peephole that optimizes
-        // out the redundant conversion.
-        SILType loweredResultTy;
-        SILType loweredSubstTy;
-
-        // A converting initialization isn't necessary if the member is
-        // a property wrapper. Though the initial value can have a
-        // reabstractable type, the result of the initialization is
-        // always the property wrapper type, which is never reabstractable.
-        bool needsConvertingInit = false;
-        auto *singleVar = varPattern->getSingleVar();
-        if (!(singleVar && singleVar->getOriginalWrappedProperty())) {
-          loweredResultTy = getLoweredType(origType, substType);
-          loweredSubstTy = getLoweredType(substType);
-          needsConvertingInit = loweredResultTy != loweredSubstTy;
-        }
-
-        if (needsConvertingInit) {
-          Conversion conversion = Conversion::getSubstToOrig(
-              origType, substType,
-              loweredResultTy);
-
-          ConvertingInitialization convertingInit(conversion,
-                                                  SGFContext(memberInit.get()));
-
-          emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
-                                       origType, substType, &convertingInit);
-
-          auto finalValue = convertingInit.finishEmission(
-              *this, varPattern, ManagedValue::forInContext());
-          if (!finalValue.isInContext())
-            finalValue.forwardInto(*this, varPattern, memberInit.get());
-        } else {
-          emitAndStoreInitialValueInto(*this, varPattern, pbd, i, subs,
-                                       origType, substType, memberInit.get());
-        }
-      }
+      emitMemberInitializer(dc, selfDecl, pbd, subs);
     }
   }
 }

lib/SILGen/SILGenFunction.h

@@ -802,6 +802,17 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   void emitMemberInitializers(DeclContext *dc, VarDecl *selfDecl,
                               NominalTypeDecl *nominal);
 
+  /// Generates code to initialize stored property from its
+  /// initializer.
+  ///
+  /// \param dc The DeclContext containing the current function.
+  /// \param selfDecl The 'self' declaration within the current function.
+  /// \param field The stored property that has to be initialized.
+  /// \param substitutions The substitutions to apply to initializer and setter.
+  void emitMemberInitializer(DeclContext *dc, VarDecl *selfDecl,
+                             PatternBindingDecl *field,
+                             SubstitutionMap substitutions);
+
   void emitMemberInitializationViaInitAccessor(DeclContext *dc,
                                                VarDecl *selfDecl,
                                                PatternBindingDecl *member,

test/SILOptimizer/init_accessor_raw_sil_lowering.swift

@@ -2,6 +2,115 @@
 
 // REQUIRES: asserts
 
+protocol Storage<T> {
+  associatedtype T
+
+  func getValue<V>(_: KeyPath<T, V>) -> V
+  func setValue<V>(_: KeyPath<T, V>, _: V)
+}
+
+struct NoopStorage<T>: Storage {
+  init() {}
+
+  func getValue<V>(_: KeyPath<T, V>) -> V { fatalError() }
+  func setValue<V>(_: KeyPath<T, V>, _: V) {}
+}
+
+final class TestIndirectionThroughStorage {
+  var name: String = "item1" {
+    @storageRestrictions(accesses: _storage)
+    init {
+      _storage.setValue(\.name, newValue)
+    }
+    get { _storage.getValue(\.name) }
+    set { }
+  }
+
+  var age: Int? = nil {
+    @storageRestrictions(accesses: _storage)
+    init {
+      _storage.setValue(\.age, newValue)
+    }
+
+    get { _storage.getValue(\.age) }
+    set { }
+  }
+
+  private var _storage: any Storage<TestIndirectionThroughStorage> = NoopStorage()
+
+  var storage: any Storage<TestIndirectionThroughStorage> {
+    get { _storage }
+    set { _storage = newValue }
+  }
+
+  // CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering29TestIndirectionThroughStorageC4name3ageACSS_Sitcfc : $@convention(method) (@owned String, Int, @owned TestIndirectionThroughStorage) -> @owned TestIndirectionThroughStorage
+  // CHECK: [[STORAGE_REF:%.*]] = ref_element_addr {{.*}} : $TestIndirectionThroughStorage, #TestIndirectionThroughStorage._storage
+  // CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
+  // CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
+  // Initialization:
+  // CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
+  // CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> (
+  // Explicit set:
+  // CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
+  // CHECK: assign_or_init [set] self %2 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> (
+  init(name: String, age: Int) {
+    self.name = name
+    self.age = age
+  }
+
+  // CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering29TestIndirectionThroughStorageC7storageAcA0H0_pAC1TAaEPRts_XP_tcfc : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @owned TestIndirectionThroughStorage) -> @owned TestIndirectionThroughStorage
+  // CHECK: [[STORAGE_REF:%.*]] = ref_element_addr {{.*}} : $TestIndirectionThroughStorage, #TestIndirectionThroughStorage._storage
+  // CHECK: [[STORAGE_INIT:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC8_storage33_DE106275C2F16FB3D05881E72FBD87C8LLAA0H0_pAC1TAaFPRts_XPvpfi : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
+  // CHECK-NEXT: {{.*}} = apply [[STORAGE_INIT]]([[STORAGE_REF]]) : $@convention(thin) () -> @out any Storage<TestIndirectionThroughStorage>
+  // Initialization:
+  // CHECK: assign_or_init [set] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $String, init {{.*}} : $@convention(thin) (@owned String, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (@owned String) -> ()
+  // CHECK: assign_or_init [set] self %1 : $TestIndirectionThroughStorage, value {{.*}} : $Optional<Int>, init {{.*}} : $@convention(thin) (Optional<Int>, @inout any Storage<TestIndirectionThroughStorage>) -> (), set {{.*}} : $@callee_guaranteed (Optional<Int>) -> ()
+  // Explicit set:
+  // CHECK: [[STORAGE_SETTER:%.*]] = function_ref @$s23assign_or_init_lowering29TestIndirectionThroughStorageC7storageAA0H0_pAC1TAaEPRts_XPvs : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
+  // CHECK-NEXT: {{.*}} = apply [[STORAGE_SETTER]]({{.*}}, %1) : $@convention(method) (@in any Storage<TestIndirectionThroughStorage>, @guaranteed TestIndirectionThroughStorage) -> ()
+  init(storage: any Storage<TestIndirectionThroughStorage>) {
+    self.storage = storage
+  }
+}
+
+struct TestAccessOfOnePatternVars {
+  var data: (Int, String) = (0, "a") {
+    @storageRestrictions(accesses: x, y)
+    init {
+    }
+
+    get { (x, y) }
+    set {
+      x = newValue.0
+      y = newValue.1
+    }
+  }
+
+  var other: Bool = false {
+    @storageRestrictions(accesses: x)
+    init {}
+    get { x != 0 }
+    set {}
+  }
+
+  var x: Int = 1, y: String = ""
+
+  // CHECK-LABEL: sil hidden [ossa] @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1x1yACSi_SStcfC : $@convention(method) (Int, @owned String, @thin TestAccessOfOnePatternVars.Type) -> @owned TestAccessOfOnePatternVars
+  // CHECK: [[X_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.x
+  // CHECK: [[X_INIT:%.*]] = function_ref @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1xSivpfi : $@convention(thin) () -> Int
+  // CHECK-NEXT: {{.*}} = apply [[X_INIT]]() : $@convention(thin) () -> Int
+  // CHECK: [[Y_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.y
+  // CHECK: [[Y_INIT:%.*]] = function_ref @$s23assign_or_init_lowering26TestAccessOfOnePatternVarsV1ySSvpfi : $@convention(thin) () -> @owned String
+  // CHECK-NEXT: {{.*}} = apply [[Y_INIT]]() : $@convention(thin) () -> @owned String
+  // CHECK-NOT: [[X_REF:%.*]] = struct_element_addr %3 : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.x
+  // CHECK-NOT: [[Y_REF:%.*]] = struct_element_addr {{.*}} : $*TestAccessOfOnePatternVars, #TestAccessOfOnePatternVars.y
+  // CHECK: assign_or_init [set] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $(Int, String), init {{.*}} : $@convention(thin) (Int, @owned String, @inout Int, @inout String) -> (), set {{.*}} : $@callee_guaranteed (Int, @owned String) -> ()
+  // CHECK: assign_or_init [set] self {{.*}} : $*TestAccessOfOnePatternVars, value {{.*}} : $Bool, init {{.*}} : $@convention(thin) (Bool, @inout Int) -> (), set {{.*}} : $@callee_guaranteed (Bool) -> ()
+  init(x: Int, y: String) {
+    self.x = x
+    self.y = y
+  }
+}
 
 struct Test1 {
   var _a: Int