Merge pull request #2621 from jckarter/dont-warn-on-objc-generic-casts-master

Sema: Don't warn on casts that change an ObjC generic's parameters.

Author: jckarter

include/swift/AST/Decl.h

@@ -2255,7 +2255,7 @@ class ValueDecl : public Decl {
   bool isObjC() const {
     return getAttrs().hasAttribute<ObjCAttr>();
   }
-
+  
   void setIsObjC(bool Value);
 
   /// Is this declaration marked with 'final'?
@@ -3343,6 +3343,13 @@ class ClassDecl : public NominalTypeDecl {
     auto NTD = dyn_cast<NominalTypeDecl>(C);
     return NTD && classof(NTD);
   }
+  
+  /// Returns true if the decl uses the Objective-C generics model.
+  ///
+  /// This is true of imported Objective-C classes.
+  bool usesObjCGenericsModel() const {
+    return isObjC() && hasClangNode() && isGenericContext();
+  }
 };
 
 

lib/SIL/DynamicCasts.cpp

@@ -367,6 +367,37 @@ swift::classifyDynamicCast(Module *M,
   auto targetClass = target.getClassOrBoundGenericClass();
   if (sourceClass) {
     if (targetClass) {
+      // Imported Objective-C generics don't check the generic parameters, which
+      // are lost at runtime.
+      if (sourceClass->usesObjCGenericsModel()) {
+      
+        if (sourceClass == targetClass)
+          return DynamicCastFeasibility::WillSucceed;
+        
+        if (targetClass->usesObjCGenericsModel()) {
+          // If both classes are ObjC generics, the cast may succeed if the
+          // classes are related, irrespective of their generic parameters.
+          auto isDeclSuperclass = [&](ClassDecl *proposedSuper,
+                                      ClassDecl *proposedSub) -> bool {
+            do {
+              if (proposedSuper == proposedSub)
+                return true;
+            } while ((proposedSub = proposedSub->getSuperclassDecl()));
+            
+            return false;
+          };
+          
+          if (isDeclSuperclass(sourceClass, targetClass))
+            return DynamicCastFeasibility::MaySucceed;
+          
+          if (isDeclSuperclass(targetClass, sourceClass)) {
+            return DynamicCastFeasibility::WillSucceed;
+          }          
+          return DynamicCastFeasibility::WillFail;
+        }
+      }
+
+
       if (target->isExactSuperclassOf(source, nullptr))
         return DynamicCastFeasibility::WillSucceed;
       if (target->isBindableToSuperclassOf(source, nullptr))

lib/SIL/SILVerifier.cpp

@@ -2219,10 +2219,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "upcast operand must be a class or class metatype instance");
       CanType opInstTy(UI->getOperand()->getType().castTo<MetatypeType>()
                          ->getInstanceType());
-      require(instTy->getClassOrBoundGenericClass(),
+      auto instClass = instTy->getClassOrBoundGenericClass();
+      require(instClass,
               "upcast must convert a class metatype to a class metatype");
-      require(instTy->isExactSuperclassOf(opInstTy, nullptr),
-              "upcast must cast to a superclass or an existential metatype");
+      
+      if (instClass->usesObjCGenericsModel()) {
+        require(instClass->getDeclaredTypeInContext()
+                  ->isBindableToSuperclassOf(opInstTy, nullptr),
+                "upcast must cast to a superclass or an existential metatype");
+      } else {
+        require(instTy->isExactSuperclassOf(opInstTy, nullptr),
+                "upcast must cast to a superclass or an existential metatype");
+      }
       return;
     }
 
@@ -2245,10 +2253,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
           FromTy.getSwiftRValueType().getAnyOptionalObjectType());
     }
 
