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

rustc: Tweak the borrow on closure invocations #13686

Merged
merged 5 commits into from
Apr 23, 2014

Conversation

alexcrichton
Copy link
Member

This alters the borrow checker's requirements on invoking closures from
requiring an immutable borrow to requiring a unique immutable borrow. This means
that it is illegal to invoke a closure through a & pointer because there is no
guarantee that is not aliased. This does not mean that a closure is required to
be in a mutable location, but rather a location which can be proven to be
unique (often through a mutable pointer).

For example, the following code is unsound and is no longer allowed:

type Fn<'a> = ||:'a;                                                         

fn call(f: |Fn|) {                                                           
    f(|| {                                                                   
        f(|| {})                                                             
    });                                                                      
}                                                                            

fn main() {                                                                  
    call(|a| {                                                               
        a();                                                                 
    });                                                                      
}                                                                            

There is no replacement for this pattern. For all closures which are stored in
structures, it was previously allowed to invoke the closure through &self but
it now requires invocation through &mut self.

The standard library has a good number of violations of this new rule, but the
fixes will be separated into multiple breaking change commits.

Closes #12224

@brson
Copy link
Contributor

brson commented Apr 22, 2014

cc @nikomatsakis

@@ -676,6 +677,9 @@ impl<'a> BorrowckCtxt<'a> {
AddrOf | RefBinding | AutoRef => {
format!("cannot borrow {} as mutable", descr)
}
ClosureInvocation => {
format!("cannot reborrow {} while it's invoked", descr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a test for this error in this PR. Could you add one?

@nikomatsakis
Copy link
Contributor

r+ This looks good modulo nits.

This alters the borrow checker's requirements on invoking closures from
requiring an immutable borrow to requiring a unique immutable borrow. This means
that it is illegal to invoke a closure through a `&` pointer because there is no
guarantee that is not aliased. This does not mean that a closure is required to
be in a mutable location, but rather a location which can be proven to be
unique (often through a mutable pointer).

For example, the following code is unsound and is no longer allowed:

    type Fn<'a> = ||:'a;

    fn call(f: |Fn|) {
        f(|| {
            f(|| {})
        });
    }

    fn main() {
        call(|a| {
            a();
        });
    }

There is no replacement for this pattern. For all closures which are stored in
structures, it was previously allowed to invoke the closure through `&self` but
it now requires invocation through `&mut self`.

The standard library has a good number of violations of this new rule, but the
fixes will be separated into multiple breaking change commits.

Closes rust-lang#12224

[breaking-change]
Many iterators go through a closure when dealing with the `idx` method, which
are invalid after the previous change (closures cannot be invoked through a `&`
pointer). This commit alters the `fn idx` method on the RandomAccessIterator
to take `&mut self` rather than `&self`.

[breaking-change]
This is similar to the previous commits to allow invocation of a closure through
a `&mut self` pointer because `&self` is disallowed. One of the primary
implementors of the CharEq trait is a closure type, which would not work if the
method continued to have `&self`.

In addition to changing mutability of the `matches` method, this modifies the
following methods from &CharEq to take a type which implements CharEq by value.

* trim_chars
* trim_left_chars
* trim_right_chars

Where these methods were previously invoked via

    s.trim_chars(&'a')

it would now be invoked through

    s.trim_chars('a')

[breaking-change]
As with the previous commits, the Finally trait is primarily implemented for
closures, so the trait was modified from `&self` to `&mut self`. This will
require that any closure variable invoked with `finally` to be stored in a
mutable slot.

[breaking-change]
This fixes various issues throughout the standard distribution and tests.
bors added a commit that referenced this pull request Apr 23, 2014
This alters the borrow checker's requirements on invoking closures from
requiring an immutable borrow to requiring a unique immutable borrow. This means 
that it is illegal to invoke a closure through a `&` pointer because there is no 
guarantee that is not aliased. This does not mean that a closure is required to
be in a mutable location, but rather a location which can be proven to be
unique (often through a mutable pointer).
                                                                                 
For example, the following code is unsound and is no longer allowed:             
                                                                                 
    type Fn<'a> = ||:'a;                                                         
                                                                                 
    fn call(f: |Fn|) {                                                           
        f(|| {                                                                   
            f(|| {})                                                             
        });                                                                      
    }                                                                            
                                                                                 
    fn main() {                                                                  
        call(|a| {                                                               
            a();                                                                 
        });                                                                      
    }                                                                            
                                                                                 
There is no replacement for this pattern. For all closures which are stored in
structures, it was previously allowed to invoke the closure through `&self` but
it now requires invocation through `&mut self`.

The standard library has a good number of violations of this new rule, but the
fixes will be separated into multiple breaking change commits.
                                                                                 
Closes #12224
@bors bors closed this Apr 23, 2014
@bors bors merged commit 823c7ee into rust-lang:master Apr 23, 2014
@alexcrichton alexcrichton deleted the issue-12224 branch April 24, 2014 03:25
arcnmx pushed a commit to arcnmx/rust that referenced this pull request Dec 17, 2022
Don't show runnable code lenses in libraries outside of the workspace

Addresses rust-lang#13664. For now I'm just disabling runnable code lenses since the ones that display the number of references and implementations do work correctly with external code.

Also made a tiny TypeScript change to use the typed `sendNotification` overload.
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

Successfully merging this pull request may close these issues.

Calling a closure needs to be treated like a unique-immutable borrow
5 participants