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

[NLL] mutability difference #50897

Closed
leonardo-m opened this issue May 19, 2018 · 2 comments · Fixed by #51964
Closed

[NLL] mutability difference #50897

leonardo-m opened this issue May 19, 2018 · 2 comments · Fixed by #51964
Labels
A-NLL Area: Non-lexical lifetimes (NLL)

Comments

@leonardo-m
Copy link

#![feature(generator_trait, generators)]
#![feature(nll)]

use std::ops::{Generator, GeneratorState};

fn generator_to_iterator<G>(g: G) -> impl Iterator<Item = G::Yield>
where G: Generator<Return = ()> {
    struct It<G>(G);

    impl<G: Generator<Return = ()>> Iterator for It<G> {
        type Item = G::Yield;

        fn next(&mut self) -> Option<Self::Item> {
            unsafe {
                match self.0.resume() {
                    GeneratorState::Yielded(y) => Some(y),
                    GeneratorState::Complete(()) => None,
                }
            }
        }
    }

    It(g)
}

fn foo(mut n: u32) -> impl Iterator<Item=u32> {
    generator_to_iterator(move || {
        while n != 0 {
            yield n;
            n -= 1;
        }
    })
}

fn main() {
    for d in foo(10) {
        println!("{}", d);
    }
}

Gives a warning:

warning: variable does not need to be mutable
  --> ...\test.rs:26:8
   |
26 | fn foo(mut n: u32) -> impl Iterator<Item=u32> {
   |        ----^
   |        |
   |        help: remove this `mut`
   |
   = note: #[warn(unused_mut)] on by default

If I remove the "mut" that warning goes away and the program runs correctly.

But if I comment out the feature(nll) line (without the "mut" in foo signature) it gives:

error[E0594]: cannot assign to immutable captured outer variable in an `FnOnce` closure `n`
  --> ...\test.rs:30:13
   |
30 |             n -= 1;
   |             ^^^^^^
@matthewjasper matthewjasper added the A-NLL Area: Non-lexical lifetimes (NLL) label May 19, 2018
@matthewjasper matthewjasper added WG-compiler-nll NLL-sound Working towards the "invalid code does not compile" goal labels May 30, 2018
@Arnavion
Copy link

Arnavion commented Jun 4, 2018

Here's a smaller repro reduced from code that uses futures-await's async proc macro.

#![feature(generators, nll)]

struct Config;
impl Config { fn foo(&mut self) { } }

fn foo(mut c: Config) {
    let _g = move || {
        if false {
            yield ();
        }

        c.foo();
    };
}

fn main() {
    foo(Config);
}

raises unused_mut on the c parameter. Does not happen if the closure is not a generator (does not yield) or if nll is not enabled.

@nikomatsakis
Copy link
Contributor

Tagging as NLL-deferred because #51918 has precedence

@nikomatsakis nikomatsakis added NLL-deferred and removed NLL-sound Working towards the "invalid code does not compile" goal labels Jun 29, 2018
bors added a commit that referenced this issue Jul 5, 2018
…omatsakis

[NLL] Fix various unused mut errors

Closes #51801
Closes #50897
Closes #51830
Closes #51904
cc #51918 - keeping this one open in case there are any more issues

This PR contains multiple changes. List of changes with examples of what they fix:

* Change mir generation so that the parameter variable doesn't get a name when a `ref` pattern is used as an argument
```rust
fn f(ref y: i32) {} // doesn't trigger lint
```
* Change mir generation so that by-move closure captures don't get first moved into a temporary.
```rust
let mut x = 0; // doesn't trigger lint
move || {
    x = 1;
};
```
* Treat generator upvars the same as closure upvars
```rust
let mut x = 0; // This mut is now necessary and is not linted against.
move || {
    x = 1;
    yield;
};
```

r? @nikomatsakis
Arnavion pushed a commit to Arnavion/fac-rs that referenced this issue Jul 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants