[Sema] Don't treat non-class archetypes as non-optional when casting to them

Author: hamishknight

include/swift/AST/Types.h

@@ -573,6 +573,12 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// whether a type parameter exists at any position.
   bool isTypeParameter();
 
+  /// \brief Determine whether this type can dynamically be an optional type.
+  ///
+  /// \param includeExistential Whether an existential type should be considered
+  /// such a type.
+  bool canDynamicallyBeOptionalType(bool includeExistential);
+
   /// Determine whether this type contains a type parameter somewhere in it.
   bool hasTypeParameter() {
     return getRecursiveProperties().hasTypeParameter();
@@ -4998,6 +5004,14 @@ inline bool TypeBase::isOpenedExistentialWithError() {
   return false;
 }
 
+inline bool TypeBase::canDynamicallyBeOptionalType(bool includeExistential) {
+  CanType T = getCanonicalType();
+  auto isArchetypeOrExistential = isa<ArchetypeType>(T) ||
+    (includeExistential && T.isExistentialType());
+
+  return isArchetypeOrExistential && !T.isAnyClassReferenceType();
+}
+
 inline ClassDecl *TypeBase::getClassOrBoundGenericClass() {
   return getCanonicalType().getClassOrBoundGenericClass();
 }

lib/SIL/DynamicCasts.cpp

@@ -239,14 +239,13 @@ bool swift::isError(ModuleDecl *M, CanType Ty) {
   return false;
 }
 
-/// Given that a type is not statically known to be an optional type, check whether
-/// it might dynamically be an optional type.
-static bool canDynamicallyBeOptionalType(CanType type) {
+/// Given that a type is not statically known to be an optional type, check
+/// whether it might dynamically be able to store an optional.
+static bool canDynamicallyStoreOptional(CanType type) {
   assert(!type.getOptionalObjectType());
-  return (isa<ArchetypeType>(type) || type.isExistentialType())
-      && !type.isAnyClassReferenceType();
+  return type->canDynamicallyBeOptionalType(/* includeExistential */ true);
 }
-
+  
 /// Given two class types, check whether there's a hierarchy relationship
 /// between them.
 static DynamicCastFeasibility
@@ -326,7 +325,7 @@ swift::classifyDynamicCast(ModuleDecl *M,
     auto result = classifyDynamicCast(M, source, targetObject,
                                       /* isSourceTypeExact */ false,
                                       isWholeModuleOpts);
-    if (canDynamicallyBeOptionalType(source))
+    if (canDynamicallyStoreOptional(source))
       result = atWorst(result, DynamicCastFeasibility::MaySucceed);
     return result;
 

lib/Sema/CSApply.cpp

@@ -3299,14 +3299,21 @@ namespace {
       auto destValueType
         = finalResultType->lookThroughAllOptionalTypes(destOptionals);
 
-      // When performing a bridging operation, if the destination value type
-      // is 'AnyObject', leave any extra optionals on the source in place.
-      if (castKind == OptionalBindingsCastKind::Bridged &&
-          srcOptionals.size() > destOptionals.size() &&
-          destValueType->isAnyObject()) {
-        srcType = srcOptionals[destOptionals.size()];
-        srcOptionals.erase(srcOptionals.begin() + destOptionals.size(),
-                           srcOptionals.end());
+      auto isBridgeToAnyObject =
+        castKind == OptionalBindingsCastKind::Bridged &&
+        destValueType->isAnyObject();
+
+      // If the destination value type is 'AnyObject' when performing a
+      // bridging operation, or if the destination value type could dynamically
+      // be an optional type, leave any extra optionals on the source in place.
+      if (isBridgeToAnyObject || destValueType->canDynamicallyBeOptionalType(
+                                              /* includeExistential */ false)) {
+        auto destOptionalsCount = destOptionals.size() - destExtraOptionals;
+        if (srcOptionals.size() > destOptionalsCount) {
+          srcType = srcOptionals[destOptionalsCount];
+          srcOptionals.erase(srcOptionals.begin() + destOptionalsCount,
+                             srcOptionals.end());
+        }
       }
 
       // When performing a bridging operation, if the destination type

test/SILGen/generic_casts.swift

@@ -217,7 +217,74 @@ func class_existential_is_class(_ p: ClassBound) -> Bool {
 }
 
 // CHECK-LABEL: sil hidden @$S13generic_casts27optional_anyobject_to_classyAA1CCSgyXlSgF
-// CHECK:         checked_cast_br {{%.*}} : $AnyObject to $C
 func optional_anyobject_to_class(_ p: AnyObject?) -> C? {
   return p as? C
+  // CHECK: checked_cast_br {{%.*}} : $AnyObject to $C
 }
+
+// The below tests are to ensure we don't dig into an optional operand when
+// casting to a non-class archetype, as it could dynamically be an optional type.
+
+// CHECK-LABEL: sil hidden @$S13generic_casts32optional_any_to_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_to_opaque_archetype<T>(_ x: Any?) -> T {
+  return x as! T
+  // CHECK: bb0([[RET:%.*]] : @trivial $*T, {{%.*}} : @trivial $*Optional<Any>):
+  // CHECK: unconditional_checked_cast_addr Optional<Any> in {{%.*}} : $*Optional<Any> to T in [[RET]] : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts46optional_any_conditionally_to_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_conditionally_to_opaque_archetype<T>(_ x: Any?) -> T? {
+  return x as? T
+  // CHECK: checked_cast_addr_br take_always Optional<Any> in {{%.*}} : $*Optional<Any> to T in {{%.*}} : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts32optional_any_is_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_is_opaque_archetype<T>(_ x: Any?, _: T) -> Bool {
+  return x is T
+  // CHECK: checked_cast_addr_br take_always Optional<Any> in {{%.*}} : $*Optional<Any> to T in {{%.*}} : $*T
+}
+
+// But we can dig into at most one layer of the operand if it's
+// an optional archetype...
+
+// CHECK-LABEL: sil hidden @$S13generic_casts016optional_any_to_C17_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_to_optional_opaque_archetype<T>(_ x: Any?) -> T? {
+  return x as! T?
+  // CHECK: unconditional_checked_cast_addr Any in {{%.*}} : $*Any to T in {{%.*}} : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts030optional_any_conditionally_to_C17_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_conditionally_to_optional_opaque_archetype<T>(_ x: Any?) -> T?? {
+  return x as? T?
+  // CHECK: checked_cast_addr_br take_always Any in {{%.*}} : $*Any to T in {{%.*}} : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts016optional_any_is_C17_opaque_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_is_optional_opaque_archetype<T>(_ x: Any?, _: T) -> Bool {
+  return x is T?
+  //   Because the levels of optional are the same, 'is' doesn't transform into an 'as?',
+  //   so we just cast directly without digging into the optional operand.
+  // CHECK: checked_cast_addr_br take_always Optional<Any> in {{%.*}} : $*Optional<Any> to Optional<T> in {{%.*}} : $*Optional<T>
+}
+
+// And we can dig into the operand when casting to a class archetype, as it
+// cannot dynamically be optional...
+
+// CHECK-LABEL: sil hidden @$S13generic_casts31optional_any_to_class_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_to_class_archetype<T : AnyObject>(_ x: Any?) -> T {
+  return x as! T
+  // CHECK: unconditional_checked_cast_addr Any in {{%.*}} : $*Any to T in {{%.*}} : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts45optional_any_conditionally_to_class_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_conditionally_to_class_archetype<T : AnyObject>(_ x: Any?) -> T? {
+  return x as? T
+  // CHECK: checked_cast_addr_br take_always Any in {{%.*}} : $*Any to T in {{%.*}} : $*T
+}
+
+// CHECK-LABEL: sil hidden @$S13generic_casts31optional_any_is_class_archetype{{[_0-9a-zA-Z]*}}F
+func optional_any_is_class_archetype<T : AnyObject>(_ x: Any?, _: T) -> Bool {
+  return x is T
+  // CHECK: checked_cast_addr_br take_always Any in {{%.*}} : $*Any to T in {{%.*}} : $*T
+}
+