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

Document heap allocation location guarantee #32383

Merged
merged 1 commit into from
Mar 26, 2016

Conversation

aidanhs
Copy link
Member

@aidanhs aidanhs commented Mar 20, 2016

14:25 < aidanhs> is there any guarantee that boxes will not move the value on the heap when they are moved?
14:26 <@steveklabnik> aidanhs: ... i'm not sure if it's a guarantee, but it follows, generally
14:26 <@steveklabnik> aidanhs: moves mean memcpy, so you're memcpying the structure of the box itself, which is copying the pointer
14:26 <@steveklabnik> so the pointer won't be updated
14:26 <@steveklabnik> moves cannot do complex things like move the memory around on the heap
14:26 <@kmc> aidanhs: I would say it's guaranteed
14:27 < aidanhs> steveklabnik: yeah, that's what I was thinking, it'd be pretty strange for rust to do something, but I couldn't find any docs one way or the other
14:27 <@steveklabnik> kmc: aidanhs yeah, it's like a borderline thing that we don't explicitly guanratee but i think IS guaranteed by our other guarantees
14:27 <@steveklabnik> mostly that move == memcpy
14:28 < aidanhs> kmc: steveklabnik great thanks! would a PR to the rust reference along these lines be ok?
14:28 < jmesmon> aidanhs: I believe owning_ref has some discussion of that (stable references)
14:29 <@steveklabnik> aidanhs: i would probably take that, yeah
14:29 < aidanhs> jmesmon: thanks, I'll take a look at that

https://botbot.me/mozilla/rust/2016-02-22/?msg=60657619&page=18

r? @steveklabnik

@steveklabnik
Copy link
Member

cc @rust-lang/lang, is this a guarantee we actually want to make?

@nagisa
Copy link
Member

nagisa commented Mar 21, 2016

We rely on moving Box<T> not actually moving the T in various places in libstd. One example would be implementation of Mutex<T>.

@huonw
Copy link
Member

huonw commented Mar 21, 2016

std can assume things about implementation details that external libraries can't/shouldn't, because changes to those details can be accompanied by changes to the places that rely on them.

That said, I would think that this is a guarantee we want to make, it seems to me like the only downside is that it would stop an implementation using a moving GC for Box?

(cc @rust-lang/libs too.)

@alexcrichton
Copy link
Member

@huonw huonw added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 21, 2016
@aidanhs
Copy link
Member Author

aidanhs commented Mar 21, 2016

I can see that opinion is leaning in favour, but a couple of motivating examples just for completeness:

  • https://github.com/Kimundi/owning-ref-rs/blob/beb4f21/src/lib.rs#L405 relies on Box<T> (and others) having a stable address.
  • Being able to create a function (e.g. to pass to C at a later point in time) at initialisation time of a struct which reads fields from the struct - as long as the struct stays in the same place in memory (e.g. because it's boxed), the function will be ok to call.

@pnkfelix
Copy link
Member

it seems to me like the only downside is that it would stop an implementation using a moving GC for Box?

I would say "that ship has sailed", except I don't think that such a ship was ever constructed, let alone given a port to dock at...

@nikomatsakis
Copy link
Contributor

I think we can not -- and should not -- change the fact that Box<T> has a stable address.

@alexcrichton
Copy link
Member

@bors: r+ bb43f58

The libs team discussed this during triage yesterday and the conclusion was to merge this for now. We probably want to in the long run take a more principled approach to defining these sorts of guarantees (for example doing them all at once), but this seems like it'd be clearly true in any situation so seems fine to merge.

Thanks @aidanhs!

@alexcrichton alexcrichton removed I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 24, 2016
@steveklabnik
Copy link
Member

🎊

@steveklabnik
Copy link
Member

@bors: rollup

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 26, 2016
…=alexcrichton

Document heap allocation location guarantee

```
14:25 < aidanhs> is there any guarantee that boxes will not move the value on the heap when they are moved?
14:26 <@steveklabnik> aidanhs: ... i'm not sure if it's a guarantee, but it follows, generally
14:26 <@steveklabnik> aidanhs: moves mean memcpy, so you're memcpying the structure of the box itself, which is copying the pointer
14:26 <@steveklabnik> so the pointer won't be updated
14:26 <@steveklabnik> moves cannot do complex things like move the memory around on the heap
14:26 <@kmc> aidanhs: I would say it's guaranteed
14:27 < aidanhs> steveklabnik: yeah, that's what I was thinking, it'd be pretty strange for rust to do something, but I couldn't find any docs one way or the other
14:27 <@steveklabnik> kmc: aidanhs yeah, it's like a borderline thing that we don't explicitly guanratee but i think IS guaranteed by our other guarantees
14:27 <@steveklabnik> mostly that move == memcpy
14:28 < aidanhs> kmc: steveklabnik great thanks! would a PR to the rust reference along these lines be ok?
14:28 < jmesmon> aidanhs: I believe owning_ref has some discussion of that (stable references)
14:29 <@steveklabnik> aidanhs: i would probably take that, yeah
14:29 < aidanhs> jmesmon: thanks, I'll take a look at that
```
https://botbot.me/mozilla/rust/2016-02-22/?msg=60657619&page=18

r? @steveklabnik
bors added a commit that referenced this pull request Mar 26, 2016
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 26, 2016
…=alexcrichton

Document heap allocation location guarantee

```
14:25 < aidanhs> is there any guarantee that boxes will not move the value on the heap when they are moved?
14:26 <@steveklabnik> aidanhs: ... i'm not sure if it's a guarantee, but it follows, generally
14:26 <@steveklabnik> aidanhs: moves mean memcpy, so you're memcpying the structure of the box itself, which is copying the pointer
14:26 <@steveklabnik> so the pointer won't be updated
14:26 <@steveklabnik> moves cannot do complex things like move the memory around on the heap
14:26 <@kmc> aidanhs: I would say it's guaranteed
14:27 < aidanhs> steveklabnik: yeah, that's what I was thinking, it'd be pretty strange for rust to do something, but I couldn't find any docs one way or the other
14:27 <@steveklabnik> kmc: aidanhs yeah, it's like a borderline thing that we don't explicitly guanratee but i think IS guaranteed by our other guarantees
14:27 <@steveklabnik> mostly that move == memcpy
14:28 < aidanhs> kmc: steveklabnik great thanks! would a PR to the rust reference along these lines be ok?
14:28 < jmesmon> aidanhs: I believe owning_ref has some discussion of that (stable references)
14:29 <@steveklabnik> aidanhs: i would probably take that, yeah
14:29 < aidanhs> jmesmon: thanks, I'll take a look at that
```
https://botbot.me/mozilla/rust/2016-02-22/?msg=60657619&page=18

r? @steveklabnik
bors added a commit that referenced this pull request Mar 26, 2016
Rollup of 6 pull requests

- Successful merges: #32383, #32387, #32440, #32470, #32478, #32492
- Failed merges:
@bors bors merged commit bb43f58 into rust-lang:master Mar 26, 2016
@aidanhs
Copy link
Member Author

aidanhs commented Mar 29, 2016

Thanks all!

@aidanhs aidanhs deleted the aphs-heap-move-guarantee branch March 29, 2016 15:17
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.

8 participants