-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Fix unused_parens false positive #143698
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 unused_parens false positive #143698
Conversation
#![allow(warnings)] | ||
trait MyTrait {} | ||
|
||
fn foo(_: Box<dyn FnMut(&mut u32) -> &mut (dyn MyTrait) + Send + Sync>) {} |
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.
How does this behave with dyn FnMut(&()) -> &(dyn Trait)
(i.e. one bound that is parenthesized)?
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've added that case to the test: We continue to emit a warning.
compiler/rustc_lint/src/unused.rs
Outdated
let own_constraint = self.in_no_bounds_pos.get(&ty.id).copied(); | ||
let constraint = own_constraint.unwrap_or(NoBoundsException::OneBound); | ||
self.in_no_bounds_pos.insert(mut_ty.ty.id, constraint); |
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.
in a sense we want tis to be constraint
= own_constraint.max(NoBoundsException::OneBound)
where as NoBoundsException::OneBound
is currently the minimal value this is correct?
I would feel safer if we were to make this a match on own_constraint
or add a NoBoundsException.merge
function or sth 🤔
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.
in a sense we want tis to be
constraint
=own_constraint.max(NoBoundsException::OneBound)
where asNoBoundsException::OneBound
is currently the minimal value this is correct?
Correct.
I would feel safer if we were to make this a match on
own_constraint
or add aNoBoundsException.merge
function or sth 🤔
I see where you're coming from. I went with the match because 1) I don't see this sprawling and 2) the Option
makes the alternatives a bit "messy". Happy to take this any other direction though. 🙂
This comment has been minimized.
This comment has been minimized.
9655a85
to
307f664
Compare
thx, sorry for the slow review! @bors r=lcnr,compiler-errors rollup |
…compiler-errors Fix unused_parens false positive Resolves rust-lang#143653. The "no bounds exception" was indiscriminately set to `OneBound` for referents and pointees. However, if the reference or pointer type itself appears in no-bounds position, any constraints it has must be propagated. ```rust // unused parens: not in no-bounds position fn foo(_: Box<(dyn Send)>) {} // unused parens: in no-bounds position, but one-bound exception applies fn bar(_: Box<dyn Fn(&u32) -> &(dyn Send)>) {} // *NOT* unused parens: in no-bounds position, but no exceptions to be made fn baz(_: Box<dyn Fn(&u32) -> &(dyn Send) + Send>) {} ```
…compiler-errors Fix unused_parens false positive Resolves rust-lang#143653. The "no bounds exception" was indiscriminately set to `OneBound` for referents and pointees. However, if the reference or pointer type itself appears in no-bounds position, any constraints it has must be propagated. ```rust // unused parens: not in no-bounds position fn foo(_: Box<(dyn Send)>) {} // unused parens: in no-bounds position, but one-bound exception applies fn bar(_: Box<dyn Fn(&u32) -> &(dyn Send)>) {} // *NOT* unused parens: in no-bounds position, but no exceptions to be made fn baz(_: Box<dyn Fn(&u32) -> &(dyn Send) + Send>) {} ```
Rollup of 9 pull requests Successful merges: - #140871 (Don't lint against named labels in `naked_asm!`) - #141663 (rustdoc: add ways of collapsing all impl blocks) - #143272 (Upgrade the `fortanix-sgx-abi` dependency) - #143585 (`loop_match`: suggest extracting to a `const` item) - #143698 (Fix unused_parens false positive) - #143859 (Guarantee 8 bytes of alignment in Thread::into_raw) - #144042 (Verify llvm-needs-components are not empty and match the --target value) - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc) - #144431 (Disable has_reliable_f128_math on musl targets) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #140871 (Don't lint against named labels in `naked_asm!`) - #141663 (rustdoc: add ways of collapsing all impl blocks) - #143272 (Upgrade the `fortanix-sgx-abi` dependency) - #143585 (`loop_match`: suggest extracting to a `const` item) - #143698 (Fix unused_parens false positive) - #143859 (Guarantee 8 bytes of alignment in Thread::into_raw) - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc) - #144412 (Small cleanup: Use LocalKey<Cell> methods more) - #144431 (Disable has_reliable_f128_math on musl targets) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143698 - benschulz:unused-parens-2, r=lcnr,compiler-errors Fix unused_parens false positive Resolves #143653. The "no bounds exception" was indiscriminately set to `OneBound` for referents and pointees. However, if the reference or pointer type itself appears in no-bounds position, any constraints it has must be propagated. ```rust // unused parens: not in no-bounds position fn foo(_: Box<(dyn Send)>) {} // unused parens: in no-bounds position, but one-bound exception applies fn bar(_: Box<dyn Fn(&u32) -> &(dyn Send)>) {} // *NOT* unused parens: in no-bounds position, but no exceptions to be made fn baz(_: Box<dyn Fn(&u32) -> &(dyn Send) + Send>) {} ```
Resolves #143653.
The "no bounds exception" was indiscriminately set to
OneBound
for referents and pointees. However, if the reference or pointer type itself appears in no-bounds position, any constraints it has must be propagated.