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

Inlining of let should preserve borrowing soundness on "pure" left arguments #77104

Closed
jeapostrophe opened this issue Sep 23, 2020 · 6 comments
Closed
Labels
C-bug Category: This is a bug.

Comments

@jeapostrophe
Copy link

In the context of this code:

struct Obj {
    o: i32
}

fn make() -> Obj {
    Obj { o: 42 }
}

impl Obj {
fn touch(self: &mut Obj, amt: i32) -> i32 {
    self.o = self.o + amt;
    return self.o;
}
}

I expect this code:

fn main() {
    let mut t = make();
    let x1 = t.touch(t.touch(6));
    println!("{:?}", x1);
}

To run the same as this code:

fn main() {
    let mut t = make();
    let x0 = t.touch(6);
    let x1 = t.touch(x0);
    println!("{:?}", x1);
}

Because the first is the inlined version of the second.

Instead, I get this error:

24 |     let x1 = t.touch(t.touch(6));                     
   |              - ----- ^ second mutable borrow occurs here                                                         
   |              | |                                      
   |              | first borrow later used by call        
   |              first mutable borrow occurs here 

I expect that programming languages should be invariant to the inlining of variables that occur only a single time and preserve the order of effects.

I asked about this on the forum and they told me that the expansion of my "inlined" version is actually:

let self0 = &mut t;
let x0 = {
    let self1  = &mut t;
    let x1 = 6;
    Obj::touch(self1, x1)
};
let x1 = Obj::touch(self0, x0);

Because you need to evaluate t, then you need to evaluate t.touch(6), then you can call touch again. The issue being that evaluating t as an argument to Obj::touch has an effect of borrowing t. I think that the Rust compiler should have a case for situations like this to allow my original program. I don't know anything about the internals of the Rust compiler, but perhaps the special situation is when earlier arguments are pure, except for borrowing, then they can be evaluated after. In other words, the expansion could be

let x0 = {
    let self1  = &mut t;
    let x1 = 6;
    Obj::touch(self1, x1)
};
let self0 = &mut t;  // <--- pure but for borrowing
let x1 = Obj::touch(self0, x0);

Thank you!

Jay

Meta

rustc --version --verbose:

rustc 1.46.0 (04488afe3 2020-08-24)
binary: rustc
commit-hash: 04488afe34512aa4c33566eb16d8c912a3ae04f9
commit-date: 2020-08-24
host: x86_64-apple-darwin
release: 1.46.0
LLVM version: 10.0
@jeapostrophe jeapostrophe added the C-bug Category: This is a bug. label Sep 23, 2020
@jonas-schievink
Copy link
Contributor

This is currently behaving as expected, and changing this would have to go through the RFC process, so closing in favor of that. You might also be interested in two-phase borrows, tracked in #49434.

@jeapostrophe
Copy link
Author

This is a very unfriendly message to someone trying to help Rust and understand it. I feel like a bureaucrat just stomped on me and said I filled a form in the wrong place at the wrong time so my house is now going to be bulldozed.

Are you really saying that my problem is going to be completely ignored because I don't know how to go through the complex Rust RFC process? I'm just a casual user of Rust with enough knowledge to see a problem and a potential solution.

@CryZe
Copy link
Contributor

CryZe commented Sep 23, 2020

What Jonas seemingly wanted to basically say is that there is already work ongoing towards the problem that you encountered. The solution is to extend the borrow checker to support two-phase borrows, i.e. a borrow happens earlier, but isn't used until later. This is roughly the solution that you propose as far as I can tell. Two-phase borrows were intended to be part of Rust 2018, but got disabled again. If I recall correctly it's unclear how to integrate it into the stacked borrows memory model, which defines which pointer interactions are defined behavior and which are not. The order of borrowing is really important in that case. So not only does it affect the borrow checker but also potential aliasing semantics that allow certain optimizations and cause certain undefined behavior. Don't quote me on all that though, that's just roughly what I recall. Nonetheless, all that work is tracked in the linked issue, which is why your issue got closed as basically a duplicate. Feel free to contribute to two-phase borrows though.

@jonas-schievink
Copy link
Contributor

@jeapostrophe Sorry, I didn't mean to dismiss your issue. Let me clarify a bit:

We use this issue tracker to track bugs and improvements in the compiler and standard library. Since the behavior you have reported here is working as intended (as explained in the forum thread), changing it would be a change to the language specification instead, and our policy for those is that they need to go through an RFC.

The two-phase borrows RFC already tries to address similar issues, which is why I have linked it here. However, my understanding is that it (and even the more relaxed form of it) does not aim to make the code you posted compile.

Please feel free to contact the moderation team at rust-mods@rust-lang.org if you feel like my behavior was not in line with the Code of Conduct.

@jeapostrophe
Copy link
Author

Keep track of improvements and problems with Rust however you want. I'm not going to write an RFC, because I have no intention of being a Rust designer; I just want to have a useful language with safe memory, not some multi-layered bureaucracy.

Good luck if you want to continue with this attitude where reports that correct programs are being mishandled by Rust need to be turned into language design documents before being paid attention to.

@mbrubeck
Copy link
Contributor

In any case, thank you for the feedback here, and I'll try to make sure it's tracked somewhere useful.

For future reference, if you choose, you can file issues on the rust-lang/rfcs repo without actually writing an RFC, for proposals like this. (Someone will eventually need to do the work of specifying and implementing the change, but it doesn't need to be the same person who opens the issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants