Skip to content

by-ref closure capture semantics are a backwards compatibility hazard #12831

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
thestinger opened this issue Mar 11, 2014 · 2 comments
Closed

by-ref closure capture semantics are a backwards compatibility hazard #12831

thestinger opened this issue Mar 11, 2014 · 2 comments
Milestone
1.0

Comments

@thestinger
Copy link
Contributor

Unboxed closures (#8622) would capture by-value, and there would be no need to add syntax for a by-reference capture. Rust's references are first-class values, unlike C++, so it's trivial to capture a reference by-value.

This would create an unboxed closure (f) with the same size as b:

let b = 5;
let f = |a| a + b;

A by-reference capture would need to be explicit:

let b = &mut 5;
let f = |a| *b = a;

I think we should consider changing the current rules, to permit dropping in unboxed closures as a backwards compatible enhancement in the future.
@pnkfelix
Copy link
Member

We do need to address the problem described here; it is not yet clear whether the suggested strategy is what we will follow, but we need to do something.

Assigning 1.0, P-backcompat-lang.

@pnkfelix pnkfelix added this to the 1.0 milestone Mar 13, 2014
@alexcrichton alexcrichton mentioned this issue May 16, 2014
Closed
@pcwalton pcwalton mentioned this issue May 27, 2014
Closed
pcwalton added a commit to pcwalton/rust that referenced this issue May 29, 2014
@pcwalton
librustc: Copy or move upvars by value.
a43ff2d 
Closes rust-lang#12831.

This breaks many closures (about 10%) that look like this:

    let mut sum = 0;
    f.map(|x| sum += x);

Instead, rewrite them like this:

    let mut sum = 0;
    {
        let sum_ptr = &mut sum;
        f.map(|x| *sum_ptr += x);
    }

Or, ideally, switch to RAII- or iterator-like patterns.

[breaking-change]
@pcwalton pcwalton mentioned this issue May 29, 2014
Closed
pnkfelix pushed a commit to pnkfelix/rust that referenced this issue Jun 3, 2014
@pcwalton @pnkfelix
librustc: Copy or move upvars by value.
7aa3ba1 
Closes rust-lang#12831.

This breaks many closures (about 10%) that look like this:

    let mut sum = 0;
    f.map(|x| sum += x);

Instead, rewrite them like this:

    let mut sum = 0;
    {
        let sum_ptr = &mut sum;
        f.map(|x| *sum_ptr += x);
    }

Or, ideally, switch to RAII- or iterator-like patterns.

[breaking-change]
pnkfelix pushed a commit to pnkfelix/rust that referenced this issue Jun 3, 2014
@pcwalton @pnkfelix
librustc: Copy or move upvars by value [breaking-change].
816a4d3 
Closes rust-lang#12831.

This breaks many closures (about 10%) that look like this:

    let mut sum = 0;
    f.map(|x| sum += x);

Instead, rewrite them like this:

    let mut sum = 0;
    {
        let sum_ptr = &mut sum;
        f.map(|x| *sum_ptr += x);
    }

Or, ideally, switch to RAII- or iterator-like patterns.

[breaking-change]
@pnkfelix pnkfelix mentioned this issue Jun 3, 2014
Closed
@pcwalton pcwalton mentioned this issue Jul 23, 2014
Merged
bors added a commit that referenced this issue Aug 14, 2014
@bors
auto merge of #15929 : pcwalton/rust/by-ref-closures, r=alexcrichton
9d45d63 
by-reference upvars.

This partially implements RFC 38. A snapshot will be needed to turn this
on, because stage0 cannot yet parse the keyword.

Part of #12831.

r? @alexcrichton
pcwalton added a commit to pcwalton/rust that referenced this issue Aug 19, 2014
@pcwalton
librustc: Require the ref keyword to get by-reference closure
3c08ed8 
captures.

Because captured variables are now captured by value by default, this
breaks code like:

    let mut a = 10;
    [ 1i, 2, 3 ].iter().map(|x| a += *x);

Change this code to:

    let mut a = 10;
    [ 1i, 2, 3 ].iter().map(ref |x| a += *x);

As a simple change, you may wish to uniformly add the `ref` keyword to
all old boxed closures. This will guarantee that the semantics remain
the same.

Issue rust-lang#12831.

[breaking-change]
@pcwalton pcwalton mentioned this issue Aug 19, 2014
Closed
@pcwalton pcwalton mentioned this issue Sep 24, 2014
Merged
pcwalton added a commit to pcwalton/rust that referenced this issue Sep 25, 2014
@pcwalton
librustc: Implement the unboxed closure trait hierarchy.
5de1e01 
This makes `FnMut` require `FnOnce` and `Fn` require `FnMut`. Therefore,
this change breaks code that implements the `FnMut` and/or `Fn` traits
directly, without also implementing their dependencies. A simple
forwarding implementation that defines `FnOnce` in terms of `FnMut`
and/or `Fn` in terms of `FnMut` will suffice.

This does not affect code that simply uses the `|&:|`/`|&mut:|`/`|:|`
unboxed closure construction notation.

Part of RFC rust-lang#44; needed to implement RFC rust-lang#63.

Part of issue rust-lang#12831.

[breaking-change]
@pcwalton pcwalton mentioned this issue Sep 25, 2014
Closed
pcwalton added a commit to pcwalton/rust that referenced this issue Sep 26, 2014
@pcwalton
librustc: Eliminate the ref syntax for unboxed closure capture clauses
2257e23 
in favor of `move`.

This breaks code that used `move` as an identifier, because it is now a
keyword. Change such identifiers to not use the keyword `move`.
Additionally, this breaks code that was counting on by-value or
by-reference capture semantics for unboxed closures (behind the feature
gate). Change `ref |:|` to `|:|` and `|:|` to `move |:|`.

Part of RFC rust-lang#63; part of issue rust-lang#12831.

[breaking-change]
@aturon aturon mentioned this issue Oct 16, 2014
Closed
47 tasks
@nikomatsakis
Copy link
Contributor

This is done.

fasterthanlime pushed a commit to fasterthanlime/rust that referenced this issue Jul 22, 2022
@bors
Auto merge of rust-lang#12831 - fasterthanlime:proc-macro-test-in-tmp…
7dc36ee 
…-dir, r=fasterthanlime

Build proc-macro-test-impl out-of-tree

Building it in-place fails in rust CI because the source directory is read-only. This changes `proc-macro-test`'s build script to first
copy `imp` under `OUT_DIR` (which is read-write).

It also prints stdout/stderr for the nested cargo invocation, should it fail. (I've seen failures in rust CI that I couldn't explain, and
when they take 25 minutes to reproduce, you want to have that info)

This change is tracked in:

  * rust-lang/rust-analyzer#12818

Maintainer impact: none.
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
3 participants
@nikomatsakis @pnkfelix @thestinger