Skip to content

Commit 7da688d

Browse files
rjmccallrjmccall
committed
Always manage subobject projections with formal-access cleanups.
To make that work, enter appropriate scopes (ArgumentScopes and FormalEvaluationScopes) at a bunch of places. But note that l-value emission generally can't enter such a scope, so in generic routines like emitOpenExistentialExpr we have to just assert that we're already in a scope.
1 parent cbd2cd7 commit 7da688d

18 files changed

+80
-45
lines changed

‎lib/SILGen/ManagedValue.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ ManagedValue ManagedValue::copyUnmanaged(SILGenFunction &SGF, SILLocation loc) {
8585
/// have cleanups. It returns a +1 value with one.
8686
ManagedValue ManagedValue::formalAccessCopyUnmanaged(SILGenFunction &SGF,
8787
SILLocation loc) {
88+
assert(SGF.isInFormalEvaluationScope());
89+
8890
if (getType().isObject()) {
8991
return SGF.B.createFormalAccessCopyValue(loc, *this);
9092
}
@@ -141,6 +143,7 @@ ManagedValue ManagedValue::borrow(SILGenFunction &SGF, SILLocation loc) const {
141143

142144
ManagedValue ManagedValue::formalAccessBorrow(SILGenFunction &SGF,
143145
SILLocation loc) const {
146+
assert(SGF.isInFormalEvaluationScope());
144147
assert(getValue() && "cannot borrow an invalid or in-context value");
145148
if (isLValue())
146149
return *this;

‎lib/SILGen/SILGenApply.cpp

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ static ManagedValue borrowedCastToOriginalSelfType(SILGenFunction &SGF,
207207
}
208208

209209
// Otherwise, we have a non-metatype. Use a borrow+unchecked_ref_cast.
210-
return SGF.B.createUncheckedRefCast(loc, self.borrow(SGF, loc),
210+
return SGF.B.createUncheckedRefCast(loc, self.formalAccessBorrow(SGF, loc),
211211
originalSelfType);
212212
}
213213

@@ -226,7 +226,7 @@ static ManagedValue convertOwnershipConventionGivenParamInfo(
226226
if (isForCoroutine && value.getOwnershipKind() == ValueOwnershipKind::Owned) {
227227
if (param.isDirectGuaranteed() || (!SGF.silConv.useLoweredAddresses() &&
228228
param.isIndirectInGuaranteed())) {
229-
return value.borrow(SGF, loc);
229+
return value.formalAccessBorrow(SGF, loc);
230230
}
231231
}
232232

@@ -592,7 +592,7 @@ class Callee {
592592
auto methodTy = SGF.SGM.Types.getConstantOverrideType(*constant);
593593

594594
// Otherwise, do the dynamic dispatch inline.
595-
Scope S(SGF, Loc);
595+
ArgumentScope S(SGF, Loc);
596596

597597
SILValue methodVal;
598598
if (!constant->isForeign) {
@@ -603,26 +603,30 @@ class Callee {
603603
Loc, borrowedSelf->getValue(), *constant,
604604
SILType::getPrimitiveObjectType(methodTy));
605605
}
606+
S.pop();
606607
return ManagedValue::forUnmanaged(methodVal);
607608
}
608609
case Kind::SuperMethod: {
609610
assert(!constant->isCurried);
610611

611-
Scope S(SGF, Loc);
612+
ArgumentScope S(SGF, Loc);
612613
ManagedValue castValue = borrowedCastToOriginalSelfType(
613614
SGF, Loc, *borrowedSelf);
614615

615616
auto base = constant->getOverriddenVTableEntry();
616617
auto constantInfo =
617618
SGF.SGM.Types.getConstantOverrideInfo(*constant, base);
618619

620+
ManagedValue fn;
619621
if (!constant->isForeign) {
620-
return SGF.B.createSuperMethod(Loc, castValue, *constant,
621-
constantInfo.getSILType());
622+
fn = SGF.B.createSuperMethod(Loc, castValue, *constant,
623+
constantInfo.getSILType());
622624
} else {
623-
return SGF.B.createObjCSuperMethod(Loc, castValue, *constant,
624-
constantInfo.getSILType());
625+
fn = SGF.B.createObjCSuperMethod(Loc, castValue, *constant,
626+
constantInfo.getSILType());
625627
}
628+
S.pop();
629+
return fn;
626630
}
627631
case Kind::WitnessMethod: {
628632
auto constantInfo = SGF.getConstantInfo(*constant);
@@ -632,28 +636,29 @@ class Callee {
632636
auto lookupType = selfType.subst(Substitutions)->getCanonicalType();
633637
auto conformance = *Substitutions.lookupConformance(selfType, proto);
634638

635-
SILValue fn;
639+
ArgumentScope S(SGF, Loc);
636640

641+
SILValue fn;
637642
if (!constant->isForeign) {
638-
639643
fn = SGF.B.createWitnessMethod(
640644
Loc, lookupType, conformance, *constant,
641645
constantInfo.getSILType());
642646
} else {
643647
fn = SGF.B.createObjCMethod(Loc, borrowedSelf->getValue(),
644648
*constant, constantInfo.getSILType());
645649
}
646-
650+
S.pop();
647651
return ManagedValue::forUnmanaged(fn);
648652
}
649653
case Kind::DynamicMethod: {
650654
auto closureType = getDynamicMethodLoweredType(
651655
SGF.SGM.M, *constant, getSubstFormalType());
652656

653-
Scope S(SGF, Loc);
657+
ArgumentScope S(SGF, Loc);
654658
SILValue fn = SGF.B.createObjCMethod(
655659
Loc, borrowedSelf->getValue(), *constant,
656660
closureType);
661+
S.pop();
657662
return ManagedValue::forUnmanaged(fn);
658663
}
659664
}
@@ -1483,6 +1488,8 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
14831488
if (!fd || !fd->isObjC())
14841489
return false;
14851490

1491+
FormalEvaluationScope writebackScope(SGF);
1492+
14861493
// Local function that actually emits the dynamic member reference.
14871494
auto emitDynamicMemberRef = [&] {
14881495
// We found it. Emit the base.
@@ -1617,7 +1624,8 @@ static void emitRawApply(SILGenFunction &SGF,
16171624
bool isUnowned = substFnType->isCalleeUnowned();
16181625
SILValue fnValue =
16191626
isUnowned ? fn.getValue()
1620-
: isConsumed ? fn.forward(SGF) : fn.borrow(SGF, loc).getValue();
1627+
: isConsumed ? fn.forward(SGF)
1628+
: fn.formalAccessBorrow(SGF, loc).getValue();
16211629

16221630
SmallVector<SILValue, 4> argValues;
16231631

@@ -3002,6 +3010,14 @@ class ArgEmitter {
30023010

30033011
// Otherwise, just use the default logic.
30043012
value = SGF.emitRValueAsSingleValue(expr, contexts.FinalContext);
3013+
3014+
// We want any borrows done by the ownership-convention adjustment to
3015+
// happen outside of the formal-evaluation scope we pushed for the
3016+
// expression evaluation, but any copies to be done inside of it.
3017+
// Copies are only done if the parameter is consumed.
3018+
if (!param.isConsumed())
3019+
S.pop();
3020+
30053021
Args.push_back(convertOwnershipConvention(value));
30063022
return;
30073023
}
@@ -4451,7 +4467,7 @@ CallEmission::applyPartiallyAppliedSuperMethod(SGFContext C) {
44514467
auto functionTy = constantInfo.getSILType();
44524468
ManagedValue superMethod;
44534469
{
4454-
Scope S(SGF, loc);
4470+
ArgumentScope S(SGF, loc);
44554471
ManagedValue castValue =
44564472
borrowedCastToOriginalSelfType(SGF, loc, upcastedSelf);
44574473
if (!constant.isForeign) {
@@ -4461,6 +4477,7 @@ CallEmission::applyPartiallyAppliedSuperMethod(SGFContext C) {
44614477
superMethod = SGF.B.createObjCSuperMethod(loc, castValue, constant,
44624478
functionTy);
44634479
}
4480+
S.pop();
44644481
}
44654482
auto calleeConvention = ParameterConvention::Direct_Guaranteed;
44664483
auto closureTy = SILGenBuilder::getPartialApplyResultType(
@@ -5719,7 +5736,7 @@ ArgumentSource AccessorBaseArgPreparer::prepareAccessorObjectBaseArg() {
57195736

57205737
// We need to produce the value at +1 if it's going to be consumed.
57215738
if (selfParam.isConsumed() && !base.hasCleanup()) {
5722-
base = base.formalAccessCopyUnmanaged(SGF, loc);
5739+
base = base.copyUnmanaged(SGF, loc);
57235740
}
57245741

57255742
// If the parameter is indirect, we need to drop the value into

‎lib/SILGen/SILGenBridging.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ emitBridgeNativeToObjectiveC(SILGenFunction &SGF,
131131
AbstractionPattern origSelfType(witness->getInterfaceType());
132132
origSelfType = origSelfType.getFunctionParamType(0);
133133

134+
ArgumentScope scope(SGF, loc);
135+
134136
swiftValue = SGF.emitSubstToOrigValue(loc, swiftValue,
135137
origSelfType,
136138
swiftValueType,
@@ -158,6 +160,7 @@ emitBridgeNativeToObjectiveC(SILGenFunction &SGF,
158160
swiftValue.borrow(SGF, loc).getValue());
159161

160162
auto bridgedMV = SGF.emitManagedRValueWithCleanup(bridgedValue);
163+
bridgedMV = scope.popPreservingValue(bridgedMV);
161164

162165
// The Objective-C value doesn't necessarily match the desired type.
163166
bridgedMV = emitUnabstractedCast(SGF, loc, bridgedMV,
@@ -703,6 +706,8 @@ static ManagedValue emitNativeToCBridgedNonoptionalValue(SILGenFunction &SGF,
703706
if (nativeType->isExistentialType()) {
704707
auto openedType = ArchetypeType::getOpened(nativeType);
705708

709+
FormalEvaluationScope scope(SGF);
710+
706711
auto openedExistential = SGF.emitOpenExistential(
707712
loc, v, openedType, SGF.getLoweredType(openedType),
708713
AccessKind::Read);

‎lib/SILGen/SILGenBuilder.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ ManagedValue SILGenBuilder::createInitExistentialRef(
264264
ManagedValue SILGenBuilder::createStructExtract(SILLocation loc,
265265
ManagedValue base,
266266
VarDecl *decl) {
267-
ManagedValue borrowedBase = base.borrow(SGF, loc);
267+
ManagedValue borrowedBase = base.formalAccessBorrow(SGF, loc);
268268
SILValue extract = createStructExtract(loc, borrowedBase.getValue(), decl);
269269
return ManagedValue::forUnmanaged(extract);
270270
}
@@ -273,7 +273,7 @@ ManagedValue SILGenBuilder::createRefElementAddr(SILLocation loc,
273273
ManagedValue operand,
274274
VarDecl *field,
275275
SILType resultTy) {
276-
operand = operand.borrow(SGF, loc);
276+
operand = operand.formalAccessBorrow(SGF, loc);
277277
SILValue result = createRefElementAddr(loc, operand.getValue(), field);
278278
return ManagedValue::forUnmanaged(result);
279279
}
@@ -782,7 +782,7 @@ ManagedValue SILGenBuilder::createOpenExistentialRef(SILLocation loc,
782782
ManagedValue SILGenBuilder::createOpenExistentialValue(SILLocation loc,
783783
ManagedValue original,
784784
SILType type) {
785-
ManagedValue borrowedExistential = original.borrow(SGF, loc);
785+
ManagedValue borrowedExistential = original.formalAccessBorrow(SGF, loc);
786786
SILValue openedExistential =
787787
createOpenExistentialValue(loc, borrowedExistential.getValue(), type);
788788
return ManagedValue::forUnmanaged(openedExistential);
@@ -791,7 +791,7 @@ ManagedValue SILGenBuilder::createOpenExistentialValue(SILLocation loc,
791791
ManagedValue SILGenBuilder::createOpenExistentialBoxValue(SILLocation loc,
792792
ManagedValue original,
793793
SILType type) {
794-
ManagedValue borrowedExistential = original.borrow(SGF, loc);
794+
ManagedValue borrowedExistential = original.formalAccessBorrow(SGF, loc);
795795
SILValue openedExistential =
796796
createOpenExistentialBoxValue(loc, borrowedExistential.getValue(), type);
797797
return ManagedValue::forUnmanaged(openedExistential);

‎lib/SILGen/SILGenConvert.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,8 @@ SILGenFunction::emitOpenExistential(
930930
ArchetypeType *openedArchetype,
931931
SILType loweredOpenedType,
932932
AccessKind accessKind) {
933+
assert(isInFormalEvaluationScope());
934+
933935
// Open the existential value into the opened archetype value.
934936
bool isUnique = true;
935937
bool canConsume;

‎lib/SILGen/SILGenDestructor.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
#include "SILGenFunction.h"
1414
#include "RValue.h"
15-
#include "Scope.h"
15+
#include "ArgumentScope.h"
1616
#include "swift/AST/GenericSignature.h"
1717
#include "swift/AST/SubstitutionMap.h"
1818
#include "swift/SIL/TypeLowering.h"
@@ -74,20 +74,20 @@ void SILGenFunction::emitDestroyingDestructor(DestructorDecl *dd) {
7474
resultSelfValue = selfValue;
7575
}
7676

77-
{
78-
Scope S(Cleanups, cleanupLoc);
79-
ManagedValue borrowedValue =
80-
ManagedValue::forUnmanaged(resultSelfValue).borrow(*this, cleanupLoc);
81-
82-
if (classTy != borrowedValue.getType()) {
83-
borrowedValue =
84-
B.createUncheckedRefCast(cleanupLoc, borrowedValue, classTy);
85-
}
77+
ArgumentScope S(*this, Loc);
78+
ManagedValue borrowedValue =
79+
ManagedValue::forUnmanaged(resultSelfValue).borrow(*this, cleanupLoc);
8680

87-
// Release our members.
88-
emitClassMemberDestruction(borrowedValue, cd, cleanupLoc);
81+
if (classTy != borrowedValue.getType()) {
82+
borrowedValue =
83+
B.createUncheckedRefCast(cleanupLoc, borrowedValue, classTy);
8984
}
9085

86+
// Release our members.
87+
emitClassMemberDestruction(borrowedValue, cd, cleanupLoc);
88+
89+
S.pop();
90+
9191
if (resultSelfValue->getType() != objectPtrTy) {
9292
resultSelfValue =
9393
B.createUncheckedRefCast(cleanupLoc, resultSelfValue, objectPtrTy);

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,6 +2294,7 @@ class NominalTypeMemberRefRValueEmitter {
22942294
if (strategy.getKind() != AccessStrategy::Storage)
22952295
return None;
22962296

2297+
FormalEvaluationScope scope(SGF);
22972298
if (isa<StructDecl>(Base))
22982299
return emitStructDecl(SGF);
22992300
assert(isa<ClassDecl>(Base) && "Expected class");
@@ -2778,6 +2779,8 @@ emitKeyPathRValueBase(SILGenFunction &subSGF,
27782779
opened = subs.getReplacementTypes()[0]->castTo<ArchetypeType>();
27792780
}
27802781
assert(opened->isOpenedExistential());
2782+
2783+
FormalEvaluationScope scope(subSGF);
27812784

27822785
baseType = opened->getCanonicalType();
27832786
auto openedOpaqueValue = subSGF.emitOpenExistential(loc, paramSubstValue,
@@ -2962,7 +2965,7 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
29622965
indexPtrArg = entry->createFunctionArgument(indexArgTy);
29632966
}
29642967

2965-
Scope scope(subSGF, loc);
2968+
ArgumentScope scope(subSGF, loc);
29662969

29672970
auto baseSubstValue = emitKeyPathRValueBase(subSGF, property,
29682971
loc, baseArg,
@@ -5163,7 +5166,7 @@ RValue RValueEmitter::emitForceValue(ForceValueExpr *loc, Expr *E,
51635166
void SILGenFunction::emitOpenExistentialExprImpl(
51645167
OpenExistentialExpr *E,
51655168
llvm::function_ref<void(Expr *)> emitSubExpr) {
5166-
Optional<FormalEvaluationScope> writebackScope;
5169+
assert(isInFormalEvaluationScope());
51675170

51685171
// Emit the existential value.
51695172
if (E->getExistentialValue()->getType()->is<LValueType>()) {
@@ -5198,6 +5201,7 @@ RValue RValueEmitter::visitOpenExistentialExpr(OpenExistentialExpr *E,
51985201
return RValue(SGF, E, *result);
51995202
}
52005203

5204+
FormalEvaluationScope writebackScope(SGF);
52015205
return SGF.emitOpenExistentialExpr<RValue>(E,
52025206
[&](Expr *subExpr) -> RValue {
52035207
return visit(subExpr, C);

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2698,7 +2698,7 @@ LValue SILGenLValue::visitOpaqueValueExpr(OpaqueValueExpr *e,
26982698

26992699
RegularLocation loc(e);
27002700
LValue lv;
2701-
lv.add<ValueComponent>(entry.Value.borrow(SGF, loc), None,
2701+
lv.add<ValueComponent>(entry.Value.formalAccessBorrow(SGF, loc), None,
27022702
getValueTypeData(SGF, accessKind, e));
27032703
return lv;
27042704
}

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ static ManagedValue emitTransformExistential(SILGenFunction &SGF,
196196
SGFContext ctxt) {
197197
assert(inputType != outputType);
198198

199+
FormalEvaluationScope scope(SGF);
200+
199201
SILGenFunction::OpaqueValueState state;
200202
ArchetypeType *openedArchetype = nullptr;
201203

@@ -579,6 +581,8 @@ ManagedValue Transform::transform(ManagedValue v,
579581
// Unwrap zero or more metatype levels
580582
auto openedArchetype = getOpenedArchetype(openedType);
581583

584+
FormalEvaluationScope scope(SGF);
585+
582586
auto state = SGF.emitOpenExistential(Loc, v, openedArchetype,
583587
loweredOpenedType,
584588
AccessKind::Read);

‎test/ClangImporter/optional.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ class A {
2727
// CHECK: [[T0:%.*]] = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF
2828
// CHECK-NEXT: [[BORROWED_STR:%.*]] = begin_borrow [[STR]]
2929
// CHECK-NEXT: [[T1:%.*]] = apply [[T0]]([[BORROWED_STR]])
30-
// CHECK-NEXT: enum $Optional<NSString>, #Optional.some!enumelt.1, [[T1]]
3130
// CHECK-NEXT: end_borrow [[BORROWED_STR:%.*]]
31+
// CHECK-NEXT: enum $Optional<NSString>, #Optional.some!enumelt.1, [[T1]]
3232
// CHECK-NEXT: destroy_value [[STR]]
3333
// CHECK-NEXT: br
3434
// Nothing branch: inject nothing into result.

‎test/SILGen/foreign_errors.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ extension NSObject {
103103
// CHECK: [[T0:%.*]] = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF : $@convention(method) (@guaranteed String) -> @owned NSString
104104
// CHECK: [[BORROWED_RESULT:%.*]] = begin_borrow [[RESULT]]
105105
// CHECK: [[T1:%.*]] = apply [[T0]]([[BORROWED_RESULT]])
106-
// CHECK: [[T2:%.*]] = enum $Optional<NSString>, #Optional.some!enumelt.1, [[T1]] : $NSString
107106
// CHECK: end_borrow [[BORROWED_RESULT]]
107+
// CHECK: [[T2:%.*]] = enum $Optional<NSString>, #Optional.some!enumelt.1, [[T1]] : $NSString
108108
// CHECK: destroy_value [[RESULT]]
109109
// CHECK: br bb6([[T2]] : $Optional<NSString>)
110110
//

‎test/SILGen/guaranteed_normal_args.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ class KlassWithBuffer {
133133
// CHECK: [[BUF_BORROW:%.*]] = begin_borrow [[BUF]]
134134
// CHECK: [[K:%.*]] = struct_extract [[BUF_BORROW]] : $Buffer, #Buffer.k
135135
// CHECK: [[COPIED_K:%.*]] = copy_value [[K]]
136-
// CHECK: [[CASTED_COPIED_K:%.*]] = unchecked_ref_cast [[COPIED_K]]
137136
// CHECK: end_borrow [[BUF_BORROW]]
137+
// CHECK: [[CASTED_COPIED_K:%.*]] = unchecked_ref_cast [[COPIED_K]]
138138
// CHECK: destroy_value [[BUF]]
139139
// CHECK: return [[CASTED_COPIED_K]]
140140
// CHECK: } // end sil function '$ss15KlassWithBufferC03getC14AsNativeObjectBoyF'

‎test/SILGen/newtype.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ func createErrorDomain(str: String) -> ErrorDomain {
2626
// CHECK-RAW: [[BRIDGE_FN:%[0-9]+]] = function_ref @{{.*}}_bridgeToObjectiveC
2727
// CHECK-RAW: [[BORROWED_COPIED_STR:%.*]] = begin_borrow [[COPIED_STR]]
2828
// CHECK-RAW: [[BRIDGED:%[0-9]+]] = apply [[BRIDGE_FN]]([[BORROWED_COPIED_STR]])
29+
// CHECK-RAW: end_borrow [[BORROWED_COPIED_STR]]
2930
// CHECK-RAW: [[WRITE:%.*]] = begin_access [modify] [unknown] [[PB_BOX]]
3031
// CHECK-RAW: [[RAWVALUE_ADDR:%[0-9]+]] = struct_element_addr [[WRITE]]
3132
// CHECK-RAW: assign [[BRIDGED]] to [[RAWVALUE_ADDR]]
32-
// CHECK-RAW: end_borrow [[BORROWED_COPIED_STR]]
3333
// CHECK-RAW: end_borrow [[BORROWED_STR]]
3434

3535
func getRawValue(ed: ErrorDomain) -> String {

0 commit comments

Comments
 (0)