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

"path statement with no effect" issued even when drop makes effect #48852

Closed
vi opened this issue Mar 8, 2018 · 3 comments · Fixed by #75056
Closed

"path statement with no effect" issued even when drop makes effect #48852

vi opened this issue Mar 8, 2018 · 3 comments · Fixed by #75056
Labels
A-destructors Area: Destructors (`Drop`, …) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vi
Copy link
Contributor

vi commented Mar 8, 2018

struct Droppy;

impl Drop for Droppy {
    fn drop(&mut self) {
        println!("2");
    }
}

fn main() {
    let q = Droppy;
    (||{
        println!("1");
        q;
    })();
    println!("3");
}

(playground link)

warning: path statement with no effect
  --> src/main.rs:13:9
   |
13 |         q;
   |         ^^
   |
   = note: #[warn(path_statements)] on by default

Removing the "no effect" line definitely has effect.

The warning may instead suggest to explicitly use std::mem::drop if that is what is needed.

@vi vi changed the title "path statement with no effect" is issues even when drop makes effect "path statement with no effect" issued even when drop makes effect Mar 8, 2018
@Centril Centril added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels Mar 8, 2018
@ExpHP
Copy link
Contributor

ExpHP commented Mar 10, 2018

Huh, I ran into this a couple weeks ago and typed up an issue, then decided at the last second to back out of it for some reason. Maybe I just figured that writing let _ = q; is more idiomatic anyways.

This lint is old, by the way. Older than older than old. (in Rust years, anyways)

@vi
Copy link
Contributor Author

vi commented Mar 10, 2018

let _ = q; requires adding move before closure (somewhat suprisingly unlike let _q = q;)

Writing mem::drop(q) instead of let _ = q may be more idiomatic if one wants to emphasize that running Drop::drop is significant.

@CAD97
Copy link
Contributor

CAD97 commented Feb 20, 2020

Bumping this, as it's relevant to this recent Twitter poll. (See also rust-lang/rust-clippy#5205, rust-lang/rust-clippy#5206.)

When the path statement invokes a move, this lint is plain wrong; as the value has been moved, the old binding is no longer usable. When this move invokes drop glue, this lint is even more wrong, because the drop glue is an observable effect, and removing the statement would change the semantics of the program.

"Path statement with no effect is further potentially damaging, as it may lead the user to write let _ = $path; to silence the warning, which again changes the semantics of the program, because binding to the _ wildcard pattern does actually not perform a move from the place.

I believe the correct behavior for this lint would be to suggest removing path statements that have no associated drop glue, but to suggest drop($path) for types that do have drop glue, signaling clarity of intent.

@jonas-schievink jonas-schievink added A-destructors Area: Destructors (`Drop`, …) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 20, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Aug 3, 2020
…-obk

Lint path statements to suggest using drop when the type needs drop

Fixes rust-lang#48852. With this change the current lint description doesn't really fit entirely anymore I think.
Manishearth added a commit to Manishearth/rust that referenced this issue Aug 3, 2020
…-obk

Lint path statements to suggest using drop when the type needs drop

Fixes rust-lang#48852. With this change the current lint description doesn't really fit entirely anymore I think.
Manishearth added a commit to Manishearth/rust that referenced this issue Aug 3, 2020
…-obk

Lint path statements to suggest using drop when the type needs drop

Fixes rust-lang#48852. With this change the current lint description doesn't really fit entirely anymore I think.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 3, 2020
…-obk

Lint path statements to suggest using drop when the type needs drop

Fixes rust-lang#48852. With this change the current lint description doesn't really fit entirely anymore I think.
@bors bors closed this as completed in 485bfa7 Aug 4, 2020
@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-destructors Area: Destructors (`Drop`, …) A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants