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

"variable does not need to be mutable" not detected #61424

Closed
ghost opened this issue Jun 1, 2019 · 7 comments · Fixed by #61446
Closed

"variable does not need to be mutable" not detected #61424

ghost opened this issue Jun 1, 2019 · 7 comments · Fixed by #61446
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ghost
Copy link

ghost commented Jun 1, 2019

(sorry if this is an duplicate or the wrong repo 😇 )

fn main() {
    let mut name;
    name = format!("foo{}", "bar");
    dbg!(name);
}

(Playground)

Current behavior:
no warning

Expected behavior:

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

The variable clearly doesn't need to be mutable, still, nothing is detected and no warning is emitted.

Rust 1.35.0
(Using the playground, also in 1.37.0-nightly (2019-05-31 7840a0b753a065a41999))

@varkor varkor added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. labels Jun 1, 2019
@czipperz
Copy link
Contributor

czipperz commented Jun 1, 2019

The warning appears again if you switch to 2015 edition. Huh. Nice catch!

@czipperz
Copy link
Contributor

czipperz commented Jun 1, 2019

I found that Stable 2015 edition produces the warning and everything else doesn't.

| Warning | Optimization | Channel | Edition |
|---------+--------------+---------+---------|
| Present | Debug        | Stable  |    2015 |
| Bug!    | Debug        | Beta    |    2015 |
| Bug!    | Debug        | Nightly |    2015 |
| Present | Release      | Stable  |    2015 |
| Bug!    | Release      | Beta    |    2015 |
| Bug!    | Release      | Nightly |    2015 |
| Bug!    | Debug        | Stable  |    2018 |
| Bug!    | Debug        | Beta    |    2018 |
| Bug!    | Debug        | Nightly |    2018 |
| Bug!    | Release      | Stable  |    2018 |
| Bug!    | Release      | Beta    |    2018 |
| Bug!    | Release      | Nightly |    2018 |

@jonas-schievink jonas-schievink added A-NLL Area: Non-lexical lifetimes (NLL) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 1, 2019
@jonas-schievink
Copy link
Contributor

cc @matthewjasper (likely caused by NLL migrate mode, which was enabled in #59114 for the 2015 edition)

@matthewjasper
Copy link
Contributor

Yes, looks like another nll bug. If someone wants to have a go at this, the issue is that we are treating DropAndReplace differently to Assign for the purposes of this lint.

@matthewjasper matthewjasper self-assigned this Jun 1, 2019
@czipperz
Copy link
Contributor

czipperz commented Jun 1, 2019

I can try doing it I just don't know here to begin.

@jonas-schievink
Copy link
Contributor

@czipperz I don't know this code too well, but take a look at src/librustc_mir/borrow_check/used_muts.rs. The first thing I'd try is adding a case for DropAndReplace to the match in visit_statement that mirrors what's done for Assign.

@czipperz
Copy link
Contributor

czipperz commented Jun 1, 2019

Makes sense thank you. I was looking at librustc_lint and couldn't find anything.

@ghost ghost changed the title "variable does not need to be mutable" no detected "variable does not need to be mutable" not detected Jun 2, 2019
Centril added a commit to Centril/rust that referenced this issue Jun 4, 2019
…asper

On TerminatorKind::DropAndReplace still handle unused_mut correctly

Closes rust-lang#61424

- [x] Todo add regression test
Centril added a commit to Centril/rust that referenced this issue Jun 4, 2019
…asper

On TerminatorKind::DropAndReplace still handle unused_mut correctly

Closes rust-lang#61424

- [x] Todo add regression test
Centril added a commit to Centril/rust that referenced this issue Jun 4, 2019
…asper

On TerminatorKind::DropAndReplace still handle unused_mut correctly

Closes rust-lang#61424

- [x] Todo add regression test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. 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