-    require(ToTy.getClassOrBoundGenericClass(),
+    auto ToClass = ToTy.getClassOrBoundGenericClass();
+    require(ToClass,
             "upcast must convert a class instance to a class type");
-    require(ToTy.isExactSuperclassOf(FromTy),
-            "upcast must cast to a superclass");
+      if (ToClass->usesObjCGenericsModel()) {
+        require(ToClass->getDeclaredTypeInContext()
+                  ->isBindableToSuperclassOf(FromTy.getSwiftRValueType(),
+                                             nullptr),
+                "upcast must cast to a superclass or an existential metatype");
+      } else {
+        require(ToTy.isExactSuperclassOf(FromTy),
+                "upcast must cast to a superclass or an existential metatype");
+      }
   }
 
   void checkIsNonnullInst(IsNonnullInst *II) {

lib/Sema/TypeCheckConstraints.cpp

@@ -2746,7 +2746,7 @@ CheckedCastKind TypeChecker::typeCheckCheckedCast(Type fromType,
   if (toExistential || fromExistential || fromArchetype || toArchetype)
     return CheckedCastKind::ValueCast;
 
-  // Reality check casts between concrete types.
+  // Check for casts between concrete types that cannot succeed.
 
   ConstraintSystem cs(*this, dc, ConstraintSystemOptions());
 
@@ -2821,14 +2821,25 @@ CheckedCastKind TypeChecker::typeCheckCheckedCast(Type fromType,
   // This "always fails" diagnosis makes no sense when paired with the CF
   // one.
   auto clas = toType->getClassOrBoundGenericClass();
-  if (!clas || !clas->isForeign()) {
-    if (suppressDiagnostics) {
-      return CheckedCastKind::Unresolved;
-    }
-    diagnose(diagLoc, diag::downcast_to_unrelated, origFromType, origToType)
-      .highlight(diagFromRange)
-      .highlight(diagToRange);
+  if (clas && clas->isForeign())
+    return CheckedCastKind::ValueCast;
+  
+  // Don't warn on casts that change the generic parameters of ObjC generic
+  // classes. This may be necessary to force-fit ObjC APIs that depend on
+  // covariance, or for APIs where the generic parameter annotations in the
+  // ObjC headers are inaccurate.
+  if (clas && clas->usesObjCGenericsModel()) {
+    if (fromType->getClassOrBoundGenericClass() == clas)
+      return CheckedCastKind::ValueCast;
   }
+  
+  if (suppressDiagnostics) {
+    return CheckedCastKind::Unresolved;
+  }
+  diagnose(diagLoc, diag::downcast_to_unrelated, origFromType, origToType)
+    .highlight(diagFromRange)
+    .highlight(diagToRange);
+
   return CheckedCastKind::ValueCast;
 }
 

test/ClangModules/objc_bridging_generics.swift

@@ -308,3 +308,11 @@ class SwiftConcreteSubclassC<T>: GenericClass<NSString> {
   override func arrayOfThings() -> [NSString] {}
 }
 
+// FIXME: Some generic ObjC APIs rely on covariance. We don't handle this well
+// in Swift yet, but ensure we don't emit spurious warnings when
+// `as!` is used to force types to line up.
+func foo(x: GenericClass<NSMutableString>) {
+  let x2 = x as! GenericClass<NSString>
+  takeGenericClass(x2)
+  takeGenericClass(unsafeBitCast(x, to: GenericClass<NSString>.self))
+}

test/SILOptimizer/cast_folding_objc_generics.swift

@@ -0,0 +1,28 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -O -emit-sil %s | FileCheck %s
+// REQUIRES: objc_interop
+
+import objc_generics
+
+// CHECK-LABEL: sil [noinline] @_TF26cast_folding_objc_generics26testObjCGenericParamChange
+// CHECK:         upcast
+// CHECK-NOT:     int_trap
+@inline(never)
+public func testObjCGenericParamChange(_ a: GenericClass<NSMutableString>) -> GenericClass<NSString> {
+  return a as! GenericClass<NSString>
+}
+
+// CHECK-LABEL: sil [noinline] @_TF26cast_folding_objc_generics34testObjCGenericParamChangeSubclass
+// CHECK:         unconditional_checked_cast
+// CHECK-NOT:     int_trap
+@inline(never)
+public func testObjCGenericParamChangeSubclass(_ a: GenericClass<NSMutableString>) -> GenericSubclass<NSString> {
+  return a as! GenericSubclass<NSString>
+}
+
+// CHECK-LABEL: sil [noinline] @_TF26cast_folding_objc_generics36testObjCGenericParamChangeSuperclass
+// CHECK:         upcast
+// CHECK-NOT:     int_trap
+@inline(never)
+public func testObjCGenericParamChangeSuperclass(_ a: GenericSubclass<NSMutableString>) -> GenericClass<NSString> {
+  return a as! GenericClass<NSString>
+}

test/SILOptimizer/cast_folding_objc_no_foundation.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -O -emit-sil %s | FileCheck %s
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -O -emit-sil %s | FileCheck %s
 // REQUIRES: objc_interop
 
 // Note: no 'import Foundation'