Skip to content
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

Fix ICE in diagnostics for parenthesized type arguments #122400

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

wutchzone
Copy link
Contributor

@wutchzone wutchzone commented Mar 12, 2024

The second time is the charm 🤞 😁

Fixes #122345

r? fmease

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 12, 2024
@@ -449,7 +449,11 @@ impl<'a> Parser<'a> {
prev_token_before_parsing: Token,
error: &mut Diag<'_>,
) {
if ((style == PathStyle::Expr && self.parse_paren_comma_seq(|p| p.parse_expr()).is_ok())
if ((style == PathStyle::Expr
Copy link
Member

@fmease fmease Mar 12, 2024

Choose a reason for hiding this comment

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

I've pondered this in the original PR, too. We might be able to make this check more readable by rewriting it into sth. like that:

if !match style {
    PathStyle::Expr => /* parse seq is ok */,
    PathStyle::Pat => /* parse seq is ok */,
    _ => false,
} {
    return;
}
let (token::ModSep | token::RArrow) = self.token.kind else { return };
/* span suggestion verbose */

However that might be even more confusing lol ^^

Copy link
Member

Choose a reason for hiding this comment

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

Or

match style {
    PathStyle::Expr if let Ok(_) = /* parse seq */ => {}
    PathStyle::Pat if let Ok(_) = /* parse seq */ => {}
    _ => return,
}
let (token::ModSep | token::RArrow) = self.token.kind else { return };
/* span suggestion verbose */

Copy link
Member

@fmease fmease Mar 12, 2024

Choose a reason for hiding this comment

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

No need to apply my suggestions if they turn out to be illegible! Just throwing something out there because the check is growing in complexity :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree, the if has grown into a chungus. I kinda like the match-if-let variant, even though the empty body of the match arm kinda bothers me. But it looks more readable to me.

Also, shouldn't the token deconstruction should be more like this?

let (token::ModSep | token::RArrow) = self.token.kind else { 
    /* span suggestion verbose */
   return;
 };

Because we do not want to emit the suggestion when ModSep or RArrow is present.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you are right about the last part, yes it should be sth. like if let ModSep | RArrow = … { return } /* suggestion */

@fmease
Copy link
Member

fmease commented Mar 12, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 12, 2024

📌 Commit eab1f30 has been approved by fmease

It is now in the queue for this repository.

@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 Mar 12, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 13, 2024
Fix ICE in diagnostics for parenthesized type arguments

The second time is the charm 🤞 😁

Fixes rust-lang#122345

r? fmease
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121820 (pattern analysis: Store field indices in `DeconstructedPat` to avoid virtual wildcards)
 - rust-lang#121908 (match lowering: don't collect test alternatives ahead of time)
 - rust-lang#122203 (Add `intrinsic_name` to get plain intrinsic name)
 - rust-lang#122226 (coverage: Remove or migrate all unstable values of `-Cinstrument-coverage`)
 - rust-lang#122255 (Use `min_exhaustive_patterns` in core & std)
 - rust-lang#122360 ( Don't Create `ParamCandidate` When Obligation Contains Errors )
 - rust-lang#122375 (CFI: Break tests into smaller files)
 - rust-lang#122383 (Enable PR tracking review assignment for rust-lang/rust)
 - rust-lang#122386 (Move `Once` implementations to `sys`)
 - rust-lang#122400 (Fix ICE in diagnostics for parenthesized type arguments)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121820 (pattern analysis: Store field indices in `DeconstructedPat` to avoid virtual wildcards)
 - rust-lang#121908 (match lowering: don't collect test alternatives ahead of time)
 - rust-lang#122203 (Add `intrinsic_name` to get plain intrinsic name)
 - rust-lang#122226 (coverage: Remove or migrate all unstable values of `-Cinstrument-coverage`)
 - rust-lang#122255 (Use `min_exhaustive_patterns` in core & std)
 - rust-lang#122360 ( Don't Create `ParamCandidate` When Obligation Contains Errors )
 - rust-lang#122383 (Enable PR tracking review assignment for rust-lang/rust)
 - rust-lang#122386 (Move `Once` implementations to `sys`)
 - rust-lang#122400 (Fix ICE in diagnostics for parenthesized type arguments)
 - rust-lang#122410 (rustdoc: do not preload fonts when browsing locally)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1ffa5de into rust-lang:master Mar 13, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 13, 2024
Rollup merge of rust-lang#122400 - wutchzone:122345, r=fmease

Fix ICE in diagnostics for parenthesized type arguments

The second time is the charm 🤞 😁

Fixes rust-lang#122345

r? fmease
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ice: error was constructed but not emitted
4 participants