Skip to content

Commit 7e992ce

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 e3054a0 commit 7e992ce

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()
@@ -723,7 +723,7 @@ func testGlobalClosureCaptureInOut(_ x: inout SingleElt) {
723723
consumeVal(x) // expected-note {{captured here}}
724724
borrowConsumeVal(x, x) // expected-note {{captured here}}
725725
// expected-note @-1 {{captured here}}
726-
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
726+
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
727727
// expected-note @-3 {{conflicting access is here}}
728728
}
729729
globalClosureCaptureInOut()
@@ -767,7 +767,7 @@ func testLocalLetClosureCaptureInOut(_ x: inout SingleElt) {
767767
consumeVal(x) // expected-note {{captured here}}
768768
borrowConsumeVal(x, x) // expected-note {{captured here}}
769769
// expected-note @-1 {{captured here}}
770-
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
770+
// expected-error @-2 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
771771
// expected-note @-3 {{conflicting access is here}}
772772
}
773773
f()
@@ -817,7 +817,7 @@ func testLocalVarClosureCaptureInOut(_ x: inout SingleElt) {
817817
borrowConsumeVal(x, x)
818818
// expected-note @-1 {{captured here}}
819819
// expected-note @-2 {{captured here}}
820-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
820+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
821821
// expected-note @-4 {{conflicting access is here}}
822822
}
823823
f = {}
@@ -867,7 +867,7 @@ func testInOutVarClosureCaptureInOut(_ f: inout () -> (), _ x: inout SingleElt)
867867
borrowConsumeVal(x, x)
868868
// expected-note @-1 {{captured here}}
869869
// expected-note @-2 {{captured here}}
870-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
870+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
871871
// expected-note @-4 {{conflicting access is here}}
872872
}
873873
f()
@@ -923,7 +923,7 @@ func testConsumingNoEscapeClosureCaptureInOut(_ f: consuming () -> (), _ x: inou
923923
consumeVal(x) // expected-note {{consuming use here}}
924924
// expected-note @-1 {{consuming use here}}
925925
borrowConsumeVal(x, x)
926-
// expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
926+
// expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
927927
// expected-note @-2 {{conflicting access is here}}
928928
// expected-note @-3 {{non-consuming use here}}
929929
// expected-note @-4 {{non-consuming use here}}
@@ -980,7 +980,7 @@ func testConsumingEscapeClosureCaptureInOut(_ f: consuming @escaping () -> (), _
980980
borrowConsumeVal(x, x)
981981
// expected-note @-1 {{captured here}}
982982
// expected-note @-2 {{captured here}}
983-
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
983+
// expected-error @-3 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
984984
// expected-note @-4 {{conflicting access is here}}
985985
}
986986
f()
@@ -1043,7 +1043,7 @@ func testGlobalClosureCaptureConsuming(_ x: consuming SingleElt) {
10431043
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}}
10441044
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}}
10451045
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}}
1046-
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1046+
// expected-error @-1:29 {{overlapping accesses, but deinitialization requires exclusive access}}
10471047
// expected-note @-2:26 {{conflicting access is here}}
10481048
}
10491049
globalClosureCaptureConsuming()
@@ -1104,7 +1104,7 @@ func testLocalLetClosureCaptureConsuming(_ x: consuming SingleElt) {
11041104
consumeVal(x) // expected-note {{consuming use here}}
11051105
consumeVal(x) // expected-note {{consuming use here}}
11061106
borrowConsumeVal(x, x) // expected-note {{consuming use here}}
1107-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1107+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
11081108
// expected-note @-2 {{conflicting access is here}}
11091109
}
11101110
f()
@@ -1158,7 +1158,7 @@ func testLocalVarClosureCaptureConsuming(_ x: consuming SingleElt) {
11581158
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}}
11591159
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}}
11601160
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}}
1161-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1161+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
11621162
// expected-note @-2 {{conflicting access is here}}
11631163
}
11641164
f = {}
@@ -1225,7 +1225,7 @@ func testConsumingNoEscapeClosureCaptureConsuming(_ f: consuming () -> (),
12251225
// expected-note @-1 {{consuming use here}}
12261226
// expected-note @-2 {{consuming use here}}
12271227
// expected-note @-3 {{non-consuming use here}}
1228-
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access; consider copying to a local variable}}
1228+
// expected-error @-4 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
12291229
// expected-note @-5 {{conflicting access is here}}
12301230
}
12311231
f()
@@ -1289,7 +1289,7 @@ func testConsumingEscapeClosureCaptureConsuming(_ f: consuming @escaping () -> (
12891289
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}}
12901290
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}}
12911291
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}}
1292-
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access; consider copying to a local variable}}
1292+
// expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
12931293
// expected-note @-2 {{conflicting access is here}}
12941294
}
12951295
f()

0 commit comments

Comments
 (0)