-
Notifications
You must be signed in to change notification settings - Fork 440
[SwiftParser] Implement nonisolated(nonsending)
specifier
#3047
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
Conversation
9edef08
to
2cd441e
Compare
…ng` argument This is part of SE-0461 proposal where `nonisolated` has to be marked as `nonsending` in type context to indicate that the function type it's attached to is caller isolated.
…suming '(' It's possible that `(nonsending)` was omitted and instead we are at the parameter list of a function type. Checking ahead allows for a better diagnostics and recovery.
…s with arguments
swiftlang/swift#80668 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just a few comments inline.
diagnostics: [ | ||
DiagnosticSpec( | ||
message: "expected identifier in modifier", | ||
fixIts: ["replace 'sending' with identifier"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this error message isn’t great. I would prefer if we just accept any identifier here in the parser and let ASTGen diagnose unknown identifiers as the detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to consider here before we make a final decision: if we did allow any and all identifiers here, it would mean that we'd always parse nonisolated as a declaration modifier, which is a break from what C++ parser is doing and is not always correct.
Consider this situation:
func nonisolated() {
}
let v: Int = 42
nonisolated(v)
func test() -> Int { 42 }
This is currently a call + a decl but it would be interpreted as a a decl with a modifier with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let’s just stick with the current solution. It’s not the best but maybe good enough for now. And I don’t want to block this PR on improving this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
edc83c8
to
1df326d
Compare
1df326d
to
2b13304
Compare
swiftlang/swift#80668 |
swiftlang/swift#80668 |
…odifiers while skipping Otherwise in situations like `nonisolated()` `canParseType` would return `true` because `()` is valid type.
…ing` in expression context
ccc7696
to
2ae464e
Compare
swiftlang/swift#80668 |
swiftlang/swift#80668 |
swiftlang/swift#80668 |
This is part of SE-0461 proposal where
nonisolated
has to bemarked as
nonsending
in type context to indicate that thefunction type it's attached to is caller isolated.