Skip to content

Commit 0089d4d

Browse files
nate-chandlernate-chandler
committed
[CoroutineAccessors] Ban _read+read.
1 parent de86cc0 commit 0089d4d

File tree

4 files changed

+58
-29
lines changed

4 files changed

+58
-29
lines changed

‎lib/AST/ASTPrinter.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2391,12 +2391,6 @@ void PrintAST::printSelfAccessKindModifiersIfNeeded(const FuncDecl *FD) {
23912391
}
23922392
}
23932393

2394-
static bool
2395-
shouldPrintUnderscoredCoroutineAccessors(const AbstractStorageDecl *ASD) {
2396-
// TODO: CoroutineAccessors: Print only when necessary.
2397-
return true;
2398-
}
2399-
24002394
void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
24012395
if (isa<VarDecl>(ASD) && !Options.PrintPropertyAccessors)
24022396
return;
@@ -2573,7 +2567,7 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
25732567
break;
25742568
case ReadImplKind::Read2:
25752569
if (ASD->getAccessor(AccessorKind::Read) &&
2576-
shouldPrintUnderscoredCoroutineAccessors(ASD)) {
2570+
Options.SuppressCoroutineAccessors) {
25772571
AddAccessorToPrint(AccessorKind::Read);
25782572
}
25792573
AddAccessorToPrint(AccessorKind::Read2);
@@ -2607,7 +2601,7 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
26072601
break;
26082602
case WriteImplKind::Modify2:
26092603
if (ASD->getAccessor(AccessorKind::Modify) &&
2610-
shouldPrintUnderscoredCoroutineAccessors(ASD)) {
2604+
Options.SuppressCoroutineAccessors) {
26112605
AddAccessorToPrint(AccessorKind::Modify);
26122606
}
26132607
AddAccessorToPrint(AccessorKind::Modify2);

‎lib/Parse/ParseDecl.cpp

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8336,16 +8336,45 @@ void Parser::ParsedAccessors::record(Parser &P, AbstractStorageDecl *storage,
83368336
storage->setAccessors(LBLoc, Accessors, RBLoc);
83378337
}
83388338

8339+
static std::optional<AccessorKind>
8340+
getCorrespondingUnderscoredAccessorKind(AccessorKind kind) {
8341+
switch (kind) {
8342+
case AccessorKind::Read2:
8343+
return {AccessorKind::Read};
8344+
case AccessorKind::Modify2:
8345+
return {AccessorKind::Modify};
8346+
case AccessorKind::Get:
8347+
case AccessorKind::DistributedGet:
8348+
case AccessorKind::Set:
8349+
case AccessorKind::Read:
8350+
case AccessorKind::Modify:
8351+
case AccessorKind::WillSet:
8352+
case AccessorKind::DidSet:
8353+
case AccessorKind::Address:
8354+
case AccessorKind::MutableAddress:
8355+
case AccessorKind::Init:
8356+
return std::nullopt;
8357+
}
8358+
}
8359+
83398360
static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
83408361
AccessorDecl *&second) {
83418362
if (!second) return;
8342-
P.diagnose(second->getLoc(), diag::conflicting_accessor,
8343-
isa<SubscriptDecl>(first->getStorage()),
8344-
getAccessorNameForDiagnostic(second, /*article*/ true),
8345-
getAccessorNameForDiagnostic(first, /*article*/ true));
8346-
P.diagnose(first->getLoc(), diag::previous_accessor,
8347-
getAccessorNameForDiagnostic(first, /*article*/ false),
8348-
/*already*/ false);
8363+
bool underscored =
8364+
(getCorrespondingUnderscoredAccessorKind(first->getAccessorKind()) ==
8365+
second->getAccessorKind()) ||
8366+
(getCorrespondingUnderscoredAccessorKind(second->getAccessorKind()) ==
8367+
first->getAccessorKind()) ||
8368+
first->getASTContext().LangOpts.hasFeature(Feature::CoroutineAccessors);
8369+
P.diagnose(
8370+
second->getLoc(), diag::conflicting_accessor,
8371+
isa<SubscriptDecl>(first->getStorage()),
8372+
getAccessorNameForDiagnostic(second, /*article*/ true, underscored),
8373+
getAccessorNameForDiagnostic(first, /*article*/ true, underscored));
8374+
P.diagnose(
8375+
first->getLoc(), diag::previous_accessor,
8376+
getAccessorNameForDiagnostic(first, /*article*/ false, underscored),
8377+
/*already*/ false);
83498378
second->setInvalid();
83508379
}
83518380

@@ -8388,12 +8417,13 @@ void Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
83888417

83898418
// Okay, observers are out of the way.
83908419

8391-
// 'get', 'read', and a non-mutable addressor are all exclusive.
8420+
// 'get', '_read', 'read' and a non-mutable addressor are all exclusive.
83928421
if (Get) {
83938422
diagnoseConflictingAccessors(P, Get, Read);
83948423
diagnoseConflictingAccessors(P, Get, Read2);
83958424
diagnoseConflictingAccessors(P, Get, Address);
83968425
} else if (Read) {
8426+
diagnoseConflictingAccessors(P, Read, Read2);
83978427
diagnoseConflictingAccessors(P, Read, Address);
83988428
} else if (Read2) {
83998429
diagnoseConflictingAccessors(P, Read2, Address);
@@ -8422,11 +8452,13 @@ void Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
84228452
}
84238453
}
84248454

8425-
// A mutable addressor is exclusive with 'set' and 'modify', but
8426-
// 'set' and 'modify' can appear together.
8455+
// '_modify', 'modify' and 'unsafeMutableAddress' are all mutually exclusive.
8456+
// 'unsafeMutableAddress' and 'set' are mutually exclusive, but 'set' and
8457+
// 'modify' can appear together.
84278458
if (Set) {
84288459
diagnoseConflictingAccessors(P, Set, MutableAddress);
84298460
} else if (Modify) {
8461+
diagnoseConflictingAccessors(P, Modify, Modify2);
84308462
diagnoseConflictingAccessors(P, Modify, MutableAddress);
84318463
}
84328464

‎test/ModuleInterface/coroutine_accessors.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ var _i: Int = 0
1212

1313
// CHECK: #if compiler(>=5.3) && $CoroutineAccessors
1414
// CHECK-NEXT: public var i: Swift.Int {
15-
// CHECK-NEXT: _read
1615
// CHECK-NEXT: read
17-
// CHECK-NEXT: _modify
1816
// CHECK-NEXT: modify
1917
// CHECK-NEXT: }
2018
// CHECK-NEXT: #else
@@ -24,15 +22,9 @@ var _i: Int = 0
2422
// CHECK-NEXT: }
2523
// CHECK-NEXT: #endif
2624
public var i: Int {
27-
_read {
28-
yield _i
29-
}
3025
read {
3126
yield _i
3227
}
33-
_modify {
34-
yield &_i
35-
}
3628
modify {
3729
yield &_i
3830
}

‎test/Parse/coroutine_accessors.swift

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,16 @@ var _i: Int = 0
3030
// disabled: implicit getter.
3131
var ir_r: Int {
3232
read { // expected-disabled-error{{cannot_find_in_scope}}
33+
// expected-enabled-error@-1{{variable cannot provide both a 'read' accessor and a '_read' accessor}}
3334
fatalError()
3435
}
3536
_read { // expected-disabled-error{{cannot_find_in_scope}}
37+
// expected-enabled-note@-1{{previous_accessor}}
3638
fatalError()
3739
}
3840
}
3941

40-
// enabled: ok
42+
// enabled: conflicting accessors
4143
var igr: Int {
4244
get {
4345
1
@@ -255,14 +257,16 @@ var iumam: Int {
255257
}
256258
}
257259

258-
// enabled: need a reader.
260+
// enabled: conflicting accessors. need a reader.
259261
// disabled: implicit getter.
260262
var im_m: Int {
261263
modify { // expected-disabled-error{{cannot_find_in_scope}}
264+
// expected-enabled-error@-1{{variable cannot provide both a 'modify' accessor and a '_modify' accessor}}
262265
fatalError()
263266
}
264267
_modify { // expected-enabled-error{{variable with a '_modify' accessor must also have a getter, addressor, or 'read' accessor}}
265268
// expected-disabled-error@-1{{cannot_find_in_scope}}
269+
// expected-enabled-note@-2{{previous_accessor}}
266270
fatalError()
267271
}
268272
}
@@ -272,9 +276,11 @@ var im_m: Int {
272276
var i_mm: Int {
273277
_modify { // expected-enabled-error{{variable with a '_modify' accessor must also have a getter, addressor, or 'read' accessor}}
274278
// expected-disabled-error@-1{{variable with a 'modify' accessor must also have a getter, addressor, or 'read' accessor}}
279+
// expected-note@-2{{previous_accessor}}
275280
fatalError()
276281
}
277282
modify { // expected-disabled-error{{'modify' accessor is only valid when experimental feature coroutine accessors is enabled}}
283+
// expected-error@-1{{variable cannot provide both a 'modify' accessor and a '_modify' accessor}}
278284
fatalError()
279285
}
280286
}
@@ -304,9 +310,10 @@ var i_rm_m: Int {
304310
yield _i
305311
}
306312
modify { // expected-disabled-error{{'modify' accessor is only valid when experimental feature coroutine accessors is enabled}}
313+
// expected-error@-1{{variable cannot provide both a 'modify' accessor and a '_modify' accessor}}
307314
yield &_i
308315
}
309-
_modify {
316+
_modify { // expected-note{{previous_accessor}}
310317
yield &_i
311318
}
312319
}
@@ -315,15 +322,19 @@ var i_rm_m: Int {
315322
// disabled: implicit getter
316323
var ir_rm_m: Int {
317324
read { // expected-disabled-error{{cannot_find_in_scope}}
325+
// expected-enabled-error@-1{{variable cannot provide both a 'read' accessor and a '_read' accessor}}
318326
fatalError()
319327
}
320328
_read { // expected-disabled-error{{cannot_find_in_scope}}
329+
// expected-enabled-note@-1{{previous_accessor}}
321330
fatalError()
322331
}
323332
modify { // expected-disabled-error{{cannot_find_in_scope}}
333+
// expected-enabled-error@-1{{variable cannot provide both a 'modify' accessor and a '_modify' accessor}}
324334
fatalError()
325335
}
326336
_modify { // expected-disabled-error{{cannot_find_in_scope}}
337+
// expected-enabled-note@-1{{previous_accessor}}
327338
fatalError()
328339
}
329340
}

0 commit comments

Comments
 (0)