Skip to content

Disallow the self/Self keyword where it’s not allowed in types #1986

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.

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 2 commits into from
Aug 23, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 2, 2023

We weren’t matching the C++ parser’s behavior here. The self keyword is not allowed as a type name. Self in a nested type refers to a type named Self instead of the Self keyword.

@ahoppen ahoppen requested a review from bnbarham August 2, 2023 23:06
@ahoppen
Copy link
Member Author

ahoppen commented Aug 2, 2023

@swift-ci Please test

@ahoppen ahoppen marked this pull request as draft August 3, 2023 21:12
@ahoppen ahoppen force-pushed the ahoppen/disallow-self-in-types branch from 056901f to f75b5fd Compare August 18, 2023 17:11
@ahoppen ahoppen marked this pull request as ready for review August 18, 2023 17:11
@ahoppen
Copy link
Member Author

ahoppen commented Aug 18, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Aug 18, 2023

@swift-ci Please test Windows

DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected 'self' keyword before type"),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected type", fixIts: ["insert type"]),
],
fixedSource: "<#type#>self"
Copy link
Contributor

Choose a reason for hiding this comment

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

<#type#>.self?

Copy link
Member Author

Choose a reason for hiding this comment

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

We consume self as unexpected here. I’m not sure if <#type#>.self is really any more useful than what we currently produce, so not sure if it’s worth adding a special case for this recovery.

To put it into context, if you write let a: self, I don’t think that let a: <#type#>.self is a meaningful recovery strategy. We could suggest wrapping self in backticks here, but I’m 100% sure that’s not what the developer intended either.


assertParse(
"""
@derivitave(of: 1️⃣Self.other)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@derivitave(of: 1️⃣Self.other)
@derivative(of: 1️⃣Self.other)

@bnbarham bnbarham requested a review from rintaro August 18, 2023 18:44
We weren’t matching the C++ parser’s behavior here. The `self` keyword is not allowed as a type name. `Self` in a nested type refers to a type named `Self` instead of the `Self` keyword.
@ahoppen ahoppen force-pushed the ahoppen/disallow-self-in-types branch from f75b5fd to d171dc5 Compare August 21, 2023 21:46
…rse`

In many of these places, the test case tested the exact same thing if we didn’t pass a custom `parse` function to `assertParsse`. Default to `SourceFileSyntax.parse` because it produces the more natural diagnostics.
@ahoppen
Copy link
Member Author

ahoppen commented Aug 21, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Aug 23, 2023

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge August 23, 2023 14:38
@ahoppen ahoppen merged commit b57df1e into swiftlang:main Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants