Skip to content

Commit bfdbc76

Browse files
committed
[region-isolation] Convert UseAfterTransfer's typed isolation transfer error to the split small error/large note format we are standardizing on.
I also cleaned up the diagnostic a little bit.
1 parent 4713880 commit bfdbc76

8 files changed

+153
-23
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,16 @@ ERROR(regionbasedisolation_named_transfer_yields_race, none,
971971
NOTE(regionbasedisolation_type_is_non_sendable, none,
972972
"%0 is a non-Sendable type",
973973
(Type))
974+
ERROR(regionbasedisolation_type_transfer_yields_race, none,
975+
"sending value of non-Sendable type %0 risks causing data races",
976+
(Type))
977+
978+
NOTE(regionbasedisolation_type_use_after_transfer, none,
979+
"sending value of non-Sendable type %0 to %1 callee risks causing data races between %1 and local %2 uses",
980+
(Type, ActorIsolation, ActorIsolation))
981+
NOTE(regionbasedisolation_type_use_after_transfer_callee, none,
982+
"sending value of non-Sendable type %0 to %1 %2 %3 risks causing data races between %1 and local %4 uses",
983+
(Type, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
974984

975985
ERROR(regionbasedisolation_inout_sending_cannot_be_actor_isolated, none,
976986
"'inout sending' parameter %0 cannot be %1at end of function",

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -701,12 +701,21 @@ class UseAfterTransferDiagnosticEmitter {
701701

702702
void emitTypedIsolationCrossing(SILLocation loc, Type inferredType,
703703
ApplyIsolationCrossing isolationCrossing) {
704-
diagnoseError(
705-
loc, diag::regionbasedisolation_transfer_yields_race_with_isolation,
706-
inferredType, isolationCrossing.getCallerIsolation(),
707-
isolationCrossing.getCalleeIsolation())
708-
.highlight(loc.getSourceRange())
704+
diagnoseError(loc, diag::regionbasedisolation_type_transfer_yields_race,
705+
inferredType)
709706
.limitBehaviorIf(getBehaviorLimit());
707+
708+
if (auto calleeInfo = getTransferringCalleeInfo()) {
709+
diagnoseNote(loc,
710+
diag::regionbasedisolation_type_use_after_transfer_callee,
711+
inferredType, isolationCrossing.getCalleeIsolation(),
712+
calleeInfo->first, calleeInfo->second,
713+
isolationCrossing.getCallerIsolation());
714+
} else {
715+
diagnoseNote(loc, diag::regionbasedisolation_type_use_after_transfer,
716+
inferredType, isolationCrossing.getCalleeIsolation(),
717+
isolationCrossing.getCallerIsolation());
718+
}
710719
emitRequireInstDiagnostics();
711720
}
712721

test/Concurrency/transfernonsendable.sil

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ bb0:
159159
%f2 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
160160
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f2<NonSendableKlass>(%1) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
161161
// expected-warning @-1 {{}}
162+
// expected-note @-2 {{}}
162163
%useIndirect = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
163164
apply %useIndirect<NonSendableKlass>(%1) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
164165
// expected-note @-1 {{access can happen concurrently}}

test/Concurrency/transfernonsendable_cfg.sil

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ bb3:
4545
%transferKlass = function_ref @transferKlass : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
4646
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transferKlass(%klass) : $@convention(thin) @async (@guaranteed NonSendableKlass) -> ()
4747
// expected-warning @-1 {{}}
48+
// expected-note @-2 {{}}
4849
%useKlass = function_ref @useKlass : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
4950
apply %useKlass(%klass) : $@convention(thin) (@guaranteed NonSendableKlass) -> ()
5051
// expected-note @-1 {{access can happen concurrently}}

test/Concurrency/transfernonsendable_instruction_matching.sil

Lines changed: 69 additions & 5 deletions
Large diffs are not rendered by default.

test/Concurrency/transfernonsendable_instruction_matching_differentiability.sil

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ bb0:
8484

8585
%transfer = function_ref @transferLinearFunction : $@async @convention(thin) (@guaranteed @differentiable(_linear) @callee_guaranteed (Float) -> Float) -> ()
8686
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transfer(%0) : $@async @convention(thin) (@guaranteed @differentiable(_linear) @callee_guaranteed (Float) -> Float) -> () // expected-warning {{}}
87+
// expected-note @-1 {{}}
8788

8889
// No error since this is just look through.
8990
%1 = begin_borrow %0 : $@differentiable(_linear) @callee_guaranteed (Float) -> Float
@@ -104,6 +105,7 @@ bb0:
104105

105106
%transfer = function_ref @transferDifferentiableFunction : $@async @convention(thin) (@guaranteed @differentiable(reverse) @callee_guaranteed (Float) -> Float) -> ()
106107
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transfer(%0) : $@async @convention(thin) (@guaranteed @differentiable(reverse) @callee_guaranteed (Float) -> Float) -> () // expected-warning {{}}
108+
// expected-note @-1 {{}}
107109

108110
// No error since this is just look through.
109111
%1 = begin_borrow %0 : $@differentiable(reverse) @callee_guaranteed (Float) -> Float
@@ -124,6 +126,7 @@ bb0:
124126

125127
%transfer = function_ref @transferFunction : $@async @convention(thin) (@guaranteed @callee_guaranteed (Float) -> Float) -> ()
126128
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transfer(%1) : $@async @convention(thin) (@guaranteed @callee_guaranteed (Float) -> Float) -> () // expected-warning {{}}
129+
// expected-note @-1 {{}}
127130

128131
%2 = begin_borrow %1 : $@callee_guaranteed (Float) -> Float
129132
%3 = linear_function [parameters 0] %2 : $@callee_guaranteed (Float) -> Float // expected-note {{access can happen concurrently}}
@@ -145,6 +148,7 @@ bb0:
145148

146149
%transfer = function_ref @transferFunction : $@async @convention(thin) (@guaranteed @callee_guaranteed (Float) -> Float) -> ()
147150
apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %transfer(%1) : $@async @convention(thin) (@guaranteed @callee_guaranteed (Float) -> Float) -> () // expected-warning {{}}
151+
// expected-note @-1 {{}}
148152

149153
%derivative = function_ref @derivative : $@convention(thin) (Float) -> (Float, @owned @callee_guaranteed (Float) -> Float)
150154
%vjp = function_ref @vjp : $@convention(thin) (Float) -> (Float, @owned @callee_guaranteed (Float) -> Float)

test/Concurrency/transfernonsendable_region_based_sendability.swift

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -667,8 +667,8 @@ func test_tuple_formation(a : A, i : Int) async {
667667
foo_noniso(ns012);
668668
foo_noniso(ns13);
669669
case 4:
670-
await a.foo(ns012) // expected-tns-warning {{sending value of non-Sendable type '(NonSendable, NonSendable, NonSendable)' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
671-
// TODO: Interestingly with complete, we emit this error 3 times?!
670+
await a.foo(ns012) // expected-tns-warning {{sending value of non-Sendable type '(NonSendable, NonSendable, NonSendable)' risks causing data races}}
671+
// expected-tns-note @-1 {{sending value of non-Sendable type '(NonSendable, NonSendable, NonSendable)' to actor-isolated instance method 'foo' risks causing data races between actor-isolated and local nonisolated uses}}
672672
// expected-complete-warning @-2 3{{passing argument of non-sendable type '(NonSendable, NonSendable, NonSendable)' into actor-isolated context may introduce data races}}
673673

674674
if bool {
@@ -687,7 +687,8 @@ func test_tuple_formation(a : A, i : Int) async {
687687
foo_noniso(ns13); // expected-tns-note {{access can happen concurrently}}
688688
}
689689
default:
690-
await a.foo(ns13) // expected-tns-warning {{sending value of non-Sendable type '(NonSendable, NonSendable)' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
690+
await a.foo(ns13) // expected-tns-warning {{sending value of non-Sendable type '(NonSendable, NonSendable)' risks causing data races}}
691+
// expected-tns-note @-1 {{sending value of non-Sendable type '(NonSendable, NonSendable)' to actor-isolated instance method 'foo' risks causing data races between actor-isolated and local nonisolated uses}}
691692
// expected-complete-warning @-1 2{{passing argument of non-sendable type '(NonSendable, NonSendable)' into actor-isolated context may introduce data races}}
692693

693694
if bool {
@@ -823,9 +824,10 @@ func one_consume_many_require_varag(a : A) async {
823824
let ns3 = NonSendable();
824825
let ns4 = NonSendable();
825826

826-
//TODO: find a way to make the type used in the diagnostic more specific than the signature type
827+
// TODO: find a way to make the type used in the diagnostic more specific than the signature type
827828
await a.foo_vararg(ns0, ns1, ns2);
828-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
829+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
830+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
829831
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
830832

831833
if bool {
@@ -843,7 +845,8 @@ func one_consume_one_require_vararg(a : A) async {
843845
let ns2 = NonSendable();
844846

845847
await a.foo_vararg(ns0, ns1, ns2);
846-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
848+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
849+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
847850
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
848851

849852
foo_noniso_vararg(ns0, ns1, ns2); // expected-tns-note 1{{access can happen concurrently}}
@@ -858,13 +861,16 @@ func many_consume_one_require_vararg(a : A) async {
858861
let ns5 = NonSendable();
859862

860863
await a.foo_vararg(ns0, ns3, ns3)
861-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
864+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
865+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
862866
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
863867
await a.foo_vararg(ns4, ns1, ns4)
864-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
868+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
869+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
865870
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
866871
await a.foo_vararg(ns5, ns5, ns2)
867-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
872+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
873+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
868874
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
869875

870876
foo_noniso_vararg(ns0, ns1, ns2); // expected-tns-note 3{{access can happen concurrently}}
@@ -881,13 +887,16 @@ func many_consume_many_require_vararg(a : A) async {
881887
let ns7 = NonSendable();
882888

883889
await a.foo_vararg(ns0, ns3, ns3)
884-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
890+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
891+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
885892
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
886893
await a.foo_vararg(ns4, ns1, ns4)
887-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
894+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
895+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
888896
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
889897
await a.foo_vararg(ns5, ns5, ns2)
890-
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
898+
// expected-tns-warning @-1 {{sending value of non-Sendable type 'Any...' risks causing data races}}
899+
// expected-tns-note @-2 {{sending value of non-Sendable type 'Any...' to actor-isolated instance method 'foo_vararg' risks causing data races between actor-isolated and local nonisolated uses}}
891900
// expected-complete-warning @-2 {{passing argument of non-sendable type 'Any...' into actor-isolated context may introduce data races}}
892901

893902
if bool {
@@ -925,7 +934,8 @@ func enum_test(a : A) async {
925934
case .E2:
926935
switch (e3) {
927936
case let .E3(ns3):
928-
await a.foo(ns3.x); // expected-tns-warning {{sending value of non-Sendable type 'Any' with later accesses from nonisolated context to actor-isolated context risks causing data races}}
937+
await a.foo(ns3.x) // expected-tns-warning {{sending value of non-Sendable type 'Any' risks causing data races}}
938+
// expected-tns-note @-1 {{sending value of non-Sendable type 'Any' to actor-isolated instance method 'foo' risks causing data races between actor-isolated and local nonisolated uses}}
929939
// expected-complete-warning @-1 {{passing argument of non-sendable type 'Any' into actor-isolated context may introduce data races}}
930940
default: ()
931941
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %target-swift-frontend -swift-version 6 -Xllvm -sil-regionbasedisolation-force-use-of-typed-errors -emit-sil -o /dev/null %s -verify
2+
3+
// REQUIRES: concurrency
4+
// REQUIRES: asserts
5+
6+
// READ THIS: This test is only intended to test typed errors that are fallback
7+
// error paths that are only invoked if we can't find a name for the value being
8+
// sent. Since in most cases we are able to infer a name, we do not invoke these
9+
// very often (and in truth in normal compilation we would like to never emit
10+
// them). To make sure that we can at least test them out, this test uses an
11+
// asserts only option that causes us to emit typed errors even when we find a
12+
// name.
13+
14+
////////////////////////
15+
// MARK: Declarations //
16+
////////////////////////
17+
18+
class NonSendableKlass {}
19+
20+
@MainActor func transferToMain<T>(_ t: T) async {}
21+
22+
/////////////////
23+
// MARK: Tests //
24+
/////////////////
25+
26+
func simpleUseAfterFree() async {
27+
let x = NonSendableKlass()
28+
await transferToMain(x) // expected-error {{sending value of non-Sendable type 'NonSendableKlass' risks causing data races}}
29+
// expected-note @-1 {{sending value of non-Sendable type 'NonSendableKlass' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
30+
print(x) // expected-note {{access can happen concurrently}}
31+
}

0 commit comments

Comments
 (0)