Skip to content

Commit 6d9fef2

Browse files
gottesmmgottesmm
committed
[move-only] When emitting exclusivity diagnostics for move only types, do not suggest to the user to make a local copy.
It doesn't make sense to give this note since one can't make a copy of a noncopyable type. rdar://108511627
1 parent f8d0c69 commit 6d9fef2

File tree

6 files changed

+108
-37
lines changed

6 files changed

+108
-37
lines changed

‎include/swift/AST/DiagnosticsSIL.def

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,17 @@ ERROR(exclusivity_access_required_unknown_decl,none,
8888
"%select{initialization|read|modification|deinitialization}0 requires "
8989
"exclusive access; consider copying to a local variable", (unsigned))
9090

91+
ERROR(exclusivity_access_required_moveonly,none,
92+
"overlapping accesses to %0, but "
93+
"%select{initialization|read|modification|deinitialization}1 requires "
94+
"exclusive access",
95+
(StringRef, unsigned))
96+
97+
ERROR(exclusivity_access_required_unknown_decl_moveonly,none,
98+
"overlapping accesses, but "
99+
"%select{initialization|read|modification|deinitialization}0 requires "
100+
"exclusive access", (unsigned))
101+
91102
NOTE(exclusivity_conflicting_access,none,
92103
"conflicting access is here", ())
93104

‎include/swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ class AccessSummaryAnalysis : public BottomUpIPAnalysis {
205205
SILModule &M,
206206
TypeExpansionContext context);
207207

208+
/// Returns the type associated with the subpath \p SubPath.
209+
///
210+
/// \p BaseType must be the type of the root of the path.
211+
static SILType getSubPathType(SILType BaseType, const IndexTrieNode *SubPath,
212+
SILModule &M, TypeExpansionContext context);
213+
208214
/// Performs a lexicographic comparison of two subpaths, first by path length
209215
/// and then by index of the last path component. Returns true when lhs
210216
/// is less than rhs.

‎lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,37 @@ AccessSummaryAnalysis::findSubPathAccessed(BeginAccessInst *BAI) {
551551
return SubPath;
552552
}
553553

554+
SILType AccessSummaryAnalysis::getSubPathType(SILType baseType,
555+
const IndexTrieNode *subPath,
556+
SILModule &mod,
557+
TypeExpansionContext context) {
558+
// Walk the trie to the root to collect the sequence (in reverse order).
559+
llvm::SmallVector<unsigned, 4> reversedIndices;
560+
const IndexTrieNode *indexTrieNode = subPath;
561+
while (!indexTrieNode->isRoot()) {
562+
reversedIndices.push_back(indexTrieNode->getIndex());
563+
indexTrieNode = indexTrieNode->getParent();
564+
}
565+
566+
SILType iterType = baseType;
567+
for (unsigned index : llvm::reverse(reversedIndices)) {
568+
if (StructDecl *decl = iterType.getStructOrBoundGenericStruct()) {
569+
VarDecl *var = decl->getStoredProperties()[index];
570+
iterType = iterType.getFieldType(var, mod, context);
571+
continue;
572+
}
573+
574+
if (auto tupleTy = iterType.getAs<TupleType>()) {
575+
iterType = iterType.getTupleElementType(index);
576+
continue;
577+
}
578+
579+
llvm_unreachable("unexpected type in projection subpath!");
580+
}
581+
582+
return iterType;
583+
}
584+
554585
/// Returns a string representation of the SubPath
555586
/// suitable for use in diagnostic text. Only supports the Projections
556587
/// that stored-property relaxation supports: struct stored properties

