Skip to content

Fix error recovery in format macro parsing #88835

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

FabianWolff
Copy link
Contributor

Fixes #88770. Basically, the assumption in the following comment is incorrect:

// `Parser::expect` tries to recover using the
// `Parser::unexpected_try_recover` function. This function is able
// to recover if the expected token is a closing delimiter.
//
// As `,` is not a closing delimiter, it will always return an `Err`
// variant.

This is only true in the first iteration of the loop, when p.clear_expected_tokens() is called. In subsequent iterations, p.expected_tokens won't be empty, so p.expect() won't actually call unexpected_try_recover():

pub fn expect(&mut self, t: &TokenKind) -> PResult<'a, bool /* recovered */> {
if self.expected_tokens.is_empty() {
if self.token == *t {
self.bump();
Ok(false)
} else {
self.unexpected_try_recover(t)
}
} else {
self.expect_one_of(slice::from_ref(t), &[])
}
}

Instead, it will call expect_one_of(), which can recover and return Ok(). This PR handles this case to fix the ICE in #88770.

@FabianWolff
Copy link
Contributor Author

It seems that @rustbot somehow passed over this PR and did not assign anybody. Therefore (feel free to reassign):

r? @petrochenkov

@petrochenkov petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2021
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Sep 22, 2021

📌 Commit a8421ca has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 22, 2021
@bors
Copy link
Collaborator

bors commented Sep 23, 2021

⌛ Testing commit a8421ca with merge 43cb20b43f165d2be40c99326f383feffd2eea94...

@bors
Copy link
Collaborator

bors commented Sep 23, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 23, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@petrochenkov
Copy link
Contributor

Looks like some infra issue.
@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 23, 2021
@bors
Copy link
Collaborator

bors commented Sep 24, 2021

⌛ Testing commit a8421ca with merge a0648ea...

@bors
Copy link
Collaborator

bors commented Sep 24, 2021

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing a0648ea to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 24, 2021
@bors bors merged commit a0648ea into rust-lang:master Sep 24, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 24, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a0648ea): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'called Result::unwrap_err() on an Ok value: false', compiler/rustc_builtin_macros/src/format.rs:173:51
6 participants