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

redundant_closure: false positive for never return type #7812

Closed
Bromeon opened this issue Oct 12, 2021 · 5 comments · Fixed by #8431
Closed

redundant_closure: false positive for never return type #7812

Bromeon opened this issue Oct 12, 2021 · 5 comments · Fixed by #8431
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@Bromeon
Copy link

Bromeon commented Oct 12, 2021

Lint name: redundant_closure

With this code:

fn explode() -> ! {
    panic!("never returns")
}

fn main() {
    Some(32).unwrap_or_else(|| explode());
}

Clippy emits the following warning:

warning: redundant closure
 --> src\main.rs:6:29
  |
6 |     Some(32).unwrap_or_else(|| explode());
  |                             ^^^^^^^^^^^^ help: replace the closure with the function itself: `explode`
  |
  = note: `#[warn(clippy::redundant_closure)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

However, I cannot inline the closure and use the function pointer directly -- this code:

fn explode() -> ! {
    panic!("never returns")
}

fn main() {
    Some(32).unwrap_or_else(explode);
}

fails to compile with:

error[E0271]: type mismatch resolving `<fn() -> ! {explode} as std::ops::FnOnce<()>>::Output == {integer}`
 --> src\main.rs:6:14
  |
6 |     Some(32).unwrap_or_else(explode);
  |              ^^^^^^^^^^^^^^ expected `!`, found integer
  |
  = note: expected type `!`
             found type `{integer}`

For more information about this error, try `rustc --explain E0271`

Meta

Rust version (rustc -Vv):

rustc 1.57.0-nightly (5b210643e 2021-10-11)
binary: rustc
commit-hash: 5b210643ebf2485aafdf2494de8cf41941a64e95
commit-date: 2021-10-11
host: x86_64-pc-windows-msvc
release: 1.57.0-nightly
LLVM version: 13.0.0

Related, but not identical issues ( redundant_closure false positives):

@Bromeon Bromeon added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Oct 12, 2021
@xFrednet xFrednet added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Oct 12, 2021
@divagant-martian
Copy link

is this something someone new to clippy could work on? I'd be interested

@xFrednet
Copy link
Member

Hey @divagant-martian, this is something you could potentially work on. You basically need to get the function signature that is called in the closure and check if the return type is Never. The return type can be gained by getting the function type and then calling fn_sig on it. This will probably require some digging, but should be doable. If you like to tackle this, you can comment @rustbot claim. We're also happy to help if you're stuck :)

@divagant-martian
Copy link

divagant-martian commented Oct 13, 2021

@rustbot claim

@rustbot

This comment has been minimized.

bors bot added a commit to godot-rust/gdnative that referenced this issue Oct 14, 2021
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
@WaffleLapkin
Copy link
Member

Actually checking only for the never type doesn't seem to be enough to fix the false positives for this lint. We need to check for all coercions. For example:

fn main() {
    fn arr() -> &'static [u8; 0] { &[] }
    
    fn slice_fn(_: impl FnOnce() -> &'static [u8]) {}
    
    slice_fn(|| arr());
}
warning: redundant closure
 --> src/main.rs:6:14
  |
6 |     slice_fn(|| arr());
  |              ^^^^^^^^ help: replace the closure with the function itself: `arr`
  |
  = note: `#[warn(clippy::redundant_closure)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure

And replacing does work:

error[E0271]: type mismatch resolving `<fn() -> &'static [u8; 0] {arr} as FnOnce<()>>::Output == &'static [u8]`
 --> src/main.rs:6:5
  |
4 |     fn slice_fn(_: impl FnOnce() -> &'static [u8]) {}
  |                                     ------------- required by this bound in `slice_fn`
5 |     
6 |     slice_fn(arr);
  |     ^^^^^^^^ expected slice `[u8]`, found array `[u8; 0]`
  |
  = note: expected reference `&'static [u8]`
             found reference `&'static [u8; 0]`

(playground)

@xFrednet xFrednet added the good-first-issue These issues are a good way to get started with Clippy label Mar 10, 2022
@bors bors closed this as completed in d5c62fd Apr 27, 2022
KarimHamidou added a commit to KarimHamidou/godot-rust that referenced this issue Feb 6, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
GuilhermeOrceziae added a commit to GuilhermeOrceziae/godot-rust that referenced this issue Feb 9, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
hesuteia added a commit to hesuteia/godot-rust that referenced this issue Feb 11, 2023
793: Fix clippy lints in nightly r=Bromeon a=Bromeon

Where "fix" means mostly "disable".

Lints turned off globally for the time being (might change in the future):
* `clippy::missing_safety_doc` -- requires each unsafe fn/trait to have a `# Safety` section.
   Nice in theory, annoying for internals, and boilerplate-enforcing for obvious public cases.
* `clippy::if_then_panic` -- suggests that `if (!cond) { panic!(msg); }` be written as `assert!(cond, msg);`
   Probably makes sense to enable later, but currently not possible due to [approx/#73](brendanzab/approx#73).

Lints turned of locally:
* `clippy::redundant_closure` -- works around false positive [rust-clippy/#7812](rust-lang/rust-clippy#7812)
* `dead_code` (compiler warning, not clippy) -- generated field `this` not always used. Naming it `_this` doesn't seem right, either.

Co-authored-by: Jan Haller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
5 participants