‎lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -541,13 +541,19 @@ static void diagnoseExclusivityViolation(const ConflictingAccess &Violation,
541541
unsigned AccessKindForMain =
542542
static_cast<unsigned>(MainAccess.getAccessKind());
543543

544+
SILType BaseType = FirstAccess.getInstruction()->getType().getAddressType();
545+
SILModule &M = FirstAccess.getInstruction()->getModule();
546+
TypeExpansionContext TypeExpansionCtx(
547+
*FirstAccess.getInstruction()->getFunction());
548+
SILType firstAccessType = AccessSummaryAnalysis::getSubPathType(
549+
BaseType, MainAccess.getSubPath(), M, TypeExpansionCtx);
550+
bool isMoveOnly = firstAccessType.isMoveOnly();
551+
544552
if (const ValueDecl *VD = Storage.getDecl()) {
545553
// We have a declaration, so mention the identifier in the diagnostic.
546-
SILType BaseType = FirstAccess.getInstruction()->getType().getAddressType();
547-
SILModule &M = FirstAccess.getInstruction()->getModule();
548-
std::string PathDescription = getPathDescription(
549-
VD->getBaseName(), BaseType, MainAccess.getSubPath(), M,
550-
TypeExpansionContext(*FirstAccess.getInstruction()->getFunction()));
554+
std::string PathDescription =
555+
getPathDescription(VD->getBaseName(), BaseType, MainAccess.getSubPath(),
556+
M, TypeExpansionCtx);
551557

552558
// Determine whether we can safely suggest replacing the violation with
553559
// a call to MutableCollection.swapAt().
@@ -562,18 +568,35 @@ static void diagnoseExclusivityViolation(const ConflictingAccess &Violation,
562568
CallsToSwap, Ctx, CallToReplace, Base, SwapIndex1, SwapIndex2);
563569
}
564570

565-
auto D =
566-
diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
567-
diag::exclusivity_access_required,
568-
PathDescription, AccessKindForMain, SuggestSwapAt);
569-
D.highlight(RangeForMain);
570-
if (SuggestSwapAt)
571-
addSwapAtFixit(D, CallToReplace, Base, SwapIndex1, SwapIndex2,
572-
Ctx.SourceMgr);
571+
if (isMoveOnly) {
572+
auto D = diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
573+
diag::exclusivity_access_required_moveonly,
574+
PathDescription, AccessKindForMain);
575+
D.highlight(RangeForMain);
576+
if (SuggestSwapAt)
577+
addSwapAtFixit(D, CallToReplace, Base, SwapIndex1, SwapIndex2,
578+
Ctx.SourceMgr);
579+
} else {
580+
auto D = diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
581+
diag::exclusivity_access_required, PathDescription,
582+
AccessKindForMain, SuggestSwapAt);
583+
D.highlight(RangeForMain);
584+
if (SuggestSwapAt)
585+
addSwapAtFixit(D, CallToReplace, Base, SwapIndex1, SwapIndex2,
586+
Ctx.SourceMgr);
587+
}
573588
} else {
574-
diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
575-
diag::exclusivity_access_required_unknown_decl, AccessKindForMain)
576-
.highlight(RangeForMain);
589+
if (isMoveOnly) {
590+
diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
591+
diag::exclusivity_access_required_unknown_decl_moveonly,
592+
AccessKindForMain)
593+
.highlight(RangeForMain);
594+
} else {
595+
diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
596+
diag::exclusivity_access_required_unknown_decl,
597+
AccessKindForMain)
598+
.highlight(RangeForMain);
599+
}
577600
}
578601
diagnose(Ctx, NoteAccess.getAccessLoc().getSourceLoc(),
579602
diag::exclusivity_conflicting_access)

