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

MIR-based borrow checker complains about rogue elves ("mut elf") when used with arbitrary self types #51578

Closed
shepmaster opened this issue Jun 15, 2018 · 4 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-diagnostics Working towards the "diagnostic parity" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@shepmaster
Copy link
Member

1.28.0-nightly (2018-06-13 5205ae8)

#![feature(nll, arbitrary_self_types)]

use std::rc::Rc;

struct Combine(u8);

impl Combine {
    fn poll(self: Rc<Self>) {
        drop(self);
        self.0 = 42;
    }
}

fn main() {}
error[E0594]: cannot assign to immutable item
  --> src/main.rs:10:9
   |
10 |         self.0 = 42;
   |         ----^^^^^^^
   |         |
   |         cannot assign through `&`-reference
   |         help: consider changing this to be a mutable reference: `&mut elf`

Unsurprisingly, &mut elf does not help the code compile.

@shepmaster shepmaster added A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal labels Jun 15, 2018
@kennytm
Copy link
Member

kennytm commented Jun 18, 2018

You don't need arbitrary self types.

#![feature(nll)]

use std::rc::Rc;

struct Combine(u8);

fn poll(welf: Rc<Combine>) {
    drop(welf);
    welf.0 = 42;
}

fn main() {}

@pnkfelix
Copy link
Member

@pnkfelix wants to double check how this behaves in the context of PR #51275

@pnkfelix
Copy link
Member

(yes, #51275 appears to fix this; it does not issue the erroneous suggestion in these cases.)

@nikomatsakis
Copy link
Contributor

assigning @pnkfelix as they think they have a fix in #51275

bors added a commit that referenced this issue Jun 19, 2018
…ermissions, r=nikomatsakis

NLL diagnostics: revise `fn check_access_permissions`

NLL: revise `fn check_access_permissions` so that its (still branchy) shares more code paths between the different cases, and also provide more diagnostics in more cases (though the added diagnostics still do not always meet the quality bar established by AST-borrowck)

----

Transcribing "checklist" suggested by Niko, except I am rendering it as a table to make it clear that I do not regard every item in the list to be a "must have" for landing this PR.

goal | does this PR do it?
-----|------------------------------
no suggestions for `ref mut` |  yes
suggestions for direct local assignment (`{ let x = 3; x = 4; }`) | yes (see commits at end)
suggestions for direct field assignment (`{ let x = (3, 4); x.0 = 5; }` | yes (see commits at end)
suggestions for upvars (`let x = 3; let c = \|\| { &mut x; }`) | yes

Note that I added support for a couple of rows via changes that are not strictly part of `fn check_access_permissions`. If desired I can remove those commits from this PR and leave them for a later PR.

Fix #51031
Fix #51032
(bug #51191 needs a little more investigation before closing.)
Fix #51578
@jkordish jkordish added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 19, 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) C-enhancement Category: An issue proposing an enhancement or a PR with one. NLL-diagnostics Working towards the "diagnostic parity" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants