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

Wrong error message about uninitialized variable in loop #72649

Closed
m-ou-se opened this issue May 27, 2020 · 6 comments · Fixed by #87998
Closed

Wrong error message about uninitialized variable in loop #72649

m-ou-se opened this issue May 27, 2020 · 6 comments · Fixed by #87998
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented May 27, 2020

Forgetting to initialize a non-Copy variable in a loop and then referring to it gives a 'borrowed after move' error, instead of a 'possibly-uninitialized variable' error:

fn bla(_: &mut String) {}

fn main() {
    for _ in 0..10 {
        let mut x: String;
        bla(&mut x);
    }
}
error[E0382]: borrow of moved value: `x`
 --> src/main.rs:6:13
  |
5 |         let mut x: String;
  |             ----- move occurs because `x` has type `std::string::String`, which does not implement the `Copy` trait
6 |         bla(&mut x);
  |             ^^^^^^ value borrowed here after move
7 |     }
  |     - value moved here, in previous iteration of loop

Using a Copy type like i32 does result in the right error:

error[E0381]: borrow of possibly-uninitialized variable: `x`
 --> src/main.rs:6:13
  |
6 |         bla(&mut x);
  |             ^^^^^^ use of possibly-uninitialized `x`

Tested with 1.43.0 (stable) and 1.45.0-nightly.

@m-ou-se m-ou-se added the C-bug Category: This is a bug. label May 27, 2020
@leonardo-m
Copy link

In both Ada and D languages (in different ways, in Ada it's more refined) there's a way to tell the type system that the input variable to that bla function is not initialized and that bla must initialize it, and inside bla you can't read it before it's initialized. This means you have a way to propagate the not initialized state in function arguments, and safety is preserved. In practice this is not a big problem for Rust because in Rust you can return tuples and now Rustc is getting logic to optimize this usage pattern.

@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-borrow-checker Area: The borrow checker labels May 27, 2020
@m-ou-se
Copy link
Member Author

m-ou-se commented May 27, 2020

@leonardo-m Uh, this issue is just about the error message. That this code gives an error is fine.

@m-ou-se
Copy link
Member Author

m-ou-se commented May 27, 2020

I'd be interested in fixing this myself, but I'm not sure where to start.

@estebank estebank added the D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. label Jun 30, 2020
@mpfaff
Copy link

mpfaff commented Jul 11, 2020

This is an issue for a command line program I'm writing. At the beginning of a function, I declare some uninitialized variables, and then I have a loop for each one. In this loop, it will read user input, and if it is valid, assign the variable that value. The loop will never complete without the variable being initialized, but rustc refuses to compile because it thinks the variable could be uninitialized after the loop.

@m-ou-se
Copy link
Member Author

m-ou-se commented Aug 18, 2020

@mpfaff That sounds like a different issue. This issue is about a confusing error message. The example is supposed to not compile. If the compiler rejects correct code, that's another issue.

@Plecra
Copy link

Plecra commented Mar 13, 2021

To add context, I find this example illustrates the problem more clearly for me:

fn borked() {
    loop {
        struct NonCopy;
        let value: NonCopy;
        let used = value;
    }
}
error[E0382]: use of moved value: `value`
 --> src/lib.rs:5:20
  |
4 |         let value: NonCopy;
  |             ----- move occurs because `value` has type `NonCopy`, which does not implement the `Copy` trait
5 |         let used = value;
  |                    ^^^^^ value moved here, in previous iteration of loop
  

...and the same code with the same types, just outside a loop context:

fn thanks_rust() {
    struct NonCopy;
    let value: NonCopy;
    let used = value;
}
error[E0381]: use of possibly-uninitialized variable: `value`
 --> src/lib.rs:4:16
  |
4 |     let used = value;
  |                ^^^^^ use of possibly-uninitialized `value`

nneonneo added a commit to nneonneo/rust that referenced this issue Sep 9, 2021
…" errors.

Only follow backwards edges during get_moved_indexes if the move path is
definitely initialized at loop entry. Otherwise, the error occurred prior to the
loop, so we ignore the backwards edges to avoid generating misleading "value
moved here, in previous iteration of loop" errors.

This patch also slightly improves the analysis of inits, including
NonPanicPathOnly initializations (which are ignored by
drop_flag_effects::for_location_inits). This is required for the definite
initialization analysis, but may also help find certain skipped reinits in rare
cases.

Patch passes all non-ignored src/test/ui testcases.
@bors bors closed this as completed in 6dc08b9 Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. 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