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

Disallow cast with trailing braced macro in let-else #125049

Merged
merged 3 commits into from May 22, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented May 12, 2024

This fixes an edge case I noticed while porting #118880 and #119062 to syn.

Previously, rustc incorrectly accepted code such as:

let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! {
    8
} else {
    return;
};

even though a right curl brace } directly before else in a let...else statement is not supposed to be valid syntax.

@rustbot
Copy link
Collaborator

rustbot commented May 12, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 May 12, 2024
@dtolnay dtolnay added the A-parser Area: The parsing of Rust source code to an AST. label May 12, 2024
@@ -3,8 +3,7 @@
#![feature(explicit_tail_calls)]

fn a() {
let foo = {
//~^ WARN irrefutable `let...else` pattern
Copy link
Member

Choose a reason for hiding this comment

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

Did other errors in this file kill these suggestions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed them intentionally in f493143 because 48% of tests/ui/parser/bad-let-else-statement.stderr was these warnings unrelated to the thing that this file is trying to test (the parser), which seemed excessive.

The "irrefutable let...else pattern" warning is already tested independently by tests/ui/let-else/let-else-irrefutable.rs.

@compiler-errors
Copy link
Member

I consider this an extension to #119062. Let me look at this once again after dinner, but I'll probably approve it w/o another crater or lang FCP.

Comment on lines 116 to 127
Cast(_, ty) => {
break type_trailing_brace(ty).then_some(expr);
}
Copy link
Member

Choose a reason for hiding this comment

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

Only ast::TyKind::MacCall could cause this to be trailed with }(ast::TyKind::AnonStruct and ast::TyKind::AnonStruct can only be used in struct fields). So it will be more accurate to report the error using errors::WrapInParentheses::MacroArgs in check_let_else_init_trailing_brace.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. I have implemented this improvement.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 22, 2024

📌 Commit a36b94d has been approved by compiler-errors

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 May 22, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#124896 (miri: rename intrinsic_fallback_checks_ub to intrinsic_fallback_is_spec)
 - rust-lang#125015 (Pattern types: Prohibit generic args on const params)
 - rust-lang#125049 (Disallow cast with trailing braced macro in let-else)
 - rust-lang#125259 (An async closure may implement `FnMut`/`Fn` if it has no self-borrows)
 - rust-lang#125296 (Fix `unexpected_cfgs` lint on std)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5b485f0 into rust-lang:master May 22, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup merge of rust-lang#125049 - dtolnay:castbrace, r=compiler-errors

Disallow cast with trailing braced macro in let-else

This fixes an edge case I noticed while porting rust-lang#118880 and rust-lang#119062 to syn.

Previously, rustc incorrectly accepted code such as:

```rust
let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! {
    8
} else {
    return;
};
```

even though a right curl brace `}` directly before `else` in a `let...else` statement is not supposed to be valid syntax.
@fmease
Copy link
Member

fmease commented May 22, 2024

bors sleepy @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 22, 2024
@dtolnay dtolnay deleted the castbrace branch May 23, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

None yet

6 participants