Skip to content

fix: Always bump in the parser in err_and_bump() #20180

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 1 commit into from
Jul 7, 2025

Conversation

ChayimFriedman2
Copy link
Contributor

@ChayimFriedman2 ChayimFriedman2 commented Jul 6, 2025

Even when at curly braces, otherwise the parser can get stuck.

This has happened in the past in #18625, but it was just worked around instead of handling the root of the problem. Now this happened again in #20171. IMO we can't let err_and_bump() not bump, that's too confusing and invites errors. We can (as I did) workaround the worse recovery instead.

Fixes #20171.

Even when at curly braces, otherwise the parser can get stuck.

This has happened in the past in rust-lang#18625, but it was just worked around instead of handling the root of the problem. Now this happened again in rust-lang#20171. IMO we can't let `err_and_bump()` not bump, that's too confusing and invites errors. We can (as I did) workaround the worse recovery instead.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 6, 2025
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

I believe the rationale here was for blocks to be always balanced or something like that, though I don't quite recall things here. See #4680 (comment) for some context.

But that is for the assumption of incremental reparsing, which I believe is something we do not do these days at all and is probably still broken given it used to fail fuzzing. So this sounds reasonable to me

@ChayimFriedman2
Copy link
Contributor Author

@Veykril can you merge this please? You approved but not merged.

@Veykril
Copy link
Member

Veykril commented Jul 7, 2025

You should be able to merge it as well no?

@Veykril Veykril added this pull request to the merge queue Jul 7, 2025
@ChayimFriedman2
Copy link
Contributor Author

You should be able to merge it as well no?

I don't want to self-merge usually.

Merged via the queue into rust-lang:master with commit 778e08d Jul 7, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2025
@ChayimFriedman2 ChayimFriedman2 deleted the parser-stuck branch July 7, 2025 19:07
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.

RA parser gets stuck on indexing: 0.3.2519-standalone
3 participants