Skip to content

[Macros] Disallow self and Self as macro names #64609

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
Mar 27, 2023
Merged

[Macros] Disallow self and Self as macro names #64609

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
5 changes: 5 additions & 0 deletions include/swift/Parse/Parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1716,6 +1716,11 @@ class Parser {

/// If passed, compound names with empty argument lists are allowed.
AllowZeroArgCompoundNames = AllowCompoundNames | 1 << 5,

/// If passed, \c self and \c Self are allowed as basenames. In a lot of
/// cases this doesn't actually make sense but we need to accept them for
/// backwards compatibility.
AllowLowercaseAndUppercaseSelf = 1 << 6,
};
using DeclNameOptions = OptionSet<DeclNameFlag>;

Expand Down
27 changes: 16 additions & 11 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,8 @@ bool Parser::parseSpecializeAttributeArguments(
loc, diag::attr_specialize_expected_function,
DeclNameFlag::AllowZeroArgCompoundNames |
DeclNameFlag::AllowKeywordsUsingSpecialNames |
DeclNameFlag::AllowOperators);
DeclNameFlag::AllowOperators |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
}
}
if (ParamLabel == "spiModule") {
Expand Down Expand Up @@ -1137,10 +1138,11 @@ Parser::parseImplementsAttribute(SourceLoc AtLoc, SourceLoc Loc) {
}

if (!Status.isErrorOrHasCompletion()) {
MemberName = parseDeclNameRef(MemberNameLoc,
diag::attr_implements_expected_member_name,
MemberName = parseDeclNameRef(
MemberNameLoc, diag::attr_implements_expected_member_name,
DeclNameFlag::AllowZeroArgCompoundNames |
DeclNameFlag::AllowOperators);
DeclNameFlag::AllowOperators |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
if (!MemberName) {
Status.setIsParseError();
}
Expand Down Expand Up @@ -1505,7 +1507,8 @@ static bool parseQualifiedDeclName(Parser &P, Diag<> nameParseError,
original.Loc, nameParseError,
Parser::DeclNameFlag::AllowZeroArgCompoundNames |
Parser::DeclNameFlag::AllowKeywordsUsingSpecialNames |
Parser::DeclNameFlag::AllowOperators);
Parser::DeclNameFlag::AllowOperators |
Parser::DeclNameFlag::AllowLowercaseAndUppercaseSelf);
// The base type is optional, but the final unqualified declaration name is
// not. If name could not be parsed, return true for error.
if (!original.Name)
Expand Down Expand Up @@ -3207,11 +3210,12 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
consumeToken(tok::colon);
{
DeclNameLoc loc;
replacedFunction = parseDeclNameRef(loc,
diag::attr_dynamic_replacement_expected_function,
replacedFunction = parseDeclNameRef(
loc, diag::attr_dynamic_replacement_expected_function,
DeclNameFlag::AllowZeroArgCompoundNames |
DeclNameFlag::AllowKeywordsUsingSpecialNames |
DeclNameFlag::AllowOperators);
DeclNameFlag::AllowKeywordsUsingSpecialNames |
DeclNameFlag::AllowOperators |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
}
}

Expand Down Expand Up @@ -3920,8 +3924,9 @@ bool Parser::parseConventionAttributeInternal(
}

DeclNameLoc unusedLoc;
convention.WitnessMethodProtocol = parseDeclNameRef(unusedLoc,
diag::convention_attribute_witness_method_expected_protocol, {});
convention.WitnessMethodProtocol = parseDeclNameRef(
unusedLoc, diag::convention_attribute_witness_method_expected_protocol,
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
}

// Parse the ')'. We can't use parseMatchingToken if we're in
Expand Down
29 changes: 20 additions & 9 deletions lib/Parse/ParseExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,8 @@ ParserResult<Expr> Parser::parseExprKeyPathObjC() {
// Parse the sequence of unqualified-names.
ParserStatus status;
SourceLoc LastDotLoc;
DeclNameOptions flags = DeclNameFlag::AllowCompoundNames;
DeclNameOptions flags = DeclNameFlag::AllowCompoundNames |
DeclNameFlag::AllowLowercaseAndUppercaseSelf;
while (true) {
// Handle code completion.
if (Tok.is(tok::code_complete))
Expand Down Expand Up @@ -1272,8 +1273,10 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
Diag<> D = isa<SuperRefExpr>(Result.get())
? diag::expected_identifier_after_super_dot_expr
: diag::expected_member_name;
auto Name = parseDeclNameRef(NameLoc, D,
DeclNameFlag::AllowKeywords | DeclNameFlag::AllowCompoundNames);
auto Name = parseDeclNameRef(
NameLoc, D,
DeclNameFlag::AllowKeywords | DeclNameFlag::AllowCompoundNames |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
if (!Name) {
SourceRange ErrorRange = Result.get()->getSourceRange();
ErrorRange.widen(TokLoc);
Expand Down Expand Up @@ -1756,7 +1759,9 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
}

Name = parseDeclNameRef(NameLoc, diag::expected_identifier_after_dot_expr,
DeclNameFlag::AllowKeywords | DeclNameFlag::AllowCompoundNames);
DeclNameFlag::AllowKeywords |
DeclNameFlag::AllowCompoundNames |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
if (!Name)
return makeParserErrorResult(new (Context) ErrorExpr(DotLoc));

Expand Down Expand Up @@ -2205,7 +2210,9 @@ DeclNameRef Parser::parseDeclNameRef(DeclNameLoc &loc,
// Consume the base name.
DeclBaseName baseName;
SourceLoc baseNameLoc;
if (Tok.isAny(tok::identifier, tok::kw_Self, tok::kw_self)) {
if (Tok.is(tok::identifier) ||
(flags.contains(DeclNameFlag::AllowLowercaseAndUppercaseSelf) &&
Tok.isAny(tok::kw_Self, tok::kw_self))) {
Identifier baseNameId;
baseNameLoc = consumeIdentifier(baseNameId, /*diagnoseDollarPrefix=*/false);
baseName = baseNameId;
Expand Down Expand Up @@ -2266,8 +2273,10 @@ ParserResult<Expr> Parser::parseExprIdentifier() {

// Parse the unqualified-decl-name.
DeclNameLoc loc;
DeclNameRef name = parseDeclNameRef(loc, diag::expected_expr,
DeclNameFlag::AllowCompoundNames);
DeclNameRef name =
parseDeclNameRef(loc, diag::expected_expr,
DeclNameFlag::AllowCompoundNames |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);

SmallVector<TypeRepr*, 8> args;
SourceLoc LAngleLoc, RAngleLoc;
Expand Down Expand Up @@ -3123,8 +3132,10 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
Expr *SubExpr = nullptr;
if (isUnappliedOperator()) {
DeclNameLoc Loc;
auto OperName = parseDeclNameRef(Loc, diag::expected_operator_ref,
DeclNameFlag::AllowOperators);
auto OperName =
parseDeclNameRef(Loc, diag::expected_operator_ref,
DeclNameFlag::AllowOperators |
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
if (!OperName) {
return makeParserError();
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Parse/ParseType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,8 @@ ParserResult<IdentTypeRepr> Parser::parseTypeIdentifier() {
// component.
DeclNameLoc Loc;
DeclNameRef Name =
parseDeclNameRef(Loc, diag::expected_identifier_in_dotted_type, {});
parseDeclNameRef(Loc, diag::expected_identifier_in_dotted_type,
DeclNameFlag::AllowLowercaseAndUppercaseSelf);
if (!Name)
return makeParserError();

Expand Down
27 changes: 27 additions & 0 deletions test/Macros/macro_self.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %target-swift-frontend -parse %s -verify

@freestanding(expression) // expected-error {{expected expression}}
macro self() = #externalMacro(module: "MacroDefinition", type: "InvalidMacro")
Comment on lines +3 to +4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message doesn't look right.

Copy link
Member Author

@ahoppen ahoppen Mar 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same but I don’t think it’s worth improving it. This is silly code that I don’t expect anyone to write and if we think it’s worth improving the diagnostics for this, we should do so in the new parser.

The main motivation here is to make sure we don’t have to carry the baggage of supporting macros named self around later on.


func sync() {}

@freestanding(expression) // expected-error {{expected expression}}
macro Self() = #externalMacro(module: "MacroDefinition", type: "InvalidMacro")

func testSelfAsFreestandingMacro() {
_ = #self // expected-error {{expected a macro identifier for a pound literal expression}}
}

func testCapitalSelfAsFreestandingMacro() {
_ = #Self // expected-error {{expected a macro identifier for a pound literal expression}}
}

func testSelfAsAttachedMacro() {
@self // expected-error {{expected expression}}
struct Foo {}
}

func testCapitalSelfAsAttachedMacro() {
@Self // expected-error {{expected expression}}
struct Foo {}
}