Merge pull request #34255 from slavapestov/reabstract-stored-property-constrained-extension-mess

SILGen: Fix crash when a stored property with an initial value has a reabstractable type

Author: slavapestov

lib/SILGen/Initialization.h

@@ -307,6 +307,39 @@ class TupleInitialization : public Initialization {
   void finishUninitialized(SILGenFunction &SGF) override;
 };
 
+/// A "null" initialization that indicates that any value being initialized
+/// into this initialization should be discarded. This represents AnyPatterns
+/// (that is, 'var (_)') that bind to values without storing them.
+class BlackHoleInitialization : public Initialization {
+public:
+  BlackHoleInitialization() {}
+
+  bool canSplitIntoTupleElements() const override {
+    return true;
+  }
+  
+  MutableArrayRef<InitializationPtr>
+  splitIntoTupleElements(SILGenFunction &SGF, SILLocation loc,
+                         CanType type,
+                         SmallVectorImpl<InitializationPtr> &buf) override {
+    // "Destructure" an ignored binding into multiple ignored bindings.
+    for (auto fieldType : cast<TupleType>(type)->getElementTypes()) {
+      (void) fieldType;
+      buf.push_back(InitializationPtr(new BlackHoleInitialization()));
+    }
+    return buf;
+  }
+
+  void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
+                           ManagedValue value, bool isInit) override {
+    /// This just ignores the provided value.
+  }
+
+  void finishUninitialized(SILGenFunction &SGF) override {
+    // do nothing
+  }
+};
+
 } // end namespace Lowering
 } // end namespace swift
 

lib/SILGen/SILGenConstructor.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ArgumentSource.h"
+#include "Conversion.h"
 #include "Initialization.h"
 #include "LValue.h"
 #include "RValue.h"
@@ -901,72 +902,71 @@ static ManagedValue emitSelfForMemberInit(SILGenFunction &SGF, SILLocation loc,
                                          SGFAccessKind::Write);
 }
 
-static LValue emitLValueForMemberInit(SILGenFunction &SGF, SILLocation loc,
-                                      VarDecl *selfDecl,
-                                      VarDecl *property) {
-  CanType selfFormalType = selfDecl->getType()->getCanonicalType();
-  auto self = emitSelfForMemberInit(SGF, loc, selfDecl);
-  return SGF.emitPropertyLValue(loc, self, selfFormalType, property,
-                                LValueOptions(), SGFAccessKind::Write,
-                                AccessSemantics::DirectToStorage);
-}
-
-/// Emit a member initialization for the members described in the
-/// given pattern from the given source value.
-static void emitMemberInit(SILGenFunction &SGF, VarDecl *selfDecl,
-                           Pattern *pattern, RValue &&src) {
+// FIXME: Can emitMemberInit() share code with InitializationForPattern in
+// SILGenDecl.cpp? Note that this version operates on stored properties of
+// types, whereas the former only knows how to handle local bindings, but
+// we could generalize it.
+static InitializationPtr
+emitMemberInit(SILGenFunction &SGF, VarDecl *selfDecl, Pattern *pattern) {
   switch (pattern->getKind()) {
   case PatternKind::Paren:
     return emitMemberInit(SGF, selfDecl,
-                          cast<ParenPattern>(pattern)->getSubPattern(),
-                          std::move(src));
+                          cast<ParenPattern>(pattern)->getSubPattern());
 
   case PatternKind::Tuple: {
+    TupleInitialization *init = new TupleInitialization();
     auto tuple = cast<TuplePattern>(pattern);
-    auto fields = tuple->getElements();
-
-    SmallVector<RValue, 4> elements;
-    std::move(src).extractElements(elements);
-    for (unsigned i = 0, n = fields.size(); i != n; ++i) {
-      emitMemberInit(SGF, selfDecl, fields[i].getPattern(),
-                     std::move(elements[i]));
+    for (auto &elt : tuple->getElements()) {
+      init->SubInitializations.push_back(
+        emitMemberInit(SGF, selfDecl, elt.getPattern()));
     }
-    break;
+    return InitializationPtr(init);
   }
 
   case PatternKind::Named: {
     auto named = cast<NamedPattern>(pattern);
-    // Form the lvalue referencing this member.
-    FormalEvaluationScope scope(SGF);
-    LValue memberRef = emitLValueForMemberInit(SGF, pattern, selfDecl,
-                                               named->getDecl());
 
-    // Assign to it.
-    SGF.emitAssignToLValue(pattern, std::move(src), std::move(memberRef));
-    return;
+    auto self = emitSelfForMemberInit(SGF, pattern, selfDecl);
+
+    auto *field = named->getDecl();
+
+    auto selfTy = self.getType();
+    auto fieldTy =
+      selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
+    SILValue slot;
+
+    if (auto *structDecl = dyn_cast<StructDecl>(field->getDeclContext())) {
+      slot = SGF.B.createStructElementAddr(pattern, self.forward(SGF), field,
+                                           fieldTy.getAddressType());
+    } else {
+      assert(isa<ClassDecl>(field->getDeclContext()));
+      slot = SGF.B.createRefElementAddr(pattern, self.forward(SGF), field,
+                                        fieldTy.getAddressType());
+    }
+
+    return InitializationPtr(new KnownAddressInitialization(slot));
   }
 
   case PatternKind::Any:
-    return;
+    return InitializationPtr(new BlackHoleInitialization());;
 
   case PatternKind::Typed:
     return emitMemberInit(SGF, selfDecl,
-                          cast<TypedPattern>(pattern)->getSubPattern(),
-                          std::move(src));
+                          cast<TypedPattern>(pattern)->getSubPattern());
 
   case PatternKind::Binding:
     return emitMemberInit(SGF, selfDecl,
-                          cast<BindingPattern>(pattern)->getSubPattern(),
-                          std::move(src));
+                          cast<BindingPattern>(pattern)->getSubPattern());
 
 #define PATTERN(Name, Parent)
 #define REFUTABLE_PATTERN(Name, Parent) case PatternKind::Name:
 #include "swift/AST/PatternNodes.def"
-    llvm_unreachable("Refutable pattern in pattern binding");
+    llvm_unreachable("Refutable pattern in stored property pattern binding");
   }
 }
 
