Skip to content

[Parser] Improving diagnostics for access-level modifiers #68894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 27, 2023
Merged

[Parser] Improving diagnostics for access-level modifiers #68894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,10 @@ ERROR(duplicate_attribute,none,
"duplicate %select{attribute|modifier}0", (bool))
NOTE(previous_attribute,none,
"%select{attribute|modifier}0 already specified here", (bool))
ERROR(multiple_access_level_modifiers,none,
"multiple incompatible access-level modifiers specified", ())
NOTE(previous_access_level_modifier,none,
"previous modifier specified here", ())
ERROR(mutually_exclusive_attrs,none,
"'%0' contradicts previous %select{attribute|modifier}2 '%1'", (StringRef, StringRef, bool))

Expand Down
39 changes: 36 additions & 3 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3093,9 +3093,25 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
if (!Tok.is(tok::l_paren)) {
// Normal access control attribute.
AttrRange = Loc;
DuplicateAttribute = Attributes.getAttribute<AccessControlAttr>();
if (!DuplicateAttribute)

if (!DiscardAttribute) {
Attributes.add(new (Context) AccessControlAttr(AtLoc, Loc, access));
break;
}

// Diagnose if there's already an access control attribute on
// this declaration with a different access level.
if (access != cast<AccessControlAttr>(DuplicateAttribute)->getAccess()) {
diagnose(Loc, diag::multiple_access_level_modifiers)
.highlight(AttrRange);
diagnose(DuplicateAttribute->getLocation(),
diag::previous_access_level_modifier)
.highlight(DuplicateAttribute->getRange());

// Remove the reference to the duplicate attribute
// to avoid the extra diagnostic.
DuplicateAttribute = nullptr;
}
break;
}

Expand Down Expand Up @@ -3125,11 +3141,28 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
return makeParserSuccess();
}

// Check for duplicate setter access control attributes.
DuplicateAttribute = Attributes.getAttribute<SetterAccessAttr>();
if (!DuplicateAttribute) {
DiscardAttribute = DuplicateAttribute != nullptr;

if (!DiscardAttribute) {
Attributes.add(new (Context) SetterAccessAttr(AtLoc, AttrRange, access));
break;
}

// Diagnose if there's already a setter access control attribute on
// this declaration with a different access level.
if (access != cast<SetterAccessAttr>(DuplicateAttribute)->getAccess()) {
diagnose(Loc, diag::multiple_access_level_modifiers)
.highlight(AttrRange);
diagnose(DuplicateAttribute->getLocation(),
diag::previous_access_level_modifier)
.highlight(DuplicateAttribute->getRange());

// Remove the reference to the duplicate attribute
// to avoid the extra diagnostic.
DuplicateAttribute = nullptr;
}
break;
}

Expand Down
48 changes: 24 additions & 24 deletions test/attr/accessibility.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,26 @@ private // expected-note {{modifier already specified here}}
private // expected-error {{duplicate modifier}}
func duplicateAttr() {}

private // expected-note {{modifier already specified here}}
public // expected-error {{duplicate modifier}}
private // expected-note {{previous modifier specified here}}
public // expected-error {{multiple incompatible access-level modifiers specified}}
func duplicateAttrChanged() {}

private // expected-note 2 {{modifier already specified here}}
public // expected-error {{duplicate modifier}}
internal // expected-error {{duplicate modifier}}
private // expected-note 2 {{previous modifier specified here}}
public // expected-error {{multiple incompatible access-level modifiers specified}}
internal // expected-error {{multiple incompatible access-level modifiers specified}}
func triplicateAttrChanged() {}

private // expected-note 3 {{modifier already specified here}}
public // expected-error {{duplicate modifier}}
package // expected-error {{duplicate modifier}}
internal // expected-error {{duplicate modifier}}
private // expected-note 3 {{previous modifier specified here}}
public // expected-error {{multiple incompatible access-level modifiers specified}}
package // expected-error {{multiple incompatible access-level modifiers specified}}
internal // expected-error {{multiple incompatible access-level modifiers specified}}
func quadruplicateAttrChanged() {}

private // expected-note 4 {{modifier already specified here}}
public // expected-error {{duplicate modifier}}
package // expected-error {{duplicate modifier}}
internal // expected-error {{duplicate modifier}}
fileprivate // expected-error {{duplicate modifier}}
private // expected-note 4 {{previous modifier specified here}}
public // expected-error {{multiple incompatible access-level modifiers specified}}
package // expected-error {{multiple incompatible access-level modifiers specified}}
internal // expected-error {{multiple incompatible access-level modifiers specified}}
fileprivate // expected-error {{multiple incompatible access-level modifiers specified}}
func quintuplicateAttrChanged() {}

private(set)
Expand All @@ -51,22 +51,22 @@ internal(set)
package
var customSetter6 = 0

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

private(set) // expected-note {{modifier already specified here}}
public // expected-note {{modifier already specified here}}
public(set) // expected-error {{duplicate modifier}}
private // expected-error {{duplicate modifier}}
private(set) // expected-note {{previous modifier specified here}}
public // expected-note {{previous modifier specified here}}
public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
private // expected-error {{multiple incompatible access-level modifiers specified}}
var customSetterDuplicateAttrsAllAround = 0

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

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

private(get) // expected-error{{expected 'set' as subject of 'private' modifier}}
Expand Down
8 changes: 4 additions & 4 deletions test/attr/open.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import OpenHelpers

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

open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
open public class OpenIsNotCompatibleWithPublic {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
open public class OpenIsNotCompatibleWithPublic {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
open open class OpenIsNotCompatibleWithOpen {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}

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