Skip to content

[PrintAsObjC] Use 'unsafe_unretained' to print 'unowned', not 'assign' #19167

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
merged 2 commits into from
Sep 10, 2018
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
69 changes: 47 additions & 22 deletions lib/PrintAsObjC/PrintAsObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1027,17 +1027,40 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
return true;
}

/// Returns whether \p ty is the C type \c CFTypeRef, or some typealias
/// thereof.
bool isCFTypeRef(Type ty) {
if (ID_CFTypeRef.empty())
ID_CFTypeRef = M.getASTContext().getIdentifier("CFTypeRef");

const TypeAliasDecl *TAD = nullptr;
while (auto aliasTy = dyn_cast<NameAliasType>(ty.getPointer())) {
TAD = aliasTy->getDecl();
ty = aliasTy->getSinglyDesugaredType();
}

return TAD && TAD->getName() == ID_CFTypeRef && TAD->hasClangNode();
if (!TAD || !TAD->hasClangNode())
return false;

if (ID_CFTypeRef.empty())
ID_CFTypeRef = M.getASTContext().getIdentifier("CFTypeRef");
return TAD->getName() == ID_CFTypeRef;
}

/// Returns true if \p ty can be used with Objective-C reference-counting
/// annotations like \c strong and \c weak.
bool isObjCReferenceCountableObjectType(Type ty) {
if (auto classDecl = ty->getClassOrBoundGenericClass()) {
switch (classDecl->getForeignClassKind()) {
case ClassDecl::ForeignKind::Normal:
case ClassDecl::ForeignKind::RuntimeOnly:
return true;
case ClassDecl::ForeignKind::CFType:
return false;
}
}

if (ty->isObjCExistentialType() && !isCFTypeRef(ty))
return true;

return false;
}

void visitVarDecl(VarDecl *VD) {
Expand Down Expand Up @@ -1065,22 +1088,27 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
os << ", readonly";

// Print the ownership semantics, if relevant.
// We treat "unowned" as "assign" (even though it's more like
// "safe_unretained") because we want people to think twice about
// allowing that object to disappear.
Type ty = VD->getInterfaceType();
if (auto weakTy = ty->getAs<WeakStorageType>()) {
auto innerTy = weakTy->getReferentType()->getOptionalObjectType();
auto innerClass = innerTy->getClassOrBoundGenericClass();
if ((innerClass &&
innerClass->getForeignClassKind()!=ClassDecl::ForeignKind::CFType) ||
(innerTy->isObjCExistentialType() && !isCFTypeRef(innerTy))) {
os << ", weak";
if (auto referenceStorageTy = ty->getAs<ReferenceStorageType>()) {
switch (referenceStorageTy->getOwnership()) {
case ReferenceOwnership::Strong:
llvm_unreachable("not represented with a ReferenceStorageType");
case ReferenceOwnership::Weak: {
auto innerTy =
referenceStorageTy->getReferentType()->getOptionalObjectType();
if (isObjCReferenceCountableObjectType(innerTy))
os << ", weak";
break;
}
case ReferenceOwnership::Unowned:
case ReferenceOwnership::Unmanaged:
// We treat "unowned" as "unsafe_unretained" (even though it's more
// like "safe_unretained") because we want people to think twice about
// allowing that object to disappear. "unowned(unsafe)" (and
// Swift.Unmanaged, handled below) really are "unsafe_unretained".
os << ", unsafe_unretained";
break;
}
} else if (ty->is<UnownedStorageType>()) {
os << ", assign";
} else if (ty->is<UnmanagedStorageType>()) {
os << ", unsafe_unretained";
} else {
Type copyTy = ty;
bool isOptional = false;
Expand Down Expand Up @@ -1117,10 +1145,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
case FunctionTypeRepresentation::CFunctionPointer:
break;
}
} else if ((nominal && isa<ClassDecl>(nominal) &&
cast<ClassDecl>(nominal)->getForeignClassKind() !=
ClassDecl::ForeignKind::CFType) ||
(copyTy->isObjCExistentialType() && !isCFTypeRef(copyTy))) {
} else if (isObjCReferenceCountableObjectType(copyTy)) {
os << ", strong";
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/PrintAsObjC/classes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public class NonObjCClass { }
// CHECK-NEXT: SWIFT_CLASS_PROPERTY(@property (nonatomic, class, readonly, strong) Properties * _Nonnull sharedRO;)
// CHECK-NEXT: + (Properties * _Nonnull)sharedRO SWIFT_WARN_UNUSED_RESULT;
// CHECK-NEXT: @property (nonatomic, weak) Properties * _Nullable weakOther;
// CHECK-NEXT: @property (nonatomic, assign) Properties * _Nonnull unownedOther;
// CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nonnull unownedOther;
// CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nonnull unmanagedOther;
// CHECK-NEXT: @property (nonatomic, unsafe_unretained) Properties * _Nullable unmanagedByDecl;
// CHECK-NEXT: @property (nonatomic, weak) id <MyProtocol> _Nullable weakProto;
Expand Down
2 changes: 1 addition & 1 deletion test/PrintAsObjC/protocols.swift
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ extension NSString : A, ZZZ {}
@objc class Subclass : RootClass1, ZZZ {}

// CHECK-LABEL: @protocol UnownedProperty
// CHECK-NEXT: @property (nonatomic, assign) id _Nonnull unownedProp;
// CHECK-NEXT: @property (nonatomic, unsafe_unretained) id _Nonnull unownedProp;
@objc protocol UnownedProperty {
unowned var unownedProp: AnyObject { get set }
}
Expand Down