-static Type getInitializationTypeInContext(
+static std::pair<AbstractionPattern, CanType>
+getInitializationTypeInContext(
     DeclContext *fromDC, DeclContext *toDC,
     Pattern *pattern) {
   auto interfaceType = pattern->getType()->mapTypeOutOfContext();
@@ -981,9 +981,53 @@ static Type getInitializationTypeInContext(
     }
   }
 
-  auto resultType = toDC->mapTypeIntoContext(interfaceType);
+  AbstractionPattern origType(
+    fromDC->getGenericSignatureOfContext().getCanonicalSignature(),
+    interfaceType->getCanonicalType());
+
+  auto substType = toDC->mapTypeIntoContext(interfaceType)->getCanonicalType();
 
-  return resultType;
+  return std::make_pair(origType, substType);
+}
+
+static void
+emitAndStoreInitialValueInto(SILGenFunction &SGF,
+                             SILLocation loc,
+                             PatternBindingDecl *pbd, unsigned i,
+                             SubstitutionMap subs,
+                             AbstractionPattern origType,
+                             CanType substType,
+                             Initialization *init) {
+  bool injectIntoWrapper = false;
+  if (auto singleVar = pbd->getSingleVar()) {
+    auto originalVar = singleVar->getOriginalWrappedProperty();
+    if (originalVar &&
+        originalVar->isPropertyMemberwiseInitializedWithWrappedType()) {
+      injectIntoWrapper = true;
+    }
+  }
+
+  SGFContext C = (injectIntoWrapper ? SGFContext() : SGFContext(init));
+
+  RValue result = SGF.emitApplyOfStoredPropertyInitializer(
+                            pbd->getExecutableInit(i),
+                            pbd->getAnchoringVarDecl(i),
+                            subs, substType, origType, C);
+
+  // need to store result into the init if its in context
+
+  // If we have the backing storage for a property with an attached
+  // property wrapper initialized with `=`, inject the value into an
+  // instance of the wrapper.
+  if (injectIntoWrapper) {
+    auto *singleVar = pbd->getSingleVar();
+    result = maybeEmitPropertyWrapperInitFromValue(
+        SGF, pbd->getExecutableInit(i),
+        singleVar, subs, std::move(result));
+  }
+
+  if (!result.isInContext())
+    std::move(result).forwardInto(SGF, loc, init);
 }
 
 void SILGenFunction::emitMemberInitializers(DeclContext *dc,
@@ -1001,35 +1045,51 @@ void SILGenFunction::emitMemberInitializers(DeclContext *dc,
         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.
-        CanType resultType = getInitializationTypeInContext(
-            pbd->getDeclContext(), dc, varPattern)->getCanonicalType();
-        AbstractionPattern origResultType(resultType);
-
-        // FIXME: Can emitMemberInit() share code with
-        // InitializationForPattern in SILGenDecl.cpp?
-        RValue result = emitApplyOfStoredPropertyInitializer(
-                                  init, pbd->getAnchoringVarDecl(i), subs,
-                                  resultType, origResultType,
-                                  SGFContext());
-
-        // If we have the backing storage for a property with an attached
-        // property wrapper initialized with `=`, inject the value into an
-        // instance of the wrapper.
-        if (auto singleVar = pbd->getSingleVar()) {
-          auto originalVar = singleVar->getOriginalWrappedProperty();
-          if (originalVar &&
-              originalVar->isPropertyMemberwiseInitializedWithWrappedType()) {
-            result = maybeEmitPropertyWrapperInitFromValue(
-                *this, init, singleVar, subs, std::move(result));
-          }
+        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.
+        auto loweredResultTy = getLoweredType(origType, substType);
+        auto loweredSubstTy = getLoweredType(substType);
+
+        if (loweredResultTy != loweredSubstTy) {
+          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());
         }
-
-        emitMemberInit(*this, selfDecl, varPattern, std::move(result));
       }
     }
   }

lib/SILGen/SILGenConvert.cpp

@@ -1354,7 +1354,25 @@ Lowering::canPeepholeConversions(SILGenFunction &SGF,
   switch (outerConversion.getKind()) {
   case Conversion::OrigToSubst:
   case Conversion::SubstToOrig:
-    // TODO: peephole these when the abstraction patterns are the same!
+    switch (innerConversion.getKind()) {
+    case Conversion::OrigToSubst:
+    case Conversion::SubstToOrig:
+      if (innerConversion.getKind() == outerConversion.getKind())
+        break;
+
+      if (innerConversion.getReabstractionOrigType().getCachingKey() !=
+          outerConversion.getReabstractionOrigType().getCachingKey() ||
+          innerConversion.getReabstractionSubstType() !=
+          outerConversion.getReabstractionSubstType()) {
+        break;
+      }
+
+      return ConversionPeepholeHint(ConversionPeepholeHint::Identity, false);
+
+    default:
+      break;
+    }
+
     return None;
 
   case Conversion::AnyErasure:

lib/SILGen/SILGenDecl.cpp

@@ -39,41 +39,6 @@ using namespace Lowering;
 void Initialization::_anchor() {}
 void SILDebuggerClient::anchor() {}
 
-namespace {
-  /// A "null" initialization that indicates that any value being initialized
-  /// into this initialization should be discarded. This represents AnyPatterns
-  /// (that is, 'var (_)') that bind to values without storing them.
-  class BlackHoleInitialization : public Initialization {
-  public:
-    BlackHoleInitialization() {}
-
-    bool canSplitIntoTupleElements() const override {
-      return true;
-    }
-    
-    MutableArrayRef<InitializationPtr>
-    splitIntoTupleElements(SILGenFunction &SGF, SILLocation loc,
-                           CanType type,
-                           SmallVectorImpl<InitializationPtr> &buf) override {
-      // "Destructure" an ignored binding into multiple ignored bindings.
-      for (auto fieldType : cast<TupleType>(type)->getElementTypes()) {
-        (void) fieldType;
-        buf.push_back(InitializationPtr(new BlackHoleInitialization()));
-      }
-      return buf;
-    }
-
-    void copyOrInitValueInto(SILGenFunction &SGF, SILLocation loc,
-                             ManagedValue value, bool isInit) override {
-      /// This just ignores the provided value.
-    }
-
-    void finishUninitialized(SILGenFunction &SGF) override {
-      // do nothing
-    }
-  };
-} // end anonymous namespace
-
 static void copyOrInitValueIntoHelper(
     SILGenFunction &SGF, SILLocation loc, ManagedValue value, bool isInit,
     ArrayRef<InitializationPtr> subInitializations,
@@ -1290,7 +1255,7 @@ void SILGenFunction::emitStmtCondition(StmtCondition Cond, JumpDest FalseDest,
     switch (elt.getKind()) {
     case StmtConditionElement::CK_PatternBinding: {
       InitializationPtr initialization =
-      InitializationForPattern(*this, FalseDest).visit(elt.getPattern());
+        emitPatternBindingInitialization(elt.getPattern(), FalseDest);
 
       // Emit the initial value into the initialization.
       FullExpr Scope(Cleanups, CleanupLocation(elt.getInitializer()));

test/IRGen/generic_structs.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -disable-type-layout -primary-file %s -emit-ir | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize -DINT=i%target-ptrsize -DINT_32=i32
+// RUN: %target-swift-frontend -disable-type-layout -primary-file %s -emit-ir
 
 struct A<T1, T2>
 {
@@ -37,13 +37,3 @@ public struct GenericStruct<T : Proto> {
 
   public init() {}
 }
-
-// CHECK-LABEL: define{{.*}} swiftcc void @"$s15generic_structs13GenericStructVACyxGycfC"
-// CHECK:  [[T0:%.*]] = call swiftcc %swift.metadata_response @"$s15generic_structs13GenericStructVMa"([[INT]] 0, %swift.type* %T, i8** %T.Proto)
-// CHECK:  [[TYPE:%.*]] = extractvalue %swift.metadata_response [[T0]], 0
-// CHECK:  [[PTR:%.*]] = bitcast %swift.type* [[TYPE]] to [[INT_32]]*
-// CHECK:  [[FIELDOFFSET:%.*]] = getelementptr inbounds [[INT_32]], [[INT_32]]* [[PTR]], [[INT]] [[IDX:6|10]]
-// CHECK:  [[OFFSET:%.*]] = load [[INT_32]], [[INT_32]]* [[FIELDOFFSET]]
-// CHECK:  [[ADDROFOPT:%.*]] = getelementptr inbounds i8, i8* {{.*}}, [[INT_32]] [[OFFSET]]
-// CHECK:  [[OPTPTR:%.*]] = bitcast i8* [[ADDROFOPT]] to %TSq*
-// CHECK:  call %TSq* @"$sxSg15generic_structs5ProtoRzlWOb"(%TSq* {{.*}}, %TSq* [[OPTPTR]]

test/SILGen/constrained_extensions.swift

@@ -177,8 +177,8 @@ struct AnythingGoes<T> {
 extension AnythingGoes where T : VeryConstrained {
   // CHECK-LABEL: sil hidden [ossa] @$s22constrained_extensions12AnythingGoesVA2A15VeryConstrainedRzlE13fromExtensionACyxGyt_tcfC : $@convention(method) <T where T : VeryConstrained> (@thin AnythingGoes<T>.Type) -> @out AnythingGoes<T> {
 
+  // CHECK: [[RESULT:%.*]] = struct_element_addr {{%.*}} : $*AnythingGoes<T>, #AnythingGoes.meaningOfLife
   // CHECK: [[INIT:%.*]] = function_ref @$s22constrained_extensions12AnythingGoesV13meaningOfLifexSgvpfi : $@convention(thin) <τ_0_0> () -> @out Optional<τ_0_0>
-  // CHECK: [[RESULT:%.*]] = alloc_stack $Optional<T>
   // CHECK: apply [[INIT]]<T>([[RESULT]]) : $@convention(thin) <τ_0_0> () -> @out Optional<τ_0_0>
   // CHECK: return
   init(fromExtension: ()) {}