Teach optional injection to peephole into more converting contexts

This allows us to propagate abstraction patterns from optional parameters
all the way to closure emission, which optimizes some code patterns but
(more importantly) propagates our knowledge that we're converting to an
`@isolated(any)` function type down to closure emission, allowing us to
set the isolation correctly.

There are still some conversion cases missing --- if we can't combine
conversions for whatever reason, we should at least shift our knowledge
that we need to produce an `@isolated(any)` type down, the same way that
we shift it when emitting a closure that we can't directly emit in the
target abstraction pattern.  But this completes the obvious cases of
peepholing for closure emission.

Author: rjmccall

lib/SILGen/Conversion.h

@@ -24,8 +24,14 @@
 namespace swift {
 namespace Lowering {
 
+class OptionalInjectionConversion;
+
 /// An abstraction representing certain kinds of conversion that SILGen can
-/// do automatically in various situations.
+/// do automatically in various situations.  These are used primarily with
+/// ConvertingInitialization in order to guide the emission of an expression.
+/// Some of these conversions are semantically required, such as propagating
+/// @isolated(any) down to the emission of the function reference, or some of
+/// the bridging conversions.
 class Conversion {
 public:
   enum KindTy {
@@ -52,6 +58,13 @@ class Conversion {
     /// and that's easier.
     AnyErasure,
 
+    /// A subtype conversion, except it's allowed to do optional injections
+    /// and existential erasures.  This comes up with bridging peepholes
+    /// and is annoying to not have a way to represent.  The conversion
+    /// should always involve class references and so will be harmless
+    /// in terms of representations.
+    BridgingSubtype,
+
     /// A subtype conversion.
     Subtype,
 
@@ -68,6 +81,7 @@ class Conversion {
     case BridgeFromObjC:
     case BridgeResultFromObjC:
     case AnyErasure:
+    case BridgingSubtype:
     case Subtype:
       return true;
 
@@ -88,6 +102,7 @@ class Conversion {
     case BridgeFromObjC:
     case BridgeResultFromObjC:
     case AnyErasure:
+    case BridgingSubtype:
     case Subtype:
       return false;
     }
@@ -104,6 +119,16 @@ class Conversion {
     bool IsExplicit;
   };
 
+  /// The types we store for reabstracting contexts.  In general, when
+  /// we're just emitting an expression, it's expected that the input
+  /// abstraction type and lowered type will match the input formal type,
+  /// which will be the type of the expression we're emitting.  They can
+  /// therefore simply be replaced if we're e.g. prepending a subtype
+  /// conversion to the reabstraction.  But it's very useful to be able to
+  /// represent both sides of the conversion uniformly so that e.g. we can
+  /// elegantly perform a single (perhaps identity) reabstraction when
+  /// receiving a function result or loading a value from abstracted
+  /// storage.
   struct ReabstractionTypes {
     AbstractionPattern InputOrigType;
     AbstractionPattern OutputOrigType;
@@ -123,6 +148,7 @@ class Conversion {
     case BridgeFromObjC:
     case BridgeResultFromObjC:
     case AnyErasure:
+    case BridgingSubtype:
     case Subtype:
       return Members::indexOf<BridgingTypes>();
 
@@ -153,6 +179,33 @@ class Conversion {
                                                inputLoweredTy, outputLoweredTy);
   }
 
+  static bool isAllowedConversion(CanType inputType, CanType outputType) {
+    // Allow all identity conversions.  (This should only happen with
+    // reabstraction.)
+    if (inputType == outputType) return true;
+
+    // Allow optional-to-optional conversions, but not injections
+    // into optional.  Emitters can be expected to just strip optionality
+    // from the result type when peepholing through an optional injection,
+    // and doing so avoids the need to handle injections specially in
+    // emitters, like those for function references and closures.
+    while (auto outputObjectType = outputType.getOptionalObjectType()) {
+      auto inputObjectType = inputType.getOptionalObjectType();
+      if (!inputObjectType) return false;
+      outputType = outputObjectType;
+      inputType = inputObjectType;
+    }
+
+    // Disallow existential erasures from being directly represented here
+    // because it may involve a representation change for the value.  Emitters
+    // shouldn't have to specially recognize those.
+    if (outputType.isExistentialType())
+      return inputType.isExistentialType();
+
+    assert(!inputType.getOptionalObjectType());
+    return true;
+  }
+
 public:
   static Conversion getOrigToSubst(AbstractionPattern origType,
                                    CanType substType,
@@ -176,6 +229,8 @@ class Conversion {
                                   AbstractionPattern outputOrigType,
                                   CanType outputSubstType,
                                   SILType outputLoweredTy) {
+    assert(isAllowedConversion(inputSubstType, outputSubstType) &&
+           "don't build subtype conversions that do existential erasures");
     return Conversion(inputOrigType, inputSubstType, inputLoweredTy,
                       outputOrigType, outputSubstType, outputLoweredTy);
   }
@@ -184,6 +239,8 @@ class Conversion {
                                 CanType resultType, SILType loweredResultTy,
                                 bool isExplicit = false) {
     assert(isBridgingKind(kind));
+    assert((kind != Subtype || isAllowedConversion(origType, resultType)) &&
+           "disallowed conversion for subtype relationship");
     return Conversion(kind, origType, resultType, loweredResultTy, isExplicit);
   }
 
@@ -274,6 +331,8 @@ class Conversion {
   /// this conversion.
   std::optional<Conversion> adjustForInitialForceValue() const;
 
+  OptionalInjectionConversion adjustForInitialOptionalInjection() const;
+
   void dump() const LLVM_ATTRIBUTE_USED;
   void print(llvm::raw_ostream &out) const;
 };
@@ -318,10 +377,67 @@ class ConversionPeepholeHint {
   bool isForced() const { return Forced; }
 };
 
+struct CombinedConversions {
+  std::optional<Conversion> first;
+  std::optional<Conversion> second;
+
+  explicit CombinedConversions() {}
+  explicit CombinedConversions(const Conversion &first)
+    : first(first) {}
+  explicit CombinedConversions(const Conversion &first,
+                               const Conversion &second)
+    : first(first), second(second) {}
+};
+
 bool canPeepholeConversions(SILGenFunction &SGF,
                             const Conversion &outer,
                             const Conversion &inner);
 
+/// The result of trying to combine an optional injection with an existing
+/// conversion.
+class OptionalInjectionConversion {
+  enum Kind {
+    None,
+    Injection,
+    Value
+  };
+
+  std::optional<Conversion> conversion;
+  Kind kind;
+
+  OptionalInjectionConversion(Kind kind, const Conversion &conv)
+    : conversion(conv), kind(kind) {}
+
+public:
+  OptionalInjectionConversion() : kind(None) {}
+  static OptionalInjectionConversion forInjection(const Conversion &conv) {
+    return { Injection, conv };
+  }
+  static OptionalInjectionConversion forValue(const Conversion &conv) {
+    return { Value, conv };
+  }
+
+  /// Is the result of this combination a conversion that produces a
+  /// value of the original optional type?
+  bool isInjection() const {
+    return kind == Injection;
+  }
+  const Conversion &getInjectionConversion() const {
+    assert(isInjection());
+    return *conversion;
+  }
+
+  /// Is the result of this combination a conversion that produces a
+  /// value of the element of the original optional type?
+  bool isValue() const {
+    return kind == Value;
+  }
+  const Conversion &getValueConversion() const {
+    assert(isValue());
+    return *conversion;
+  }
+};
+
 /// An initialization where we ultimately want to apply a conversion to
 /// the value before completing the initialization.
 ///

lib/SILGen/SILGenConvert.cpp

@@ -320,20 +320,40 @@ ManagedValue
 SILGenFunction::emitOptionalSome(SILLocation loc, SILType optTy,
                                  ValueProducerRef produceValue,
                                  SGFContext C) {
-  // If the conversion is a bridging conversion from an optional type,
-  // do a bridging conversion from the non-optional type instead.
-  // TODO: should this be a general thing for all conversions?
+  // If we're emitting into a conversion, try to peephole the
+  // injection into it.
   if (auto optInit = C.getAsConversion()) {
     const auto &optConversion = optInit->getConversion();
-    if (optConversion.isBridging()) {
-      auto sourceValueType =
-          optConversion.getBridgingSourceType().getOptionalObjectType();
-      assert(sourceValueType);
-      if (auto valueConversion =
-            optConversion.adjustForInitialOptionalConversions(sourceValueType)){
-        return optInit->emitWithAdjustedConversion(*this, loc, *valueConversion,
-                                                   produceValue);
-      }
+
+    auto adjustment = optConversion.adjustForInitialOptionalInjection();
+
+    // If the adjustment gives us a conversion that produces an optional
+    // value, that completely takes over emission.  This generally happens
+    // only because of bridging.
+    if (adjustment.isInjection()) {
+      return optInit->emitWithAdjustedConversion(*this, loc,
+                                      adjustment.getInjectionConversion(),
+                                                 produceValue);
+
+    // If the adjustment gives us a conversion that produces a non-optional
+    // value, we need to produce the value under that conversion and then
+    // inject that into an optional.  We can do that by recursing.  This
+    // will terminate because the recursive call to emitOptionalSome gets
+    // passed a strictly "smaller" context: the parent context of the
+    // converting context we were passed.
+    } else if (adjustment.isValue()) {
+      auto produceConvertedValue = [&](SILGenFunction &SGF,
+                                       SILLocation loc,
+                                       SGFContext C) {
+        return SGF.emitConvertedRValue(loc, adjustment.getValueConversion(),
+                                       C, produceValue);
+      };
+      auto result = emitOptionalSome(loc, optConversion.getLoweredResultType(),
+                                     produceConvertedValue,
+                                     optInit->getFinalContext());
+      optInit->initWithConvertedValue(*this, loc, result);
+      optInit->finishInitialization(*this);
+      return ManagedValue::forInContext();
     }
   }
 
@@ -1133,20 +1153,6 @@ void ConvertingInitialization::
   });
 }
 
-namespace {
-struct CombinedConversions {
-  std::optional<Conversion> first;
-  std::optional<Conversion> second;
-
-  explicit CombinedConversions() {}
-  explicit CombinedConversions(const Conversion &first)
-    : first(first) {}
-  explicit CombinedConversions(const Conversion &first,
-                               const Conversion &second)
-    : first(first), second(second) {}
-  };
-}
-
 static std::optional<CombinedConversions>
 combineConversions(SILGenFunction &SGF, const Conversion &outer,
                    const Conversion &inner);
@@ -1250,6 +1256,7 @@ ManagedValue Conversion::emit(SILGenFunction &SGF, SILLocation loc,
                               ManagedValue value, SGFContext C) const {
   switch (getKind()) {
   case AnyErasure:
+  case BridgingSubtype:
   case Subtype:
     return SGF.emitTransformedValue(loc, value, getBridgingSourceType(),
                                     getBridgingResultType(), C);
@@ -1304,6 +1311,48 @@ ManagedValue Conversion::emit(SILGenFunction &SGF, SILLocation loc,
   llvm_unreachable("bad kind");
 }
 
+OptionalInjectionConversion
+Conversion::adjustForInitialOptionalInjection() const {
+  switch (getKind()) {
+  case Reabstract:
+    return OptionalInjectionConversion::forValue(
+      getReabstract(
+        getReabstractionInputOrigType().getOptionalObjectType(),
+        getReabstractionInputSubstType().getOptionalObjectType(),
+        getReabstractionInputLoweredType().getOptionalObjectType(),
+        getReabstractionOutputOrigType().getOptionalObjectType(),
+        getReabstractionOutputSubstType().getOptionalObjectType(),
+        getReabstractionOutputLoweredType().getOptionalObjectType())
+    );
+
+  case Subtype:
+    return OptionalInjectionConversion::forValue(
+      getSubtype(
+        getBridgingSourceType().getOptionalObjectType(),
+        getBridgingResultType().getOptionalObjectType(),
+        getBridgingLoweredResultType().getOptionalObjectType())
+    );
+
+  // TODO: can these actually happen?
+  case ForceOptional:
+  case ForceAndBridgeToObjC:
+  case BridgingSubtype:
+    return OptionalInjectionConversion();
+
+  case AnyErasure:
+  case BridgeToObjC:
+  case BridgeFromObjC:
+  case BridgeResultFromObjC:
+    return OptionalInjectionConversion::forInjection(
+      getBridging(getKind(), getBridgingSourceType().getOptionalObjectType(),
+                  getBridgingResultType(),
+                  getBridgingLoweredResultType(),
+                  isBridgingExplicit())
+    );
+  }
+  llvm_unreachable("bad kind");
+}
+
 std::optional<Conversion>
 Conversion::adjustForInitialOptionalConversions(CanType newSourceType) const {
   switch (getKind()) {
@@ -1315,6 +1364,7 @@ Conversion::adjustForInitialOptionalConversions(CanType newSourceType) const {
   case ForceAndBridgeToObjC:
     return std::nullopt;
 
+  case BridgingSubtype:
   case Subtype:
   case AnyErasure:
   case BridgeToObjC:
@@ -1336,6 +1386,7 @@ std::optional<Conversion> Conversion::adjustForInitialForceValue() const {
   case BridgeResultFromObjC:
   case ForceOptional:
   case ForceAndBridgeToObjC:
+  case BridgingSubtype:
   case Subtype:
     return std::nullopt;
 
@@ -1389,6 +1440,8 @@ void Conversion::print(llvm::raw_ostream &out) const {
     return printReabstraction(*this, out, "Reabstract");
   case AnyErasure:
     return printBridging(*this, out, "AnyErasure");
+  case BridgingSubtype:
+    return printBridging(*this, out, "BridgingSubtype");
   case Subtype:
     return printBridging(*this, out, "Subtype");
   case ForceOptional:
@@ -1741,7 +1794,8 @@ combineBridging(SILGenFunction &SGF,
                                 sourceType, resultType, loweredResultTy));
     } else {
       return applyPeephole(
-        Conversion::getSubtype(sourceType, resultType, loweredResultTy));
+        Conversion::getBridging(Conversion::BridgingSubtype,
+                                sourceType, resultType, loweredResultTy));
     }
   }
 
@@ -1769,7 +1823,8 @@ combineBridging(SILGenFunction &SGF,
   // Look for a subtype relationship between the source and destination.
   if (areRelatedTypesForBridgingPeephole(sourceType, resultType)) {
     return applyPeephole(
-      Conversion::getSubtype(sourceType, resultType, loweredResultTy));
+      Conversion::getBridging(Conversion::BridgingSubtype,
+                              sourceType, resultType, loweredResultTy));
   }
 
   // If the inner conversion is a result conversion that removes
@@ -1784,7 +1839,8 @@ combineBridging(SILGenFunction &SGF,
         sourceType = sourceValueType;
         loweredSourceTy = loweredSourceTy.getOptionalObjectType();
         return applyPeephole(
-          Conversion::getSubtype(sourceValueType, resultType, loweredResultTy));
+          Conversion::getBridging(Conversion::BridgingSubtype,
+                                  sourceValueType, resultType, loweredResultTy));
       }
     }
   }
@@ -1816,6 +1872,7 @@ combineConversions(SILGenFunction &SGF, const Conversion &outer,
     return std::nullopt;
 
   case Conversion::AnyErasure:
+  case Conversion::BridgingSubtype:
   case Conversion::BridgeFromObjC:
   case Conversion::BridgeResultFromObjC:
     // TODO: maybe peephole bridging through a Swift type?