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

Procs don't allow capturing mutable variables #10617

Closed
alexcrichton opened this issue Nov 23, 2013 · 10 comments
Closed

Procs don't allow capturing mutable variables #10617

alexcrichton opened this issue Nov 23, 2013 · 10 comments

Comments

@alexcrichton
Copy link
Member

This code fails to compile

fn foo(_: proc()) {}

fn bar(_: &mut int) {}

fn main() {
    let mut a = 3;
    do foo {
        bar(&mut a);
    }
}

with

test.rs:8:12: 8:18 error: cannot borrow immutable captured outer variable in a heap closure as mutable
test.rs:8         bar(&mut a);
                      ^~~~~~
test.rs:8:17: 8:18 error: mutable variables cannot be implicitly captured
test.rs:8         bar(&mut a);
                           ^
error: aborting due to 2 previous errors
task 'rustc' failed at 'explicit failure', /Users/alex/code/rust-opt/src/libsyntax/diagnostic.rs:101
task '<main>' failed at 'explicit failure', /Users/alex/code/rust-opt/src/librustc/lib.rs:396

Since procs are one-shot, I don't see why I can't capture the value mutably, the proc gains ownership of the type.

cc @pcwalton, @nikomatsakis

@nikomatsakis
Copy link
Contributor

I agree, this error is obsolete -- the concern was code clarity. This used to be called #2152 (but I see we closed that). I think all we have to do is just remove the error in borrowck and everything else will work just fine.

@nickdesaulniers
Copy link

@nikomatsakis , see the comments in this test case. If you still believe it's ok, I'll remove that test case, and this branch of the match. Note how in the test, the shadow copied var is not mutable, which is the whole point (to be able to move mutable variables into a closure).

@nikomatsakis
Copy link
Contributor

@nickdesaulniers I think it's ok -- there is a potential hazard, if people don't realize the counter is being copied, but c'est la vie. I toyed with suggesting that procs always move or something like that but I think it's more consistent to just say that they move/copy in the usual way at the time of their creation.

Note: it'd be good to test whether you can re-assign the variable in the proc as well. I imagine that should be legal (obviously it'll modify the proc's copy).

@nickdesaulniers
Copy link

Note: it'd be good to test whether you can re-assign the variable in the proc as well. I imagine that should be legal (obviously it'll modify the proc's copy).

@nikomatsakis , it looks like the proc's copy does not have the same mutability as the closed over var. For example:

fn foo() {
  let mut i = 0;
    do task::spawn {
      i += 1; // fails here
      some_func(i);
      println!("spawned {}", i)
    }
    i += 1;
    println!("original {}", i)
}

will fail to compile complaining that: cannot assign to immutable captured outer variable in a heap closure. It will work if within the closure, you say let mut i = i; Is this something that should be fixed?

@nmsmith
Copy link

nmsmith commented Jan 17, 2014

I agree this needs to be fixed. For types that are moved/copied anyway, it makes no difference whether they are mutable or not when they are taken by a proc. It might stop newcomers from accidentally copying a variable, but I think it's over-the-top to make it an error for everyone if that is the only reason for making it forbidden. Speaking from experience, those error messages only make procs harder to understand.

And it looks like @nickdesaulniers has brought up another issue that should probably also be addressed.

@nikomatsakis
Copy link
Contributor

On Wed, Jan 15, 2014 at 11:39:58AM -0800, Nick Desaulniers wrote:

Is this something that should be fixed?

Probably. I guess it's no loss of expressiveness, as one can always
write let mut x = x inside the proc. But I'm not sure there's a good
reason to prevent mutation.

@nickdesaulniers
Copy link

ok then I think that this should be two separate issues (one for the two issues @alexcrichton 's code example shows):

  1. cannot borrow immutable captured outer variable in a heap closure as mutable
  2. mutable variables cannot be implicitly captured

@nickdesaulniers
Copy link

Not sure yet how to tackle the second part.

@nikomatsakis
Copy link
Contributor

@nickdesaulniers I'm not sure I understand your division into 2 parts. I guess you mean (1) cannot capture mutable variables in a proc and (2) captured mutable variables are not themselves mutable? I can help you with (2)

@nickdesaulniers
Copy link

@alexcrichton : r?

@bors bors closed this as completed in ea9db66 Jan 28, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2023
[`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`

this will check every local for `TerminatorKind::Drop` to ensure they can be evaluated at compile time, not sure if this is the best way to do this but MIR is confusing and it works so...

fixes rust-lang#10617

changelog: [`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants