Skip to content

Commit f1e3c82

Browse files
VisckmartVisckmart
committed
[Parser] Improving diagnostics for access-level modifiers
Improves the diagnostics for situations where multiple access-level modifiers are used on the same declaration. Clarifying the diagnostics' messages Considering setter access control attributes Updating the tests with the new diagnostics' messages
1 parent fad02e3 commit f1e3c82

File tree

4 files changed

+72
-35
lines changed

4 files changed

+72
-35
lines changed

‎include/swift/AST/DiagnosticsParse.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,10 @@ ERROR(duplicate_attribute,none,
14851485
"duplicate %select{attribute|modifier}0", (bool))
14861486
NOTE(previous_attribute,none,
14871487
"%select{attribute|modifier}0 already specified here", (bool))
1488+
ERROR(multiple_access_level_modifiers,none,
1489+
"multiple incompatible access-level modifiers specified", ())
1490+
NOTE(previous_access_level_modifier,none,
1491+
"incompatible modifier specified here", (StringRef))
14881492
ERROR(mutually_exclusive_attrs,none,
14891493
"'%0' contradicts previous %select{attribute|modifier}2 '%1'", (StringRef, StringRef, bool))
14901494

‎lib/Parse/ParseDecl.cpp

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2986,9 +2986,26 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
29862986
if (!Tok.is(tok::l_paren)) {
29872987
// Normal access control attribute.
29882988
AttrRange = Loc;
2989-
DuplicateAttribute = Attributes.getAttribute<AccessControlAttr>();
2990-
if (!DuplicateAttribute)
2989+
2990+
// Try to find a previously parsed access control attribute.
2991+
auto PrevAccessControlAttr = Attributes.getAttribute<AccessControlAttr>();
2992+
2993+
if (!PrevAccessControlAttr) {
29912994
Attributes.add(new (Context) AccessControlAttr(AtLoc, Loc, access));
2995+
} else {
2996+
// Diagnose if there is already an access control attribute on
2997+
// this declaration.
2998+
diagnose(Loc, diag::multiple_access_level_modifiers)
2999+
.highlight(AttrRange);
3000+
diagnose(PrevAccessControlAttr->getLocation(),
3001+
diag::previous_access_level_modifier,
3002+
PrevAccessControlAttr->getAttrName())
3003+
.highlight(PrevAccessControlAttr->getRange());
3004+
3005+
// Remove the reference to the duplicate attribute
3006+
// to avoid the extra diagnostic.
3007+
DuplicateAttribute = nullptr;
3008+
}
29923009
break;
29933010
}
29943011

@@ -3019,10 +3036,26 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
30193036
}
30203037

30213038
DuplicateAttribute = Attributes.getAttribute<SetterAccessAttr>();
3022-
if (!DuplicateAttribute) {
3039+
3040+
// Try to find a previously parsed setter access control attribute.
3041+
auto PrevSetterAccessControlAttr = Attributes.getAttribute<SetterAccessAttr>();
3042+
3043+
if (!PrevSetterAccessControlAttr) {
30233044
Attributes.add(new (Context) SetterAccessAttr(AtLoc, AttrRange, access));
3045+
} else {
3046+
// Diagnose if there is already a setter access control attribute on
3047+
// this declaration.
3048+
diagnose(Loc, diag::multiple_access_level_modifiers)
3049+
.highlight(AttrRange);
3050+
diagnose(PrevSetterAccessControlAttr->getLocation(),
3051+
diag::previous_access_level_modifier,
3052+
PrevSetterAccessControlAttr->getAttrName())
3053+
.highlight(PrevSetterAccessControlAttr->getRange());
3054+
3055+
// Remove the reference to the duplicate attribute
3056+
// to avoid the extra diagnostic.
3057+
DuplicateAttribute = nullptr;
30243058
}
3025-
30263059
break;
30273060
}
30283061

‎test/attr/accessibility.swift

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
11
// RUN: %target-typecheck-verify-swift -package-name myPkg
22

33
// CHECK PARSING
4-
private // expected-note {{modifier already specified here}}
5-
private // expected-error {{duplicate modifier}}
4+
private // expected-note {{incompatible modifier specified here}}
5+
private // expected-error {{multiple incompatible access-level modifiers specified}}
66
func duplicateAttr() {}
77

8-
private // expected-note {{modifier already specified here}}
9-
public // expected-error {{duplicate modifier}}
8+
private // expected-note {{incompatible modifier specified here}}
9+
public // expected-error {{multiple incompatible access-level modifiers specified}}
1010
func duplicateAttrChanged() {}
1111

12-
private // expected-note 2 {{modifier already specified here}}
13-
public // expected-error {{duplicate modifier}}
14-
internal // expected-error {{duplicate modifier}}
12+
private // expected-note 2 {{incompatible modifier specified here}}
13+
public // expected-error {{multiple incompatible access-level modifiers specified}}
14+
internal // expected-error {{multiple incompatible access-level modifiers specified}}
1515
func triplicateAttrChanged() {}
1616

17-
private // expected-note 3 {{modifier already specified here}}
18-
public // expected-error {{duplicate modifier}}
19-
package // expected-error {{duplicate modifier}}
20-
internal // expected-error {{duplicate modifier}}
17+
private // expected-note 3 {{incompatible modifier specified here}}
18+
public // expected-error {{multiple incompatible access-level modifiers specified}}
19+
package // expected-error {{multiple incompatible access-level modifiers specified}}
20+
internal // expected-error {{multiple incompatible access-level modifiers specified}}
2121
func quadruplicateAttrChanged() {}
2222

23-
private // expected-note 4 {{modifier already specified here}}
24-
public // expected-error {{duplicate modifier}}
25-
package // expected-error {{duplicate modifier}}
26-
internal // expected-error {{duplicate modifier}}
27-
fileprivate // expected-error {{duplicate modifier}}
23+
private // expected-note 4 {{incompatible modifier specified here}}
24+
public // expected-error {{multiple incompatible access-level modifiers specified}}
25+
package // expected-error {{multiple incompatible access-level modifiers specified}}
26+
internal // expected-error {{multiple incompatible access-level modifiers specified}}
27+
fileprivate // expected-error {{multiple incompatible access-level modifiers specified}}
2828
func quintuplicateAttrChanged() {}
2929

3030
private(set)
@@ -51,22 +51,22 @@ internal(set)
5151
package
5252
var customSetter6 = 0
5353

54-
private(set) // expected-note {{modifier already specified here}}
55-
public(set) // expected-error {{duplicate modifier}}
54+
private(set) // expected-note {{incompatible modifier specified here}}
55+
public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
5656
var customSetterDuplicateAttr = 0
5757

58-
private(set) // expected-note {{modifier already specified here}}
59-
public // expected-note {{modifier already specified here}}
60-
public(set) // expected-error {{duplicate modifier}}
61-
private // expected-error {{duplicate modifier}}
58+
private(set) // expected-note {{incompatible modifier specified here}}
59+
public // expected-note {{incompatible modifier specified here}}
60+
public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
61+
private // expected-error {{multiple incompatible access-level modifiers specified}}
6262
var customSetterDuplicateAttrsAllAround = 0
6363

64-
private(set) // expected-note {{modifier already specified here}}
65-
package(set) // expected-error {{duplicate modifier}}
64+
private(set) // expected-note {{incompatible modifier specified here}}
65+
package(set) // expected-error {{multiple incompatible access-level modifiers specified}}
6666
var customSetterDuplicateAttr2 = 0
6767

68-
package(set) // expected-note {{modifier already specified here}}
69-
public(set) // expected-error {{duplicate modifier}}
68+
package(set) // expected-note {{incompatible modifier specified here}}
69+
public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
7070
public var customSetterDuplicateAttr3 = 0
7171

7272
private(get) // expected-error{{expected 'set' as subject of 'private' modifier}}

‎test/attr/open.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import OpenHelpers
66

77
/**** General structural limitations on open. ****/
88

9-
open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
10-
open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
11-
open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
12-
open public class OpenIsNotCompatibleWithPublic {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
13-
open open class OpenIsNotCompatibleWithOpen {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
9+
open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
10+
open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
11+
open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
12+
open public class OpenIsNotCompatibleWithPublic {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
13+
open open class OpenIsNotCompatibleWithOpen {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
1414

1515
open typealias OpenIsNotAllowedOnTypeAliases = Int // expected-error {{only classes and overridable class members can be declared 'open'; use 'public'}}
1616
open struct OpenIsNotAllowedOnStructs {} // expected-error {{only classes and overridable class members can be declared 'open'; use 'public'}}

0 commit comments

Comments
 (0)