Skip to content

[3.0] Relax overly conservative "uses generic params" errors in ObjC generic extensions. #4246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion lib/SILGen/SILGenConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,40 @@ ManagedValue SILGenFunction::emitExistentialErasure(
return emitManagedRValueWithCleanup(existential);
}
case ExistentialRepresentation::Opaque: {

// If the concrete value is a pseudogeneric archetype, first erase it to
// its upper bound.
auto anyObjectProto = getASTContext()
.getProtocol(KnownProtocolKind::AnyObject);
auto anyObjectTy = anyObjectProto
? anyObjectProto->getDeclaredType()->getCanonicalType()
: CanType();
auto eraseToAnyObject =
[&, concreteFormalType, F](SGFContext C) -> ManagedValue {
auto concreteValue = F(SGFContext());
auto anyObjectConformance = SGM.SwiftModule
->lookupConformance(concreteFormalType, anyObjectProto, nullptr);
ProtocolConformanceRef buf[] = {
*anyObjectConformance,
};

auto asAnyObject = B.createInitExistentialRef(loc,
SILType::getPrimitiveObjectType(anyObjectTy),
concreteFormalType,
concreteValue.getValue(),
getASTContext().AllocateCopy(buf));
return ManagedValue(asAnyObject, concreteValue.getCleanup());
};

auto concreteTLPtr = &concreteTL;
if (this->F.getLoweredFunctionType()->isPseudogeneric()) {
if (anyObjectTy && concreteFormalType->is<ArchetypeType>()) {
concreteFormalType = anyObjectTy;
concreteTLPtr = &getTypeLowering(anyObjectTy);
F = eraseToAnyObject;
}
}

// Allocate the existential.
SILValue existential =
getBufferForExprResult(loc, existentialTL.getLoweredType(), C);
Expand All @@ -662,7 +696,7 @@ ManagedValue SILGenFunction::emitExistentialErasure(
SILValue valueAddr = B.createInitExistentialAddr(
loc, existential,
concreteFormalType,
concreteTL.getLoweredType(),
concreteTLPtr->getLoweredType(),
conformances);
// Initialize the concrete value in-place.
InitializationPtr init(
Expand Down
63 changes: 56 additions & 7 deletions lib/Sema/TypeCheckCaptures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,29 @@ class FindCapturedVars : public ASTWalker {

// We want to look through type aliases here.
type = type->getCanonicalType();


class TypeCaptureWalker : public TypeWalker {
AnyFunctionRef AFR;
llvm::function_ref<void(Type)> Callback;
public:
explicit TypeCaptureWalker(AnyFunctionRef AFR,
llvm::function_ref<void(Type)> callback)
: AFR(AFR), Callback(callback) {}

Action walkToTypePre(Type ty) override {
Callback(ty);
// Pseudogeneric classes don't use their generic parameters so we
// don't need to visit them.
if (AFR.isObjC()) {
if (auto clas = dyn_cast_or_null<ClassDecl>(ty->getAnyNominal())) {
if (clas->usesObjCGenericsModel()) {
return Action::SkipChildren;
}
}
}
return Action::Continue;
}
};
// If the type contains dynamic 'Self', conservatively assume we will
// need 'Self' metadata at runtime. We could generalize the analysis
// used below for usages of generic parameters in Objective-C
Expand All @@ -88,14 +110,14 @@ class FindCapturedVars : public ASTWalker {
// retainable pointer. Similarly stored property access does not
// need it, etc.
if (type->hasDynamicSelfType()) {
type.visit([&](Type t) {
type.walk(TypeCaptureWalker(AFR, [&](Type t) {
if (auto *dynamicSelf = t->getAs<DynamicSelfType>()) {
if (DynamicSelfCaptureLoc.isInvalid()) {
DynamicSelfCaptureLoc = loc;
DynamicSelf = dynamicSelf;
}
}
});
}));
}

// Similar to dynamic 'Self', IRGen doesn't really need type metadata
Expand All @@ -106,13 +128,13 @@ class FindCapturedVars : public ASTWalker {
// instead, but even there we don't really have enough information to
// perform it accurately.
if (type->hasArchetype()) {
type.visit([&](Type t) {
type.walk(TypeCaptureWalker(AFR, [&](Type t) {
if (t->is<ArchetypeType>() &&
!t->isOpenedExistential() &&
GenericParamCaptureLoc.isInvalid()) {
GenericParamCaptureLoc = loc;
}
});
}));
}
}

Expand Down Expand Up @@ -397,8 +419,10 @@ class FindCapturedVars : public ASTWalker {
// doesn't require its type metadata.
if (auto declRef = dyn_cast<DeclRefExpr>(E))
return (!declRef->getDecl()->isObjC()
&& !E->getType()->hasRetainablePointerRepresentation()
&& !E->getType()->is<AnyMetatypeType>());
&& !E->getType()->getLValueOrInOutObjectType()
->hasRetainablePointerRepresentation()
&& !E->getType()->getLValueOrInOutObjectType()
->is<AnyMetatypeType>());

// Loading classes or metatypes doesn't require their metadata.
if (isa<LoadExpr>(E))
Expand Down Expand Up @@ -475,18 +499,43 @@ class FindCapturedVars : public ASTWalker {
if (E->getType()->isObjCExistentialType()
|| E->getType()->is<AnyMetatypeType>())
return false;

// We also special case Any erasure in pseudogeneric contexts
// not to rely on concrete type metadata by erasing from AnyObject
// as a waypoint.
if (E->getType()->isAny()
&& erasure->getSubExpr()->getType()->is<ArchetypeType>())
return false;

// Erasure to a Swift protocol always captures the type metadata from
// its subexpression.
checkType(erasure->getSubExpr()->getType(),
erasure->getSubExpr()->getLoc());
return true;
}


// Converting an @objc metatype to AnyObject doesn't require type
// metadata.
if (isa<ClassMetatypeToObjectExpr>(E)
|| isa<ExistentialMetatypeToObjectExpr>(E))
return false;

// Casting to an ObjC class doesn't require the metadata of its type
// parameters, if any.
if (auto cast = dyn_cast<CheckedCastExpr>(E)) {
if (auto clas = dyn_cast_or_null<ClassDecl>(
cast->getCastTypeLoc().getType()->getAnyNominal())) {
if (clas->usesObjCGenericsModel()) {
return false;
}
}
}

// Assigning an object doesn't require type metadata.
if (auto assignment = dyn_cast<AssignExpr>(E))
return !assignment->getSrc()->getType()
->hasRetainablePointerRepresentation();

return true;
}
Expand Down
53 changes: 44 additions & 9 deletions test/ClangModules/objc_bridging_generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,8 @@ extension GenericClass {
func usesGenericParamG(_ x: T) {
_ = T.self // expected-note{{used here}}
}
// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamH(_ x: T) {
_ = x as Any // expected-note{{used here}}
func doesntUseGenericParamH(_ x: T) {
_ = x as Any
}
// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamI(_ y: T.Type) {
Expand Down Expand Up @@ -181,14 +180,13 @@ extension AnimalContainer {
_ = #selector(y.create)
}

// TODO: 'Any' bridging should not require reifying generic params.
func doesntUseGenericParam2(_ x: T, _ y: T.Type) { // expected-error{{cannot access the class's generic parameters}}
func doesntUseGenericParam2(_ x: T, _ y: T.Type) {
let a = x.another()
_ = a.another()
_ = x.another().another()

_ = type(of: x).create().another()
_ = type(of: x).init(noise: x).another() // expected-note{{here}}
_ = type(of: x).init(noise: x).another()
_ = y.create().another()
_ = y.init(noise: x).another()
_ = y.init(noise: x.another()).another()
Expand All @@ -212,6 +210,44 @@ extension AnimalContainer {
y.apexPredator = x
}

func doesntUseGenericParam5(y: T) {
var x = y
x = y
_ = x
}
func doesntUseGenericParam6(y: T?) {
var x = y
x = y
_ = x
}

// Doesn't use 'T', since dynamic casting to an ObjC generic class doesn't
// check its generic parameters
func doesntUseGenericParam7() {
_ = (self as AnyObject) as! GenericClass<T>
_ = (self as AnyObject) as? GenericClass<T>
_ = (self as AnyObject) as! AnimalContainer<T>
_ = (self as AnyObject) as? AnimalContainer<T>
_ = (self as AnyObject) is AnimalContainer<T>
_ = (self as AnyObject) is AnimalContainer<T>
}

// Dynamic casting to the generic parameter would require its generic params,
// though
// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamZ1() {
_ = (self as AnyObject) as! T //expected-note{{here}}
}
// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamZ2() {
_ = (self as AnyObject) as? T //expected-note{{here}}
}
// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamZ3() {
_ = (self as AnyObject) is T //expected-note{{here}}
}


// expected-error@+1{{extension of a generic Objective-C class cannot access the class's generic parameters}}
func usesGenericParamA(_ x: T) {
_ = T(noise: x) // expected-note{{used here}}
Expand Down Expand Up @@ -246,9 +282,8 @@ extension AnimalContainer {
}

extension PettableContainer {
// TODO: Any erasure shouldn't use generic parameter metadata.
func doesntUseGenericParam(_ x: T, _ y: T.Type) { // expected-error{{cannot access the class's generic parameters}}
_ = type(of: x).init(fur: x).other() // expected-note{{here}}
func doesntUseGenericParam(_ x: T, _ y: T.Type) {
_ = type(of: x).init(fur: x).other()
_ = type(of: x).adopt().other()
_ = y.init(fur: x).other()
_ = y.adopt().other()
Expand Down
11 changes: 11 additions & 0 deletions test/SILGen/objc_bridging_any.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// REQUIRES: objc_interop

import Foundation
import objc_generics

protocol P {}
protocol CP: class {}
Expand Down Expand Up @@ -498,3 +499,13 @@ func dynamicLookup(x: AnyObject) {
_ = x.anyProperty
_ = x[IndexForAnySubscript()]
}

extension GenericClass {
// CHECK-LABEL: sil hidden @_TFE17objc_bridging_anyCSo12GenericClass23pseudogenericAnyErasurefT1xx_P_
func pseudogenericAnyErasure(x: T) -> Any {
// CHECK: [[ANY_BUF:%.*]] = init_existential_addr %0 : $*Any, $AnyObject
// CHECK: [[ANYOBJECT:%.*]] = init_existential_ref %1 : $T : $T, $AnyObject
// CHECK: store [[ANYOBJECT]] to [[ANY_BUF]]
return x
}
}