Lint: useless assignments to Copy temporaries

[michaelsproul]

What it does

Detect assignments to Copy values that are immediately discarded.

rustc is pretty good at detecting when variables are assigned but never read, and Clippy has a temporary_assignments lint to catch assignments to temporaries, but neither of them catch the following case:

struct Point { x: u64, y: u64 }

fn new_point() -> Point {
    Point {
        x: 10,
        y: 30,
    }
}

fn main() {
    new_point().x = 11;
}

Whether or not new_point has side-effects should be irrelevant: assigning to its .x field and then immediately throwing it away is definitely pointless (no pun intended 😏). Things are more complicated if Point has a drop implementation, hence the lint should probably be restricted to types that implement Copy.

Categories

• Kind: correctness

Drawbacks

Potentially difficult to implement

Example

This example is more similar to the one I encountered in the real world: a Copy type with a mutable getter and a copying getter, where the copying getter is called by mistake:

#[derive(Debug, Clone, Copy)]
struct Point { x: u64, y: u64 }

#[derive(Debug)]
struct Container {
    point: Point,
}

impl Container {
    fn get_point(&self) -> Point {
        self.point
    }
    
    #[allow(unused)]
    fn get_point_mut(&mut self) -> &mut Point {
        &mut self.point
    }
}

fn main() {
    let container = Container {
        point: Point { x: 10, y: 30 },
    };
    container.get_point().x = 0; // oops! I meant `get_point_mut`!
    println!("{:?}", container);
}

[camsteffen]

Things are more complicated if Point has a drop implementation, hence the lint should probably be restricted to types that implement Copy.

We can check if the type implements Drop.

[Pzixel]

I think it may be worthwhile to implement as a hard error in rustc itself. I couldn't come with any valid scenario even for Drop types. And ever then it could be rewritten with temporary local

let p = foo.get_point();
p.x = 10;

[ghost]

This looks like an issue with the rustc lint, unused-assignments, to me.

[Pzixel]

Well you can always add explicit drop(p) in the end. It's a bit quircky I agree but how many times anyone would write this mutate-and-call-drop-to-make-some-side-effect? It something really weird and weird things deserve to be longer to make people think why they are doing this (hence long unsafe method names btw) and they also may be longer for sake of saving general case, which is the case in our case.

[camsteffen]

This looks like an issue with the rustc lint, unused-assignments, to me.

Agreed. This is maybe rust-lang/rust#65467.