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

Nested method calls with `&mut` receivers result in borrowck errors #6268

Closed
nikomatsakis opened this Issue May 6, 2013 · 10 comments

Comments

Projects
None yet
7 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 6, 2013

This is challenging to resolve. I thought I could fit into the new borrow checker, but it is worth addressing separately since there are a number of subtle issues. What this really amounts to is that you need to support borrows where the lifetime of the borrow pointer does not encompass the borrow itself. To handle this safely, we wind up with two loans, the primary one and then a secondary, aliasing loan that tracks the fact that a pointer has been created, even if the memory it points at cannot be dereferenced yet.

Part of #5074

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented May 6, 2013

There are various FIXMEs in the code with commented out bits of code related to this issue.

@Dretch

This comment has been minimized.

Copy link
Contributor

Dretch commented May 28, 2013

Here is a test case (at least I think it relates to this issue):

struct MyStruct {x: int}

impl MyStruct {

    fn takes_mut_self(&mut self, _x:int) {}

    fn takes_self(&self) -> int { 42 }

    fn also_takes_mut_self(&mut self) {
        self.takes_mut_self(self.takes_self());
    }

}

fn main() {}

The error rustc produces is:

mut-self-issues.rs:12:28: 12:33 error: cannot borrow `*self` as immutable because it is also borrowed as mutable
mut-self-issues.rs:12         self.takes_mut_self(self.takes_self());
                                                  ^~~~~
mut-self-issues.rs:12:8: 12:13 note: second borrow of `*self` occurs here
mut-self-issues.rs:12         self.takes_mut_self(self.takes_self());
                              ^~~~~
error: aborting due to previous error
@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jul 3, 2013

Nominated for feature complete (not backwards compatible since it generates strictly more errors than it should)

bors added a commit that referenced this issue Jul 22, 2013

auto merge of #7848 : michaelwoerister/rust/trans_cleanup_1, r=jdm
The following types are renamed:
```rust
block_ => Block
block => @mut Block

fn_ctx_ => FunctionContext
fn_ctx => @mut FunctionContext

scope_info => ScopeInfo
```
I also tried to convert instances of `@mut` to `&mut` or `&` but a lot of them are blocked by issue #6268, so I left it for some time later.

bors added a commit that referenced this issue Jul 23, 2013

auto merge of #7848 : michaelwoerister/rust/trans_cleanup_1, r=jdm
The following types are renamed:
```rust
block_ => Block
block => @mut Block

fn_ctx_ => FunctionContext
fn_ctx => @mut FunctionContext

scope_info => ScopeInfo
```
I also tried to convert instances of `@mut` to `&mut` or `&` but a lot of them are blocked by issue #6268, so I left it for some time later.
@catamorphism

This comment has been minimized.

Copy link
Contributor

catamorphism commented Aug 29, 2013

accepted for feature-complete

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 20, 2014

This can be added backwards-compatibly. Its a nice enhancement but not known to be a major problem to work-around.

P-low, not 1.0.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jun 19, 2014

By the way, the reason that this does not type check under our current model is provided in the internal docs for the type checker; see: typeck/infer/region_inference/doc.rs lines 171 through 262.

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Mar 16, 2015

Triage bump; still an issue.

@lilyball

This comment has been minimized.

Copy link
Contributor

lilyball commented Mar 27, 2015

cc me, I still hit this with surprising regularity.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Apr 16, 2015

Closing in favor of rust-lang/rfcs#811

steveklabnik added a commit to steveklabnik/rust that referenced this issue Jun 12, 2015

Remove a bunch of ignored tests
Most of these are old, but some specific messages for specific tests:

* trait-contravariant-self.rs: failed due to a soundess hole:
  rust-lang@05e3248

* process-detatch: rust-lang@15966c3
  says "this test is being ignored until signals are implemented" That's
  not happening for a long time, and when it is, we'll write tests for
  it.

* deep-vector{,2}.rs: "too big for our poor macro infrastructure", and has
  been ignored over a year.

* borrowck-nested-calls.rs's FIXME rust-lang#6268 was closed in favor of
  rust-lang/rfcs#811

* issue-15167.rs works properly now
* issue-9737.rs works properly now
* match-var-hygiene.rs works properly now

steveklabnik added a commit to steveklabnik/rust that referenced this issue Jun 12, 2015

ignore-test cleanup
Most of these are old, but some specific messages for specific tests:

* trait-contravariant-self.rs: failed due to a soundess hole:
  rust-lang@05e3248

* process-detatch: rust-lang@15966c3
  says "this test is being ignored until signals are implemented" That's
  not happening for a long time, and when it is, we'll write tests for
  it.

* deep-vector{,2}.rs: "too big for our poor macro infrastructure", and has
  been ignored over a year.

* borrowck-nested-calls.rs's FIXME rust-lang#6268 was closed in favor of
  rust-lang/rfcs#811

* issue-15167.rs works properly now
* issue-9737.rs works properly now
* match-var-hygiene.rs works properly now

Addresses a chunk of rust-lang#3965
@dcsommer

This comment has been minimized.

Copy link

dcsommer commented Jul 21, 2015

Seems to be a blocker for some async IO patterns needed for lowest-latency processing. cc me.

thepowersgang added a commit to thepowersgang/rust that referenced this issue Jul 25, 2015

ignore-test cleanup
Most of these are old, but some specific messages for specific tests:

* trait-contravariant-self.rs: failed due to a soundess hole:
  rust-lang@05e3248

* process-detatch: rust-lang@15966c3
  says "this test is being ignored until signals are implemented" That's
  not happening for a long time, and when it is, we'll write tests for
  it.

* deep-vector{,2}.rs: "too big for our poor macro infrastructure", and has
  been ignored over a year.

* borrowck-nested-calls.rs's FIXME rust-lang#6268 was closed in favor of
  rust-lang/rfcs#811

* issue-15167.rs works properly now
* issue-9737.rs works properly now
* match-var-hygiene.rs works properly now

Addresses a chunk of rust-lang#3965

nivkner added a commit to nivkner/rust that referenced this issue Oct 7, 2017

address more FIXME whose associated issues were marked as closed
update FIXME(rust-lang#6298) to point to open issue 15020
update FIXME(rust-lang#6268) to point to RFC 811
update FIXME(rust-lang#10520) to point to RFC 1751
remove FIXME for emscripten issue 4563 and include target in `test_estimate_scaling_factor`
remove FIXME(rust-lang#18207) since node_id isn't used for `ref` pattern analysis
remove FIXME(rust-lang#6308) since DST was implemented in rust-lang#12938
remove FIXME(rust-lang#2658) since it was decided to not reorganize module
remove FIXME(rust-lang#20590) since it was decided to stay conservative with projection types
remove FIXME(rust-lang#20297) since it was decided that solving the issue is unnecessary
remove FIXME(rust-lang#27086) since closures do correspond to structs now
remove FIXME(rust-lang#13846) and enable `function_sections` for windows
remove mention of rust-lang#22079 in FIXME(rust-lang#22079) since this is a general FIXME
remove FIXME(rust-lang#5074) since the restriction on borrow were lifted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.