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

Papercut: Suggest Default::default() when binding isn't initialized #102087

Closed
Patryk27 opened this issue Sep 21, 2022 · 4 comments · Fixed by #102184
Closed

Papercut: Suggest Default::default() when binding isn't initialized #102087

Patryk27 opened this issue Sep 21, 2022 · 4 comments · Fixed by #102184
Assignees
Labels
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.

Comments

@Patryk27
Copy link
Contributor

Patryk27 commented Sep 21, 2022

Given the following code:

fn main() {
    let foo: Vec<&str>;
    
    foo.push("hi!");
}

(playground link)

... the current output is:

error[E0381]: used binding `foo` isn't initialized
 --> src/main.rs:4:5
  |
2 |     let foo: Vec<&str>;
  |         --- binding declared here but left uninitialized
3 |     
4 |     foo.push("hi!");
  |     ^^^^^^^^^^^^^^^ `foo` used here but it isn't initialized

While this message is correct, people (especially beginners) stumbling upon it sometimes still don't know what to do (e.g. they expect let var: ty; to automatically initialize var to Default::default(), as is the case in some languages).

Maybe we could extend the message to say:

error[E0381]: used binding `foo` isn't initialized
 --> src/main.rs:4:5
  |
2 |     let foo: Vec<&str>;
  |         --- binding declared here but left uninitialized
3 |     
4 |     foo.push("hi!");
  |     ^^^^^^^^^^^^^^^ `foo` used here but it isn't initialized
  |
help: use `=` to assign some value to `foo`
  |
2 |     let foo: Vec<&str> = something;
  |                          ~~~~~~~~~

... with an extra case that if ty is Default:

help: use `=` to assign default value to `foo`
  |
2 |     let foo: Vec<&str> = Default::default();
  |                          ~~~~~~~~~~~~~~~~~~
@Patryk27 Patryk27 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. labels Sep 21, 2022
@chenyukang
Copy link
Member

@rustbot claim

@edward-shen
Copy link
Contributor

Maybe it's better to suggest ty::default() instead of Default::default()? A beginner may be confused why you can "assign" Default to a ty.

@cassaundra
Copy link
Contributor

cassaundra commented Sep 21, 2022

Maybe it's better to suggest ty::default() instead of Default::default()? A beginner may be confused why you can "assign" Default to a ty.

Agree. I'm also unsure whether Default::default or even ty::default is idiomatic often enough to be worth suggesting. I'd generally rather see a specific constructor method if one is provided in most Rust code as it's more descriptive, but obviously this isn't something we can really know about on the compiler level. If there are a bunch of other help messages that suggest this already then I suppose it doesn't matter much though.

@Patryk27
Copy link
Contributor Author

Patryk27 commented Sep 22, 2022

Maybe it's better to suggest ty::default() instead of Default::default()? A beginner may be confused why you can "assign" Default to a ty.

Sure, that's also nice, I think 🙂

but obviously this isn't something we can really know about on the compiler level.

We kinda-sorta can, right? I think it should be possible to have top-down rules such that we:

  • suggest = vec![]; (or Vec::new()) if type is Vec,
  • suggest = Ty::default(); if type is : Default,
  • suggest = something otherwise.

... although considering that Vec::new() is the same as Vec::default() (or e.g. HashMap::new() == HashMap::default(), modulo its third generic parameter, IIRC), then going with just Ty::default() might be sufficient for practical purposes.

I'm not sure on already-existing diagnostics though, maybe there exists some precedent (in which case it'd be nice to follow it).

chenyukang added a commit to chenyukang/rust that referenced this issue Sep 23, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 24, 2022
…sugg, r=nagisa

Suggest Default::default() when binding isn't initialized

Fixes rust-lang#102087
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 26, 2022
…gg, r=nagisa

Suggest Default::default() when binding isn't initialized

Fixes rust-lang#102087
@bors bors closed this as completed in 672e3f4 Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants