Change return of SkipPastLikelyEnd to be last consumed token
#3493
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonmeow
approved these changes
Dec 11, 2023
zygoloid
approved these changes
Dec 11, 2023
Co-authored-by: Jon Ross-Perkins <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 13, 2023
Note #3486 rewrites the macro behavior, and is already approved: so this PR is only for the changed enforcement during error. Also, #3493 already changed several things to allow any token while this PR was awaiting review, but this still changes enforcement for `For` and `If`. This was brought up on [#toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1182066616456970251), and I think this any-on-error approach gets at least some support. We could try setting it to the introducer, but it's quite possible we want it to be something like the token which led to the parse error, rather than a static token. That leads to a conclusion that, most typically, we'll expect arbitrary tokens when error conditions may lead to tokens which aren't the expected token. A couple related, recent `CARBON_IF_ERROR` crash fixes can be found in #3404 and #3424. Something like #3404 would've been needed regardless because `namespace` didn't have `CARBON_IF_ERROR` before, although I might've missed the underlying issue with declarations because only `namespace` had a relevant test (that is, if #3404 had added `CARBON_ANY_TOKEN_ON_ERROR`, I wouldn't have had a crash in #3462). #3424 would've been avoided with this change because there was a `CARBON_IF_ERROR`, and it was just too restrictive.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This approach means the parse subtree includes the full token range consumed.