Skip to content

Semantic sil objc deallocator fixes #11682

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
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
11 changes: 10 additions & 1 deletion lib/SILGen/SILGenBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,21 @@ AllocExistentialBoxInst *SILGenBuilder::createAllocExistentialBox(
ManagedValue SILGenBuilder::createStructExtract(SILLocation loc,
ManagedValue base,
VarDecl *decl) {
ManagedValue borrowedBase = SGF.emitManagedBeginBorrow(loc, base.getValue());
ManagedValue borrowedBase = base.borrow(SGF, loc);
SILValue extract =
SILBuilder::createStructExtract(loc, borrowedBase.getValue(), decl);
return ManagedValue::forUnmanaged(extract);
}

ManagedValue SILGenBuilder::createRefElementAddr(SILLocation loc,
ManagedValue operand,
VarDecl *field,
SILType resultTy) {
operand = operand.borrow(SGF, loc);
SILValue result = createRefElementAddr(loc, operand.getValue(), field);
return ManagedValue::forUnmanaged(result);
}

ManagedValue SILGenBuilder::createCopyValue(SILLocation loc,
ManagedValue originalValue) {
auto &lowering = SGF.getTypeLowering(originalValue.getType());
Expand Down
9 changes: 7 additions & 2 deletions lib/SILGen/SILGenBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,16 @@ class SILGenBuilder : public SILBuilder {
//

using SILBuilder::createStructExtract;
using SILBuilder::createCopyValue;
using SILBuilder::createCopyUnownedValue;
ManagedValue createStructExtract(SILLocation loc, ManagedValue base,
VarDecl *decl);

using SILBuilder::createRefElementAddr;
ManagedValue createRefElementAddr(SILLocation loc, ManagedValue operand,
VarDecl *field, SILType resultTy);

using SILBuilder::createCopyValue;
using SILBuilder::createCopyUnownedValue;

/// Emit a +1 copy on \p originalValue that lives until the end of the current
/// lexical scope.
ManagedValue createCopyValue(SILLocation loc, ManagedValue originalValue);
Expand Down
25 changes: 16 additions & 9 deletions lib/SILGen/SILGenDestructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ void SILGenFunction::emitDestroyingDestructor(DestructorDecl *dd) {

{
Scope S(Cleanups, cleanupLoc);
ManagedValue borrowedResultSelfValue =
emitManagedBeginBorrow(cleanupLoc, resultSelfValue);
SILValue borrowedValue = borrowedResultSelfValue.getUnmanagedValue();
if (classTy != borrowedValue->getType()) {
ManagedValue borrowedValue =
ManagedValue::forUnmanaged(resultSelfValue).borrow(*this, cleanupLoc);

if (classTy != borrowedValue.getType()) {
borrowedValue =
B.createUncheckedRefCast(cleanupLoc, borrowedValue, classTy);
}
Expand Down Expand Up @@ -168,23 +168,30 @@ void SILGenFunction::emitIVarDestroyer(SILDeclRef ivarDestroyer) {
RegularLocation loc(cd);
loc.markAutoGenerated();

SILValue selfValue = emitSelfDecl(cd->getDestructor()->getImplicitSelfDecl());
ManagedValue selfValue = ManagedValue::forUnmanaged(
emitSelfDecl(cd->getDestructor()->getImplicitSelfDecl()));

auto cleanupLoc = CleanupLocation::get(loc);
prepareEpilog(TupleType::getEmpty(getASTContext()), false, cleanupLoc);
emitClassMemberDestruction(selfValue, cd, cleanupLoc);
{
Scope S(*this, cleanupLoc);
emitClassMemberDestruction(selfValue, cd, cleanupLoc);
}

B.createReturn(loc, emitEmptyTuple(loc));
emitEpilog(loc);
}

void SILGenFunction::emitClassMemberDestruction(SILValue selfValue,
void SILGenFunction::emitClassMemberDestruction(ManagedValue selfValue,
ClassDecl *cd,
CleanupLocation cleanupLoc) {
selfValue = selfValue.borrow(*this, cleanupLoc);
for (VarDecl *vd : cd->getStoredProperties()) {
const TypeLowering &ti = getTypeLowering(vd->getType());
if (!ti.isTrivial()) {
SILValue addr = B.createRefElementAddr(cleanupLoc, selfValue, vd,
ti.getLoweredType().getAddressType());
SILValue addr =
B.createRefElementAddr(cleanupLoc, selfValue.getValue(), vd,
ti.getLoweredType().getAddressType());
B.createDestroyAddr(cleanupLoc, addr);
}
}
Expand Down
16 changes: 14 additions & 2 deletions lib/SILGen/SILGenExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3473,8 +3473,20 @@ RValue RValueEmitter::visitRebindSelfInConstructorExpr(
!calledCtor->isImplicit() &&
usesObjCAllocator(classDecl)) {

// Check whether the new self is null.
SILValue isNonnullSelf = SGF.B.createIsNonnull(E, newSelf.getValue());
// Check whether the new self is null. *NOTE* At this point, we can not
// access the actual new value using newSelf anymore. We need to grab self
// via the normal manner of doing so.
SILValue isNonnullSelf;
{
Scope S(SGF, E);
RValue selfRValue =
SGF.emitRValueForDecl(E, selfDecl, selfTy->getCanonicalType(),
AccessSemantics::DirectToStorage,
SGFContext::AllowGuaranteedPlusZero);
ManagedValue reloadedSelf =
std::move(selfRValue).getAsSingleValue(SGF, E);
isNonnullSelf = SGF.B.createIsNonnull(E, reloadedSelf.getValue());
}
Condition cond = SGF.emitCondition(isNonnullSelf, E,
/*hasFalseCode=*/false,
/*invertValue=*/true,
Expand Down
3 changes: 2 additions & 1 deletion lib/SILGen/SILGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
///
/// \param selfValue The 'self' value.
/// \param cd The class declaration whose members are being destroyed.
void emitClassMemberDestruction(SILValue selfValue, ClassDecl *cd,
void emitClassMemberDestruction(ManagedValue selfValue, ClassDecl *cd,
CleanupLocation cleanupLoc);

/// Generates code for a curry thunk from one uncurry level
/// of a function to another.
void emitCurryThunk(SILDeclRef thunk);
Expand Down
18 changes: 10 additions & 8 deletions test/SILGen/objc_dealloc.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -sdk %S/Inputs -I %S/Inputs -enable-source-import %s -emit-silgen | %FileCheck %s
// RUN: %target-swift-frontend -sdk %S/Inputs -I %S/Inputs -enable-source-import %s -emit-silgen -enable-sil-ownership | %FileCheck %s

// REQUIRES: objc_interop

Expand All @@ -18,7 +18,7 @@ class SwiftGizmo : Gizmo {
// CHECK-NEXT: return [[RESULT]] : $X

// CHECK-LABEL: sil hidden @_T012objc_dealloc10SwiftGizmoC{{[_0-9a-zA-Z]*}}fc
// CHECK: bb0([[SELF_PARAM:%[0-9]+]] : $SwiftGizmo):
// CHECK: bb0([[SELF_PARAM:%[0-9]+]] : @owned $SwiftGizmo):
override init() {
// CHECK: [[SELF_BOX:%.*]] = alloc_box ${ var SwiftGizmo }, let, name "self"
// CHECK: [[SELF_UNINIT:%.*]] = mark_uninitialized [derivedselfonly] [[SELF_BOX]] : ${ var SwiftGizmo }
Expand All @@ -36,7 +36,7 @@ class SwiftGizmo : Gizmo {

// CHECK-LABEL: sil hidden @_T012objc_dealloc10SwiftGizmoCfD : $@convention(method) (@owned SwiftGizmo) -> ()
deinit {
// CHECK: bb0([[SELF:%[0-9]+]] : $SwiftGizmo):
// CHECK: bb0([[SELF:%[0-9]+]] : @owned $SwiftGizmo):
// Call onDestruct()
// CHECK: [[ONDESTRUCT_REF:%[0-9]+]] = function_ref @_T012objc_dealloc10onDestructyyF : $@convention(thin) () -> ()
// CHECK: [[ONDESTRUCT_RESULT:%[0-9]+]] = apply [[ONDESTRUCT_REF]]() : $@convention(thin) () -> ()
Expand All @@ -56,7 +56,7 @@ class SwiftGizmo : Gizmo {

// Objective-C deallocation deinit thunk (i.e., -dealloc).
// CHECK-LABEL: sil hidden [thunk] @_T012objc_dealloc10SwiftGizmoCfDTo : $@convention(objc_method) (SwiftGizmo) -> ()
// CHECK: bb0([[SELF:%[0-9]+]] : $SwiftGizmo):
// CHECK: bb0([[SELF:%[0-9]+]] : @unowned $SwiftGizmo):
// CHECK: [[SELF_COPY:%.*]] = copy_value [[SELF]]

// CHECK: [[GIZMO_DTOR:%[0-9]+]] = function_ref @_T012objc_dealloc10SwiftGizmoCfD : $@convention(method) (@owned SwiftGizmo) -> ()
Expand All @@ -65,7 +65,7 @@ class SwiftGizmo : Gizmo {

// Objective-C IVar initializer (i.e., -.cxx_construct)
// CHECK-LABEL: sil hidden @_T012objc_dealloc10SwiftGizmoCfeTo : $@convention(objc_method) (@owned SwiftGizmo) -> @owned SwiftGizmo
// CHECK: bb0([[SELF_PARAM:%[0-9]+]] : $SwiftGizmo):
// CHECK: bb0([[SELF_PARAM:%[0-9]+]] : @owned $SwiftGizmo):
// CHECK-NEXT: debug_value [[SELF_PARAM]] : $SwiftGizmo, let, name "self"
// CHECK-NEXT: [[SELF:%[0-9]+]] = mark_uninitialized [rootself] [[SELF_PARAM]] : $SwiftGizmo
// CHECK: [[XINIT:%[0-9]+]] = function_ref @_T012objc_dealloc10SwiftGizmoC1xAA1XCvfi
Expand All @@ -80,10 +80,12 @@ class SwiftGizmo : Gizmo {

// Objective-C IVar destroyer (i.e., -.cxx_destruct)
// CHECK-LABEL: sil hidden @_T012objc_dealloc10SwiftGizmoCfETo : $@convention(objc_method) (SwiftGizmo) -> ()
// CHECK: bb0([[SELF:%[0-9]+]] : $SwiftGizmo):
// CHECK-NEXT: debug_value [[SELF]] : $SwiftGizmo, let, name "self"
// CHECK-NEXT: [[X:%[0-9]+]] = ref_element_addr [[SELF]] : $SwiftGizmo, #SwiftGizmo.x
// CHECK: bb0([[SELF:%[0-9]+]] : @unowned $SwiftGizmo):
// CHECK-NEXT: debug_value [[SELF]] : $SwiftGizmo, let, name "self"
// CHECK-NEXT: [[SELF_BORROW:%.*]] = begin_borrow [[SELF]]
// CHECK-NEXT: [[X:%[0-9]+]] = ref_element_addr [[SELF_BORROW]] : $SwiftGizmo, #SwiftGizmo.x
// CHECK-NEXT: destroy_addr [[X]] : $*X
// CHECK-NEXT: end_borrow [[SELF_BORROW]] from [[SELF]]
// CHECK-NEXT: [[RESULT:%[0-9]+]] = tuple ()
// CHECK-NEXT: return [[RESULT]] : $()
}
Expand Down
4 changes: 3 additions & 1 deletion test/SILGen/objc_thunks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,9 @@ extension Hoozit {
// CHECK: [[CTOR:%[0-9]+]] = class_method [volatile] [[SELF:%[0-9]+]] : $Hoozit, #Hoozit.init!initializer.1.foreign : (Hoozit.Type) -> (Int) -> Hoozit, $@convention(objc_method) (Int, @owned Hoozit) -> @owned Hoozit
// CHECK: [[NEW_SELF:%[0-9]+]] = apply [[CTOR]]
// CHECK: store [[NEW_SELF]] to [init] [[PB_BOX]] : $*Hoozit
// CHECK: [[NONNULL:%[0-9]+]] = is_nonnull [[NEW_SELF]] : $Hoozit
// CHECK: [[RELOADED_SELF:%.*]] = load_borrow [[PB_BOX]]
// CHECK: [[NONNULL:%[0-9]+]] = is_nonnull [[RELOADED_SELF]] : $Hoozit
// CHECK: end_borrow [[RELOADED_SELF]] from [[PB_BOX]]
// CHECK-NEXT: cond_br [[NONNULL]], [[NONNULL_BB:bb[0-9]+]], [[NULL_BB:bb[0-9]+]]
// CHECK: [[NULL_BB]]:
// CHECK-NEXT: destroy_value [[X_BOX]] : ${ var X }
Expand Down