Skip to content

#[warn(unused_mut)] does not always warn #25049

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

Closed
nwin opened this issue May 2, 2015 · 2 comments · Fixed by #43582
Closed

#[warn(unused_mut)] does not always warn #25049

nwin opened this issue May 2, 2015 · 2 comments · Fixed by #43582
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@nwin
Copy link
Contributor

nwin commented May 2, 2015

rustc fails to warn about unused mut for buf in this case:

use std::io;

pub fn read_all<R: io::Read + ?Sized>(this: &mut R, mut buf: &mut [u8]) -> io::Result<()> {
    let mut total = 0;
    while total < buf.len() {
        match this.read(&mut buf[total..]) {
            Ok(0) => return Err(io::Error::new(io::ErrorKind::Other,
                                               "failed to read whole buffer")),
            Ok(n) => total += n,
            Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}
            Err(e) => return Err(e),
        }
    }
    Ok(())
}

fn main() {}
@steveklabnik steveklabnik added the A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. label May 2, 2015
@steveklabnik
Copy link
Member

Triage: here's the smallest reproduction:

pub fn foo(mut buf: &mut [u8]) {
    &mut buf[..];
}

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 22, 2017
@ivanbakel
Copy link
Contributor

Small update while looking to this: the borrow checker looks like it's doing a ref adjustment when the types are already compatible. Another minimal example is this:

 pub fn foo(mut x : &mut u8) -> &mut u8 {
    let y = x; // x is marked as not needing to be mutable
    let y : &mut u8 = x; // x is not marked as not needing to be mutable
    y // In either case, y doesn't have "the same type" as the return type, 
      // so an adjustment is made, meaning you can mark y as mut, and
      // no warning will be given
 }

I might be misunderstanding the borrow checking code, but it looks the adjustment means there's a second, implicit borrow, equivalent to a &mut (*y) - similarly, if y : &mut u8 = x, then y : &mut u8 = &mut (*x). Since the borrow checker recognises the mutable borrow, it tries to mark the use of mut on *y (and this marks y) in mark_loan_paths_as_mutated.

Working on a fix, but it's not clear if the issue is with what's being marked as used, or what the conditions are for it happening.

bors added a commit that referenced this issue Aug 10, 2017
Fixed mutable vars being marked used when they weren't

#### NB : bootstrapping is slow on my machine, even with `keep-stage` - fixes for occurances in the current codebase are <s>in the pipeline</s> done. This PR is being put up for review of the fix of the issue.

Fixes #43526, Fixes #30280, Fixes #25049

### Issue
Whenever the compiler detected a mutable deref being used mutably, it marked an associated value as being used mutably as well. In the case of derefencing local variables which were mutable references, this incorrectly marked the reference itself being used mutably, instead of its contents - with the consequence of making the following code emit no warnings
```
fn do_thing<T>(mut arg : &mut T) {
    ... // don't touch arg - just deref it to access the T
}
```

### Fix
Make dereferences not be counted as a mutable use, but only when they're on borrows on local variables.
#### Why not on things other than local variables?
  * Whenever you capture a variable in a closure, it gets turned into a hidden reference - when you use it in the closure, it gets dereferenced. If the closure uses the variable mutably, that is actually a mutable use of the thing being dereffed to, so it has to be counted.
  * If you deref a mutable `Box` to access the contents mutably, you are using the `Box` mutably - so it has to be counted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants