Skip to content

Commit be7667b

Browse files
committed
[move-only] Teach the move checker how to correctly check an address only type used by escaping closures.
When we have a loadable type, SILGen always eagerly loads the value, so the move checker handled checking those cases by just looking for loads and emitting the error based off of a load. Of course for address only types, the codegen looks slightly different (we consume the value directly rather than load it). So this change just teaches the checker how to do that. rdar://105106470
1 parent a0a3aec commit be7667b

File tree

3 files changed

+101
-4
lines changed

3 files changed

+101
-4
lines changed

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,9 +1728,30 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
17281728
// Now that we have handled or loadTakeOrCopy, we need to now track our
17291729
// additional pure takes.
17301730
if (::memInstMustConsume(op)) {
1731+
// If we don't have a consumeable and assignable check kind, then we can't
1732+
// consume. Emit an error.
1733+
//
1734+
// NOTE: Since SILGen eagerly loads loadable types from memory, this
1735+
// generally will only handle address only types.
1736+
if (markedValue->getCheckKind() !=
1737+
MarkMustCheckInst::CheckKind::ConsumableAndAssignable) {
1738+
auto *fArg = dyn_cast<SILFunctionArgument>(
1739+
stripAccessMarkers(markedValue->getOperand()));
1740+
if (fArg && fArg->isClosureCapture() && fArg->getType().isAddress()) {
1741+
moveChecker.diagnosticEmitter.emitPromotedBoxArgumentError(markedValue,
1742+
fArg);
1743+
} else {
1744+
moveChecker.diagnosticEmitter
1745+
.emitAddressEscapingClosureCaptureLoadedAndConsumed(markedValue);
1746+
}
1747+
emittedEarlyDiagnostic = true;
1748+
return true;
1749+
}
1750+
17311751
auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
17321752
if (!leafRange)
17331753
return false;
1754+
17341755
LLVM_DEBUG(llvm::dbgs() << "Pure consuming use: " << *user);
17351756
useState.takeInsts.insert({user, *leafRange});
17361757
return true;
@@ -2430,7 +2451,6 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24302451
LLVM_DEBUG(llvm::dbgs() << "Failed access path visit: " << *markedAddress);
24312452
return false;
24322453
}
2433-
addressUseState.initializeInOutTermUsers();
24342454

24352455
// If we found a load [copy] or copy_addr that requires multiple copies or an
24362456
// exclusivity error, then we emitted an early error. Bail now and allow the
@@ -2445,9 +2465,14 @@ bool MoveOnlyAddressCheckerPImpl::performSingleCheck(
24452465
if (diagCount != diagnosticEmitter.getDiagnosticCount())
24462466
return true;
24472467

2448-
// Then check if we emitted an error. If we did not, return true.
2449-
if (diagCount != diagnosticEmitter.getDiagnosticCount())
2450-
return true;
2468+
// Now that we know that we have run our visitor and did not emit any errors
2469+
// and successfully visited everything, see if have any
2470+
// assignable_but_not_consumable of address only types that are consumed.
2471+
//
2472+
// DISCUSSION: For non address only types, this is not an issue since we
2473+
// eagerly load
2474+
2475+
addressUseState.initializeInOutTermUsers();
24512476

24522477
//===---
24532478
// Liveness Checking

test/SILOptimizer/moveonly_addresschecker_diagnostics.sil

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,30 @@ public struct AggStruct {
5959
var pair: KlassPair
6060
}
6161

62+
protocol P {
63+
static var value: Self { get }
64+
static var value2: any P { get }
65+
}
66+
67+
@_moveOnly
68+
public struct AddressOnlyGeneric<T : P> {
69+
var copyable: T
70+
var moveOnly: NonTrivialStruct
71+
72+
init()
73+
init(_ input1: T)
74+
}
75+
6276
sil @get_aggstruct : $@convention(thin) () -> @owned AggStruct
6377
sil @nonConsumingUseKlass : $@convention(thin) (@guaranteed Klass) -> ()
6478
sil @nonConsumingUseNonTrivialStruct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
6579
sil @classConsume : $@convention(thin) (@owned Klass) -> () // user: %18
6680
sil @copyableClassConsume : $@convention(thin) (@owned CopyableKlass) -> () // user: %24
6781
sil @copyableClassUseMoveOnlyWithoutEscaping : $@convention(thin) (@guaranteed CopyableKlass) -> () // user: %16
82+
sil @getAddressOnlyGeneric : $@convention(thin) <τ_0_0 where τ_0_0 : P> () -> @out AddressOnlyGeneric<τ_0_0>
83+
sil @addressOnlyGenericInOutUse : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@inout AddressOnlyGeneric<τ_0_0>) -> ()
84+
sil @addressOnlyGenericConsume : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@in AddressOnlyGeneric<τ_0_0>) -> ()
85+
sil @addressOnlyGenericUse : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@in_guaranteed AddressOnlyGeneric<τ_0_0>) -> ()
6886

6987
///////////
7088
// Tests //
@@ -316,3 +334,34 @@ bb0(%0 : $*Klass):
316334
%27 = tuple ()
317335
return %27 : $()
318336
}
337+
338+
////////////////////////
339+
// Address Only Tests //
340+
////////////////////////
341+
342+
sil [ossa] @inoutCaptureTestAddressOnlyGenericClosure2 : $@convention(thin) <T where T : P> (@guaranteed <τ_0_0 where τ_0_0 : P> { var AddressOnlyGeneric<τ_0_0> } <T>) -> () {
343+
bb0(%0 : @closureCapture @guaranteed $<τ_0_0 where τ_0_0 : P> { var AddressOnlyGeneric<τ_0_0> } <T>):
344+
%1 = project_box %0 : $<τ_0_0 where τ_0_0 : P> { var AddressOnlyGeneric<τ_0_0> } <T>, 0
345+
debug_value %1 : $*AddressOnlyGeneric<T>, var, name "x", argno 1, expr op_deref
346+
%3 = alloc_stack $AddressOnlyGeneric<T>
347+
%5 = function_ref @getAddressOnlyGeneric : $@convention(thin) <τ_0_0 where τ_0_0 : P> () -> @out AddressOnlyGeneric<τ_0_0>
348+
%6 = apply %5<T>(%3) : $@convention(thin) <τ_0_0 where τ_0_0 : P> () -> @out AddressOnlyGeneric<τ_0_0>
349+
%7 = begin_access [modify] [dynamic] %1 : $*AddressOnlyGeneric<T>
350+
%8 = mark_must_check [assignable_but_not_consumable] %7 : $*AddressOnlyGeneric<T>
351+
copy_addr [take] %3 to %8 : $*AddressOnlyGeneric<T>
352+
end_access %7 : $*AddressOnlyGeneric<T>
353+
dealloc_stack %3 : $*AddressOnlyGeneric<T>
354+
%12 = begin_access [modify] [dynamic] %1 : $*AddressOnlyGeneric<T>
355+
%13 = mark_must_check [assignable_but_not_consumable] %12 : $*AddressOnlyGeneric<T>
356+
%14 = function_ref @addressOnlyGenericInOutUse : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@inout AddressOnlyGeneric<τ_0_0>) -> ()
357+
%15 = apply %14<T>(%13) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@inout AddressOnlyGeneric<τ_0_0>) -> ()
358+
end_access %12 : $*AddressOnlyGeneric<T>
359+
%17 = begin_access [deinit] [dynamic] %1 : $*AddressOnlyGeneric<T>
360+
%18 = mark_must_check [assignable_but_not_consumable] %17 : $*AddressOnlyGeneric<T>
361+
// expected-error @-1 {{'x' was consumed but it is illegal to consume a noncopyable mutable capture of an escaping closure. One can only read from it or assign over it}}
362+
%19 = function_ref @addressOnlyGenericConsume : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@in AddressOnlyGeneric<τ_0_0>) -> ()
363+
%20 = apply %19<T>(%18) : $@convention(thin) <τ_0_0 where τ_0_0 : P> (@in AddressOnlyGeneric<τ_0_0>) -> ()
364+
end_access %17 : $*AddressOnlyGeneric<T>
365+
%22 = tuple ()
366+
return %22 : $()
367+
}

test/SILOptimizer/moveonly_addresschecker_diagnostics.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3609,6 +3609,29 @@ func inoutCaptureTest() -> (() -> ()) {
36093609
return f
36103610
}
36113611

3612+
func inoutCaptureTestAddressOnlyGeneric<T : P>(_ t: T.Type) -> (() -> ()) {
3613+
var x = AddressOnlyGeneric<T>()
3614+
x = AddressOnlyGeneric<T>()
3615+
3616+
func useInOut(_ x: inout AddressOnlyGeneric<T>) {}
3617+
let f = {
3618+
useInOut(&x)
3619+
}
3620+
3621+
borrowVal(x)
3622+
consumeVal(x) // expected-error {{'x' was consumed but it is illegal to consume a noncopyable mutable capture of an escaping closure. One can only read from it or assign over it}}
3623+
x = AddressOnlyGeneric<T>()
3624+
3625+
let g = {
3626+
x = AddressOnlyGeneric<T>()
3627+
useInOut(&x)
3628+
consumeVal(x) // expected-error {{'x' was consumed but it is illegal to consume a noncopyable mutable capture of an escaping closure. One can only read from it or assign over it}}
3629+
}
3630+
g()
3631+
3632+
return f
3633+
}
3634+
36123635
////////////////
36133636
// Misc Tests //
36143637
////////////////

0 commit comments

Comments
 (0)