-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Do not trigger unused_braces
for while let
#75083
Conversation
fn main() { | ||
let mut a = Some(3); | ||
// Shouldn't warn below `a`. | ||
while let Some(ref mut v) = {a} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it is not linted for unnamed struct literal in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not quite sure what you mean here, we can't warn for { a }
if it's used in patterns because it can
actually change the behavior there. From the docs
/// Due to `ref` pattern, there can be a difference between using
/// `{ expr }` and `expr` in pattern-matching contexts. This means
/// that we should only lint `unused_parens` and not `unused_braces`
/// in this case.
///
/// ```rust
/// let mut a = 7;
/// let ref b = { a }; // We actually borrow a copy of `a` here.
/// a += 1; // By mutating `a` we invalidate any borrows of `a`.
/// assert_eq!(b + 1, a); // `b` does not borrow `a`, so we can still use it here.
/// ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=d28588845e2466227a28572bb6929b30
What I trying to say is that currently we don't allow struct literals in if, while-let.
struct Foo {
a: u32
}
fn foo() {
while let Foo { a } = Foo { a: 42} {
()
}
}
By enabling around { a }
, unnamed structs in the future might be hard to be used/migrate.
I am not against this changes, also am not authorative.
What I want is improving the precision of the lint around {( yield )}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you want forbid while let pat = { a }
?
I still can't completely follow you here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it kind of ambiguous in parsing. But I'm not authoritative here.
so let's cc parser devs like @petrochenkov @estebank
I'm sorry for causing this noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already allow while let pat = { a } { /**/ }
so I don't really think we can change that now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But with the unused braces warning people will feel discouraged to use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while let pat = { a }
also doesn't cause ambiguity here as while let _ =
expects expr {
so the {
of { a }
must be part of expr
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kinda get it your point. Does it mean parsing unnamed structs at expr is unambiguous and allowed?
Even though struct literals are forbidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But with the unused braces warning people will feel discouraged to use it.
The unused braces warning can break code here and imo rustc should try quite hard to not emit potentially incorrect warnings
Thanks ❤️ @bors r+ rollup |
📌 Commit 2e5c501 has been approved by |
…r=lcnr Do not trigger `unused_braces` for `while let` Follow-up for rust-lang#75031 r? @lcnr
…r=lcnr Do not trigger `unused_braces` for `while let` Follow-up for rust-lang#75031 r? @lcnr
…r=lcnr Do not trigger `unused_braces` for `while let` Follow-up for rust-lang#75031 r? @lcnr
…r=lcnr Do not trigger `unused_braces` for `while let` Follow-up for rust-lang#75031 r? @lcnr
Rollup of 8 pull requests Successful merges: - rust-lang#74759 (add `unsigned_abs` to signed integers) - rust-lang#75043 (rustc_ast: `(Nested)MetaItem::check_name` -> `has_name`) - rust-lang#75056 (Lint path statements to suggest using drop when the type needs drop) - rust-lang#75081 (Fix logging for rustdoc) - rust-lang#75083 (Do not trigger `unused_braces` for `while let`) - rust-lang#75084 (Stabilize Ident::new_raw) - rust-lang#75103 (Disable building rust-analyzer on riscv64) - rust-lang#75106 (Enable docs on in the x86_64-unknown-linux-musl manifest) Failed merges: r? @ghost
Follow-up for #75031
r? @lcnr