Skip to content

Commit f9d9d75

Browse files
committed
SILGen: Change codegen for loading from no-implicit-copy lvalues.
Previously, we would emit this as %1 = load [copy] %address %2 = moveonly_wrapper_to_copyable [owned] %1 which is difficult for move-only checking to handle, since %2 looks like a consume of %1, making it difficult to determine %1's true lifetime. Change this to %1 = moveonly_wrapper_to_copyable_addr %address %2 = load [copy] %address which is handled better by move-only checking, improving the accuracy of existing move-checking as well as fixing a spurious diagnostic when indirect parameters get passed as by-value arguments to other functions.
1 parent 31ec175 commit f9d9d75

File tree

4 files changed

+110
-73
lines changed

4 files changed

+110
-73
lines changed

lib/SILGen/SILGenLValue.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5015,23 +5015,22 @@ SILValue SILGenFunction::emitSemanticLoad(SILLocation loc,
50155015
}
50165016

50175017
// Harder case: the srcTL and the rvalueTL match without move only.
5018-
if (srcType.removingMoveOnlyWrapper() ==
5019-
rvalueType.removingMoveOnlyWrapper()) {
5018+
if (srcType.removingMoveOnlyWrapper()
5019+
== rvalueType.removingMoveOnlyWrapper()) {
50205020
// Ok, we know that one must be move only and the other must not be. Thus we
50215021
// perform one of two things:
50225022
//
50235023
// 1. If our source address is move only and our rvalue type is not move
5024-
// only, lets perform a load [copy] and a moveonly_to_copyable. We just need
5025-
// to insert something so that the move only checker knows that this copy of
5026-
// the move only address must be a last use.
5024+
// only, let's perform a load [copy] from the unwrapped address.
50275025
//
50285026
// 2. If our dest value type is move only and our rvalue type is not move
50295027
// only, then we perform a load [copy] + copyable_to_moveonly.
5030-
SILValue newCopy = srcTL.emitLoadOfCopy(B, loc, src, isTake);
5031-
if (newCopy->getType().isMoveOnlyWrapped()) {
5032-
return B.createOwnedMoveOnlyWrapperToCopyableValue(loc, newCopy);
5028+
if (src->getType().isMoveOnlyWrapped()) {
5029+
auto copyableSrc = B.createMoveOnlyWrapperToCopyableAddr(loc, src);
5030+
return rvalueTL.emitLoadOfCopy(B, loc, copyableSrc, isTake);
50335031
}
50345032

5033+
SILValue newCopy = srcTL.emitLoadOfCopy(B, loc, src, isTake);
50355034
return B.createOwnedCopyableToMoveOnlyWrapperValue(loc, newCopy);
50365035
}
50375036

test/Concurrency/transfernonsendable_ownership.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,20 @@ func testConsumingError(_ x: consuming Klass) async {
5050
print(x)
5151
}
5252

53-
func testConsumingUseAfterConsumeError(_ x: consuming Klass) async { // expected-error {{'x' consumed more than once}}
53+
func testConsumingUseAfterConsumeError(_ x: consuming Klass) async { // expected-error {{'x' used after consume}}
5454
await consumeTransferToMain(x) // expected-warning {{sending 'x' risks causing data races}}
5555
// expected-note @-1 {{sending task-isolated 'x' to main actor-isolated global function 'consumeTransferToMain' risks causing data races between main actor-isolated and task-isolated uses}}
5656
// expected-note @-2 {{consumed here}}
5757
print(x)
58-
// expected-note @-1 {{consumed again here}}
58+
// expected-note @-1 {{used here}}
5959
}
6060

61-
@CustomActor func testConsumingUseAfterConsumeErrorGlobalActor(_ x: consuming Klass) async { // expected-error {{'x' consumed more than once}}
61+
@CustomActor func testConsumingUseAfterConsumeErrorGlobalActor(_ x: consuming Klass) async { // expected-error {{'x' used after consume}}
6262
await consumeTransferToMain(x) // expected-warning {{sending 'x' risks causing data races}}
6363
// expected-note @-1 {{sending global actor 'CustomActor'-isolated 'x' to main actor-isolated global function 'consumeTransferToMain' risks causing data races between main actor-isolated and global actor 'CustomActor'-isolated uses}}
6464
// expected-note @-2 {{consumed here}}
6565
print(x)
66-
// expected-note @-1 {{consumed again here}}
66+
// expected-note @-1 {{used here}}
6767
}
6868

6969
func testBorrowing(_ x: borrowing Klass) async {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %target-swift-frontend -emit-sil -enable-experimental-feature BuiltinModule -enable-experimental-feature LifetimeDependence -enable-experimental-feature AddressableTypes -enable-experimental-feature AddressableParameters -verify %s
2+
3+
// REQUIRES: swift_feature_BuiltinModule
4+
// REQUIRES: swift_feature_AddressableParameters
5+
// REQUIRES: swift_feature_AddressableTypes
6+
// REQUIRES: swift_feature_LifetimeDependence
7+
8+
@_addressableForDependencies
9+
struct Node {
10+
var id: AnyObject
11+
12+
func grungle() {}
13+
}
14+
15+
struct NodeRef: ~Escapable {
16+
private var parent: UnsafePointer<Node>
17+
18+
@lifetime(borrow node)
19+
init(node: borrowing Node) { fatalError() }
20+
}
21+
22+
// Ensure there aren't spurious errors about consumption when an addressable
23+
// parameter is passed as a normal loadable parameter to another function
24+
// or method.
25+
@lifetime(borrow node)
26+
func test(node: borrowing Node) -> NodeRef {
27+
node.grungle()
28+
return NodeRef(node: node)
29+
}
30+

0 commit comments

Comments
 (0)