Skip to content

Unused warnings should trigger for swaps #2911

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

Closed
catamorphism opened this issue Jul 13, 2012 · 7 comments
Closed

Unused warnings should trigger for swaps #2911

catamorphism opened this issue Jul 13, 2012 · 7 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-type-system Area: Type system C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@catamorphism
Copy link
Contributor

From @jruderman 's comment on #1832:

IMO, the "unused" warning should be per-assignment, not per-variable.

fn main() {
   let mut x = 10;
   x = 20;
   log(error, x);
}

should warn because the first assignment to x is unused.

fn main() {
   let mut x = 10;
   log(error, x);
   x = 20;
}

should warn because the second assignment to x is unused. Finally, the original example,

fn main() {
   let mut x = 10, y = 20;
   x <-> y;
   log(error, y);
}

should warn because the second assignment to x is unused.

@eholk
Copy link
Contributor

eholk commented Jul 13, 2012

I don't think the third case should warn. We have a pattern cropping up that goes something like this:

let mut x = some(thing);
do some(foo).map |foo| {
    let mut y = none;
    x <-> y;
    let x = option::unwrap(y);
    // do stuff with x
}

This is basically the dance that's needed to get an uncopyable into a closure in a way that you can move it to other places.

@eholk
Copy link
Contributor

eholk commented Jul 13, 2012

Actually, I guess the problem is not whether the third example warns, but the fact that we have to do all that with it'd be nice to do stuff like...

let x = thing;
do some(foo).map |foo, move x| {
    // do stuff with x, including possibly giving up ownership.
}

@nikomatsakis
Copy link
Contributor

The first and second example already issue warnings at the correct places.

@catamorphism
Copy link
Contributor Author

Sounds like this is done, then.

@jruderman
Copy link
Contributor

The warning wouldn't apply in eholk's case because the lhs of the swap is an upvar. I still think my third case should warn.

@nikomatsakis
Copy link
Contributor

I am inclined to agree.

@ghost ghost assigned catamorphism Jul 20, 2012
@alexcrichton
Copy link
Member

Since the swap operator no longer exists, I think this can be closed (if I'm wrong, feel free to reopen!)

@catamorphism catamorphism removed their assignment Jun 16, 2014
RalfJung pushed a commit to RalfJung/rust that referenced this issue Jun 3, 2023
celinval added a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Changes required due to:
- rust-lang/rust@99ac405b96 Move MetadataLoader{,Dyn} to rustc_metadata.
- rust-lang/rust@c997c6d822 Add more information to stable Instance
- rust-lang#116915

This also fixes an issue in the `simd_shuffle` implementation that was exposed by the update.

Resolves rust-lang#2911

---------

Co-authored-by: Celina G. Val <[email protected]>
Co-authored-by: Adrian Palacios <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-type-system Area: Type system C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

No branches or pull requests

5 participants