-
Notifications
You must be signed in to change notification settings - Fork 440
[reference-bindings] Add support for borrow, inout bindings. #1363
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
[reference-bindings] Add support for borrow, inout bindings. #1363
Conversation
@swift-ci test |
@ahoppen do I need to update anything else? |
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.
What is stopping you from just using borrow
and inout
as keywords here? If you want to introduce this as a staged process with new keywords for now, I think it would make sense to underscore borrowBindingKW
and inoutBindingKW
to indicate they aren’t the final design.
a1896bd
to
db1a31c
Compare
@swift-ci test |
db1a31c
to
306ac7b
Compare
@swift-ci test |
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.
One improvement suggestion for a comment, otherwise LGTM.
// | ||
// NOTE: We reuse this for inout bindings and choose the higher precedence level of expr keywords | ||
// so we do not break anything. |
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.
// | |
// NOTE: We reuse this for inout bindings and choose the higher precedence level of expr keywords | |
// so we do not break anything. | |
// | |
// NOTE: We reuse this for inout bindings and choose the lower precedence level of expr keywords | |
// so we don’t skip over expression keywords while looking for `inout` in an expression position. |
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.
@ahoppen I am going to do that in a follow on commit. I don't want to restart CI
@swift-ci test |
306ac7b
to
3b021d7
Compare
@swift-ci test |
…ypes. This was exposed by: decl/enum/enumtest.swift The problem here is that we are now allowing for inout to be the start of a swift decl. This means that if one defines an enum case with a inout parameter, the parser gets confused and doesnt think it is part of the type. I worked around this by changing canParseTupleTypeBody to say that an inout binding cannot be parsed as part of a tuple type. This fits already with how we want to model this.
This is rdar://106262176 |
To make sure that I did not stomp on anything already working around borrow, inout, I decided to (for now) use the keywords inoutBindingKW and borrowBindingKW, just to be careful.