Skip to content

Commit 58bf935

Browse files
authored
Merge pull request #70475 from jckarter/noncopyable-addressors
Move-only check the value projected from addressors.
2 parents daa4516 + 1b9a071 commit 58bf935

File tree

4 files changed

+169
-19
lines changed

4 files changed

+169
-19
lines changed

lib/SILGen/SILGenApply.cpp

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3716,19 +3716,6 @@ class ArgEmitter {
37163716
Args.push_back(ManagedValue());
37173717
}
37183718

3719-
void emitExpandedConsumed(Expr *arg, AbstractionPattern origParamType) {
3720-
CanType substArgType = arg->getType()->getCanonicalType();
3721-
auto count = getFlattenedValueCount(origParamType);
3722-
auto claimedParams = claimNextParameters(count);
3723-
3724-
SILType loweredSubstArgType = SGF.getLoweredType(substArgType);
3725-
SILType loweredSubstParamType =
3726-
SGF.getLoweredType(origParamType, substArgType);
3727-
3728-
return emitConsumed(arg, loweredSubstArgType, loweredSubstParamType,
3729-
origParamType, claimedParams);
3730-
}
3731-
37323719
void
37333720
emitDirect(ArgumentSource &&arg, SILType loweredSubstArgType,
37343721
AbstractionPattern origParamType, SILParameterInfo param,

lib/SILGen/SILGenLValue.cpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2009,6 +2009,25 @@ namespace {
20092009
class AddressorComponent
20102010
: public AccessorBasedComponent<PhysicalPathComponent> {
20112011
SILType SubstFieldType;
2012+
2013+
static SGFAccessKind getAccessKindForAddressor(SGFAccessKind accessKind) {
2014+
// Addressors cannot be consumed through.
2015+
switch (accessKind) {
2016+
case SGFAccessKind::IgnoredRead:
2017+
case SGFAccessKind::BorrowedAddressRead:
2018+
case SGFAccessKind::BorrowedObjectRead:
2019+
case SGFAccessKind::OwnedAddressRead:
2020+
case SGFAccessKind::OwnedObjectRead:
2021+
case SGFAccessKind::Write:
2022+
case SGFAccessKind::ReadWrite:
2023+
return accessKind;
2024+
2025+
case SGFAccessKind::OwnedAddressConsume:
2026+
case SGFAccessKind::OwnedObjectConsume:
2027+
return SGFAccessKind::ReadWrite;
2028+
}
2029+
llvm_unreachable("uncovered switch");
2030+
}
20122031
public:
20132032
AddressorComponent(AbstractStorageDecl *decl, SILDeclRef accessor,
20142033
bool isSuper,
@@ -2048,9 +2067,21 @@ namespace {
20482067

20492068
// Enter an unsafe access scope for the access.
20502069
addr =
2051-
enterAccessScope(SGF, loc, base, addr, getTypeData(), getAccessKind(),
2070+
enterAccessScope(SGF, loc, base, addr, getTypeData(),
2071+
getAccessKindForAddressor(getAccessKind()),
20522072
SILAccessEnforcement::Unsafe, ActorIso);
20532073

2074+
// Validate the use of the access if it's noncopyable.
2075+
if (addr.getType().isMoveOnly()) {
2076+
MarkUnresolvedNonCopyableValueInst::CheckKind kind
2077+
= getAccessorDecl()->getAccessorKind() == AccessorKind::MutableAddress
2078+
? MarkUnresolvedNonCopyableValueInst::CheckKind::ConsumableAndAssignable
2079+
: MarkUnresolvedNonCopyableValueInst::CheckKind::NoConsumeOrAssign;
2080+
auto checkedAddr = SGF.B.createMarkUnresolvedNonCopyableValueInst(
2081+
loc, addr.getValue(), kind);
2082+
addr = std::move(addr).transform(checkedAddr);
2083+
}
2084+
20542085
return addr;
20552086
}
20562087

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,12 @@ static bool isInOutDefThatNeedsEndOfFunctionLiveness(
379379
MarkUnresolvedNonCopyableValueInst *markedAddr) {
380380
SILValue operand = markedAddr->getOperand();
381381

382-
// Check for inout types of arguments that are marked with consumable and
383-
// assignable.
384382
if (markedAddr->getCheckKind() ==
385383
MarkUnresolvedNonCopyableValueInst::CheckKind::ConsumableAndAssignable) {
384+
// TODO: This should really be a property of the marker instruction.
385+
386+
// Check for inout types of arguments that are marked with consumable and
387+
// assignable.
386388
if (auto *fArg = dyn_cast<SILFunctionArgument>(operand)) {
387389
switch (fArg->getArgumentConvention()) {
388390
case SILArgumentConvention::Indirect_In:
@@ -402,9 +404,14 @@ static bool isInOutDefThatNeedsEndOfFunctionLiveness(
402404
return true;
403405
}
404406
}
407+
// Check for yields from a modify coroutine.
405408
if (auto bai = dyn_cast_or_null<BeginApplyInst>(operand->getDefiningInstruction())) {
406409
return true;
407410
}
411+
// Check for modify accesses.
412+
if (auto access = dyn_cast<BeginAccessInst>(operand)) {
413+
return access->getAccessKind() == SILAccessKind::Modify;
414+
}
408415
}
409416

410417
return false;
@@ -2181,8 +2188,10 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
21812188
if (moveChecker.canonicalizer.foundAnyConsumingUses()) {
21822189
LLVM_DEBUG(llvm::dbgs()
21832190
<< "Found mark must check [nocopy] error: " << *user);
2184-
auto *fArg = dyn_cast<SILFunctionArgument>(
2185-
stripAccessMarkers(markedValue->getOperand()));
2191+
auto operand = stripAccessMarkers(markedValue->getOperand());
2192+
auto *fArg = dyn_cast<SILFunctionArgument>(operand);
2193+
auto *ptrToAddr = dyn_cast<PointerToAddressInst>(operand);
2194+
21862195
// If we have a closure captured that we specialized, we should have a
21872196
// no consume or assign and should emit a normal guaranteed diagnostic.
21882197
if (fArg && fArg->isClosureCapture() &&
@@ -2198,7 +2207,7 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
21982207
// If we have a function argument that is no_consume_or_assign and we do
21992208
// not have any partial apply uses, then we know that we have a use of
22002209
// an address only borrowed parameter that we need to
2201-
if (fArg &&
2210+
if ((fArg || ptrToAddr) &&
22022211
checkKind == MarkUnresolvedNonCopyableValueInst::CheckKind::
22032212
NoConsumeOrAssign &&
22042213
!moveChecker.canonicalizer.hasPartialApplyConsumingUse()) {
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// RUN: %target-swift-frontend -enable-experimental-feature NonescapableTypes -enable-experimental-feature BuiltinModule -enable-experimental-feature NoncopyableGenerics -parse-stdlib -module-name Swift -DADDRESS_ONLY -emit-sil -verify %s
2+
// RUN: %target-swift-frontend -enable-experimental-feature NonescapableTypes -enable-experimental-feature BuiltinModule -enable-experimental-feature NoncopyableGenerics -parse-stdlib -module-name Swift -DLOADABLE -emit-sil -verify %s
3+
// RUN: %target-swift-frontend -enable-experimental-feature NonescapableTypes -enable-experimental-feature BuiltinModule -enable-experimental-feature NoncopyableGenerics -parse-stdlib -module-name Swift -DTRIVIAL -emit-sil -verify %s
4+
// RUN: %target-swift-frontend -enable-experimental-feature NonescapableTypes -enable-experimental-feature BuiltinModule -enable-experimental-feature NoncopyableGenerics -parse-stdlib -module-name Swift -DEMPTY -emit-sil -verify %s
5+
6+
// TODO: Use the real stdlib types once `UnsafePointer` supports noncopyable
7+
// types.
8+
9+
import Builtin
10+
11+
@_marker public protocol Copyable: ~Escapable {}
12+
@_marker public protocol Escapable: ~Copyable {}
13+
@frozen public struct UnsafePointer<T: ~Copyable>: Copyable {
14+
var value: Builtin.RawPointer
15+
}
16+
17+
@frozen public struct UnsafeMutablePointer<T: ~Copyable>: Copyable {
18+
var value: Builtin.RawPointer
19+
}
20+
21+
@frozen public struct Int { var value: Builtin.Word }
22+
23+
@_silgen_name("makeUpAPointer")
24+
func makeUpAPointer<T: ~Copyable>() -> UnsafePointer<T>
25+
@_silgen_name("makeUpAMutablePointer")
26+
func makeUpAPointer<T: ~Copyable>() -> UnsafeMutablePointer<T>
27+
@_silgen_name("makeUpAnInt")
28+
func makeUpAnInt() -> Int
29+
30+
class X {}
31+
32+
struct NC: ~Copyable {
33+
#if EMPTY
34+
#elseif TRIVIAL
35+
var x: Int = makeUpAnInt()
36+
#elseif LOADABLE
37+
var x: X = X()
38+
#elseif ADDRESS_ONLY
39+
var x: Any = X()
40+
#else
41+
#error("pick a mode")
42+
#endif
43+
deinit {}
44+
}
45+
46+
struct S {
47+
var data: NC {
48+
unsafeAddress { return makeUpAPointer() }
49+
}
50+
51+
var mutableData: NC {
52+
unsafeAddress { return makeUpAPointer() }
53+
unsafeMutableAddress { return makeUpAPointer() }
54+
}
55+
}
56+
57+
struct SNC: ~Copyable {
58+
var data: NC {
59+
unsafeAddress { return makeUpAPointer() }
60+
}
61+
62+
var mutableData: NC {
63+
unsafeAddress { return makeUpAPointer() }
64+
unsafeMutableAddress { return makeUpAPointer() }
65+
}
66+
}
67+
68+
class C {
69+
final var data: NC {
70+
unsafeAddress { return makeUpAPointer() }
71+
}
72+
73+
final var mutableData: NC {
74+
unsafeAddress { return makeUpAPointer() }
75+
unsafeMutableAddress { return makeUpAPointer() }
76+
}
77+
}
78+
79+
func borrow(_ nc: borrowing NC) {}
80+
func mod(_ nc: inout NC) {}
81+
func take(_ nc: consuming NC) {}
82+
83+
// TODO: Resolve thing being consumed more precisely than 'unknown'.
84+
// TODO: Use more specific diagnostic than "reinitialization of inout parameter"
85+
86+
func test(c: C) {
87+
borrow(c.data)
88+
take(c.data) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
89+
90+
borrow(c.mutableData)
91+
mod(&c.mutableData)
92+
take(c.mutableData) // expected-error{{missing reinitialization of inout parameter 'unknown' after consume}} expected-note{{consumed here}}
93+
}
94+
func test(s: S) {
95+
borrow(s.data)
96+
take(s.data) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
97+
98+
borrow(s.mutableData)
99+
take(s.mutableData) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
100+
}
101+
func test(mut_s s: inout S) {
102+
borrow(s.data)
103+
take(s.data) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
104+
105+
borrow(s.mutableData)
106+
mod(&s.mutableData)
107+
take(s.mutableData) // expected-error{{missing reinitialization of inout parameter 'unknown' after consume}} expected-note{{consumed here}}
108+
}
109+
func test(snc: borrowing SNC) {
110+
borrow(snc.data)
111+
take(snc.data) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
112+
113+
borrow(snc.mutableData)
114+
take(snc.mutableData) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
115+
}
116+
func test(mut_snc snc: inout SNC) {
117+
borrow(snc.data)
118+
take(snc.data) // expected-error{{'unknown' is borrowed and cannot be consumed}} expected-note{{consumed here}}
119+
120+
borrow(snc.mutableData)
121+
mod(&snc.mutableData)
122+
take(snc.mutableData) // expected-error{{missing reinitialization of inout parameter 'unknown' after consume}} expected-note{{consumed here}}
123+
}

0 commit comments

Comments
 (0)