Value assigned is never read

[kkayal]

I have the following simple code:

fn main() {
	let mut foo = String::new();
	foo = "Hi!".to_string();

	println!("{}", foo);
}

The compiler gives me the following warning:

warning: value assigned to `foo` is never read
 --> src\main.rs:2:6
  |
2 |     let mut foo = String::new();
  |         ^^^^^^^
  |
  = note: #[warn(unused_assignments)] on by default

This warning seems to be wrong to me. The assignment foo has been used in the println! macro. So I should not get this error. When I remove the mutable assignment as follows, I get no warning as expected:

fn main() {
	let foo = "Hi!".to_string();
	println!("{}", foo);
}

I am using rustc 1.24.1 (d3ae9a9 2018-02-27) on Windows 7.

[matthiaskrgr]

Hi, I think what it means is that the result of the assignment of "String::new()" is never used, since it is overwritten by "Hi!".to_string(); afterwards.

note that

fn main() {
	let foo;
	foo = "Hi!".to_string();

	println!("{}", foo);
}

doesn't trigger a warning.

[kkayal]

Hi, the warning underlines the the part mut foo. So it is telling me that I am not using the left side of the assignment:

2 |     let mut foo = String::new();
  |         ^^^^^^^

[matthiaskrgr]

Yeah, I agree that's a bit confusing.

[steveklabnik]

Tagging with A-diagnostics due to the mut issue; @matthiaskrgr is correct that fundamentally, this warning is correct.

[sanxiyn]

This is #33536 which was incorrectly closed.

[pr-yemibedu]

Hello,
Two things in the this and a point from the closed ticket:

• A message should point out the connecting value to the binding in question:
  • warning: valueString::new()assigned to binding foo is never read
• it would be nice to let the developer know that a mut was not needed from analysis

The tuple in the indirect link to the other ticket would not need a pretty print to know how to address the problem with the above advice with both lhs and rhs warnings. Like:

  |
2 |     let (mut a, b) = tup;
  |             ^^^     ^^^
  |

Thank you. Good day.

[zackmdavis]

(I am working on this)

[scottmcm]

it would be nice to let the developer know that a mut was not needed from analysis

Note that upon removing the unused initializer

fn main() {
	let mut foo;
	foo = "Hi!".to_string();

	println!("{}", foo);
}

one does indeed get another warning that

warning: variable does not need to be mutable
 --> src/main.rs:2:6
  |
2 |     let mut foo;
  |         ---^^^^
  |         |
  |         help: remove this `mut`
  |
  = note: #[warn(unused_mut)] on by default

I assume in the general case getting both at once could be hard or false-positive-y, so the two-step is probably fine for this.

[pr-yemibedu]

Hello,
I agree with @scottmcm about the benefits of the layered approach staying as implemented.

What @zackmdavis has presented in the merge looks clean and useful. Only one nitpick:

• The tuple test marks the whole value. That can confuse the developer into thinking x = (3, 4) instead of just 3. Can you either expose that value (in this case 3) in the warning message itself or limit the how much of RHS is marked?
  I am not sure how valid it would always be with other structured literals, but it is a clear win here. Thank you. Good day.

[estebank]

Current output:

warning: value assigned to `foo` is never read
 --> src/main.rs:2:10
  |
2 |     let mut foo = String::new();
  |             ^^^
  |
  = note: #[warn(unused_assignments)] on by default
  = help: maybe it is overwritten before being read?

That help should have a span pointing to the reassignment of foo.

[nicklasring]

Seems wrong. Unused warning should not show up here?

let mut x: u8;
x=6;
x=7;
println!("The value of x is: {}", x);

Output:

warning: value assigned to `x` is never read
 --> src/main.rs:3:5
  |
3 |     x=6;
  |     ^
  |
  = note: `#[warn(unused_assignments)]` on by default
  = help: maybe it is overwritten before being read?

[Mark-Simulacrum]

No, the 6 written on that line is never read (it is overwritten on the next line, by setting to 7). I agree with @estebank that it'd be good to point out where the write occurs, though.

[robenson-dev]

Hello,
Two things in the this and a point from the closed ticket:

• A message should point out the connecting value to the binding in question:

  • warning: valueString::new()assigned to binding foo is never read

• it would be nice to let the developer know that a mut was not needed from analysis

The tuple in the indirect link to the other ticket would not need a pretty print to know how to address the problem with the above advice with both lhs and rhs warnings. Like:

  |
2 |     let (mut a, b) = tup;
  |             ^^^     ^^^
  |

Thank you. Good day.
retry this
let (mut _x, mut _y) = (1, 2); _x = 4; println!("{}-{}", _x, _y);

[robenson-dev]

let (mut _x, mut _y) = (1, 2); _x = 4; println!("{}-{}", _x, _y);
or

let (mut x, mut y) = (1, 2); x = x + 1; y = y + 2; println!("{}-{}", x, y);

[buzmeg]

I'd also like to chime in on this, I hit it on this (obviously cut down) snippet of code:

fn main() {

    let mut ff_loop_abort = false;
    
    loop {  // Resize loop
        ff_loop_abort = false;
        
        loop {  // Rendering loop
            if true {  // A: Standin for complex conditional
                ff_loop_abort = true;
                break;
            }
        }
    
        if ff_loop_abort {
            if true {  // B: Standin for complex conditional
                break;
            }
        }
    }
    
    if ff_loop_abort {
        println!("loop aborted abnormally");
    }
    
}

The issue here is that the innermost loop is a 60 FPS rendering loop. The outer loop is a window resize loop. So, at the B: conditional, if the issue is resize, we can possibly adjust the window and recover and go back to rendering, but sometimes we can't.

So, while I do overwrite the value, I need the binding to live long enough to be able to do the right thing. Otherwise, I have to start creating multiples values at different scopes just to abort my loops correctly with appropriate cleanup.

I'm not sure I consider this a bug as I can work around it if I really wanted to. I can obviously create separate variables and scope everything properly to avoid the warning with the attendant extra code and grief.

However, most of the time I just consider this a false positive and move on with life since that code may disappear at some point if I do a refactor for some reason.

Thanks.

[scottmcm]

So, while I do overwrite the value, I need the binding to live long enough to be able to do the right thing.

At least one of those initializations can be trivially removed:

    let mut ff_loop_abort;
    loop {  // Resize loop
        ff_loop_abort = false;
        ... lots of code here ...
    }
    if ff_loop_abort {
        println!("loop aborted abnormally");
    }

rustc knows that loop always runs at least once, so you don't need to initialize it with a value. The later assignment at the top of the loop is sufficient.