‎test/SILGen/moveonly_escaping_closure.swift

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func testGlobalClosureCaptureVar() {
7272
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}}
7373
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}}
7474
borrowConsumeVal(x, 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}}
75-
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
75+
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access}}
7676
// expected-note @-2:26 {{conflicting access is here}}
7777
}
7878
globalClosureCaptureVar()
@@ -134,7 +134,7 @@ func testLocalLetClosureCaptureVar() {
134134
consumeVal(x) // expected-note {{consuming use here}}
135135
consumeVal(x) // expected-note {{consuming use here}}
136136
borrowConsumeVal(x, x) // expected-note {{consuming use here}}
137-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
137+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
138138
// expected-note @-2 {{conflicting access is here}}
139139
}
140140
f()
@@ -190,7 +190,7 @@ func testLocalVarClosureCaptureVar() {
190190
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}}
191191
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}}
192192
borrowConsumeVal(x, 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}}
193-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
193+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
194194
// expected-note @-2 {{conflicting access is here}}
195195
}
196196
f = {}
@@ -251,7 +251,7 @@ func testInOutVarClosureCaptureVar(_ f: inout () -> ()) {
251251
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}}
252252
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}}
253253
borrowConsumeVal(x, 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}}
254-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
254+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
255255
// expected-note @-2 {{conflicting access is here}}
256256
}
257257
f()
@@ -318,7 +318,7 @@ func testConsumingNoEscapeClosureCaptureVar(_ f: consuming () -> ()) {
318318
// expected-note @-1 {{consuming use here}}
319319
// expected-note @-2 {{consuming use here}}
320320
// expected-note @-3 {{non-consuming use here}}
321-
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
321+
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
322322
// expected-note @-5 {{conflicting access is here}}
323323
}
324324
f()
@@ -384,7 +384,7 @@ func testConsumingEscapeClosureCaptureVar(_ f: consuming @escaping () -> ()) {
384384
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}}
385385
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}}
386386
borrowConsumeVal(x, 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}}
387-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
387+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
388388
// expected-note @-2 {{conflicting access is here}}
389389
}
390390
f()
@@ -734,7 +734,7 @@ func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
734734
consumeVal(x) // expected-note {{captured here}}
735735
borrowConsumeVal(x, x) // expected-note {{captured here}}
736736
// expected-note @-1 {{captured here}}
737-
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
737+
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
738738
// expected-note @-3 {{conflicting access is here}}
739739
}
740740
globalClosureCaptureInOut()
@@ -778,7 +778,7 @@ func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
778778
consumeVal(x) // expected-note {{captured here}}
779779
borrowConsumeVal(x, x) // expected-note {{captured here}}
780780
// expected-note @-1 {{captured here}}
781-
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
781+
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
782782
// expected-note @-3 {{conflicting access is here}}
783783
}
784784
f()
@@ -828,7 +828,7 @@ func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
828828
borrowConsumeVal(x, x)
829829
// expected-note @-1 {{captured here}}
830830
// expected-note @-2 {{captured here}}
831-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
831+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
832832
// expected-note @-4 {{conflicting access is here}}
833833
}
834834
f = {}
@@ -878,7 +878,7 @@ func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt)
878878
borrowConsumeVal(x, x)
879879
// expected-note @-1 {{captured here}}
880880
// expected-note @-2 {{captured here}}
881-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
881+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
882882
// expected-note @-4 {{conflicting access is here}}
883883
}
884884
f()
@@ -934,7 +934,7 @@ func testConsumingNoEscapeClosureCaptureInOut(_ f: consuming () -> (), _ x: inou
934934
consumeVal(x) // expected-note {{consuming use here}}
935935
// expected-note @-1 {{consuming use here}}
936936
borrowConsumeVal(x, x)
937-
// expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
937+
// expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
938938
// expected-note @-2 {{conflicting access is here}}
939939
// expected-note @-3 {{non-consuming use here}}
940940
// expected-note @-4 {{non-consuming use here}}
@@ -991,7 +991,7 @@ func testConsumingEscapeClosureCaptureInOut(_ f: consuming @escaping () -> (), _
991991
borrowConsumeVal(x, x)
992992
// expected-note @-1 {{captured here}}
993993
// expected-note @-2 {{captured here}}
994-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
994+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
995995
// expected-note @-4 {{conflicting access is here}}
996996
}
997997
f()
@@ -1054,7 +1054,7 @@ func testGlobalClosureCaptureConsuming(_ x: consuming SingleElt) {
10541054
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}}
10551055
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}}
10561056
borrowConsumeVal(x, 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}}
1057-
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1057+
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access}}
10581058
// expected-note @-2:26 {{conflicting access is here}}
10591059
}
10601060
globalClosureCaptureConsuming()
@@ -1115,7 +1115,7 @@ func testLocalLetClosureCaptureConsuming(_ x: consuming SingleElt) {
11151115
consumeVal(x) // expected-note {{consuming use here}}
11161116
consumeVal(x) // expected-note {{consuming use here}}
11171117
borrowConsumeVal(x, x) // expected-note {{consuming use here}}
1118-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1118+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
11191119
// expected-note @-2 {{conflicting access is here}}
11201120
}
11211121
f()
@@ -1169,7 +1169,7 @@ func testLocalVarClosureCaptureConsuming(_ x: consuming SingleElt) {
11691169
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}}
11701170
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}}
11711171
borrowConsumeVal(x, 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}}
1172-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1172+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
11731173
// expected-note @-2 {{conflicting access is here}}
11741174
}
11751175
f = {}
@@ -1236,7 +1236,7 @@ func testConsumingNoEscapeClosureCaptureConsuming(_ f: consuming () -> (),
12361236
// expected-note @-1 {{consuming use here}}
12371237
// expected-note @-2 {{consuming use here}}
12381238
// expected-note @-3 {{non-consuming use here}}
1239-
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
1239+
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
12401240
// expected-note @-5 {{conflicting access is here}}
12411241
}
12421242
f()
@@ -1300,7 +1300,7 @@ func testConsumingEscapeClosureCaptureConsuming(_ f: consuming @escaping () -> (
13001300
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}}
13011301
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}}
13021302
borrowConsumeVal(x, 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}}
1303-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1303+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
13041304
// expected-note @-2 {{conflicting access is here}}
13051305
}
13061306
f()

0 commit comments

Comments
 (0)