Skip to content

Commit

Permalink
Merge pull request swiftlang#2621 from jckarter/dont-warn-on-objc-gen…
Browse files Browse the repository at this point in the history
…eric-casts-master

Sema: Don't warn on casts that change an ObjC generic's parameters.
  • Loading branch information
jckarter committed May 23, 2016
2 parents e63eaa8 + 396e484 commit 4082a5c
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 16 deletions.
9 changes: 8 additions & 1 deletion include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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'?
Expand Down Expand Up @@ -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();
}
};


Expand Down
31 changes: 31 additions & 0 deletions lib/SIL/DynamicCasts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
28 changes: 22 additions & 6 deletions lib/SIL/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down
27 changes: 19 additions & 8 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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;
}

Expand Down
8 changes: 8 additions & 0 deletions test/ClangModules/objc_bridging_generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
28 changes: 28 additions & 0 deletions test/SILOptimizer/cast_folding_objc_generics.swift
Original file line number Diff line number Diff line change
@@ -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>
}
2 changes: 1 addition & 1 deletion test/SILOptimizer/cast_folding_objc_no_foundation.swift
Original file line number Diff line number Diff line change
@@ -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'
Expand Down

0 comments on commit 4082a5c

Please sign in to comment.