Skip to content

change the box_free lang item to accept pointers to unsized types #37708

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 12, 2016

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Nov 11, 2016

in miri we use the box_free lang item as the destructor for Box objects, since the function's api matches that of an fn drop(&mut self) in a hypothetical impl<T: ?Sized> Drop for Box<T> exactly.

This works fine except if we insert a check in the size_of intrinsic to ensure that it is only called with sized types, since the box_free lang item calls that intrinsic.

cc @eddyb

no clue who to r? here, probably lang team?

@rust-highfive
Copy link
Contributor

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@sfackler
Copy link
Member

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned sfackler Nov 11, 2016
@eddyb
Copy link
Member

eddyb commented Nov 11, 2016

Do you think you could also change trans to take advantage of this? IIRC it does its own DST logic.
I prefer this form because it's closer to an eventual Drop impl for Box.
r=me with an issue open about changing trans otherwise.

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 11, 2016

The only places rustc generates calls to box_free are in librustc_mir and librustc_borrowck, and these don't check for sizedness, but instead are at locations where the Box is guaranteed to point to a Sized type. There's some interaction with exchange_free that I don't grok. It would probably be better to leave this until Box gets a real Drop impl. I'll open an issue as advised.

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 11, 2016

travis likes it + issue reported.

@eddyb
Copy link
Member

eddyb commented Nov 11, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 11, 2016

📌 Commit de371e5 has been approved by eddyb

@bluss
Copy link
Member

bluss commented Nov 11, 2016

Doesn't have the [breaking-change] text in the commit log, which we use to signal breaking changes in unstable APIs. (I'll continue nagging about this until we officially abandon the practice...)

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 11, 2016

added [breaking-change] to commit message

@eddyb
Copy link
Member

eddyb commented Nov 11, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 11, 2016

📌 Commit 323c20c has been approved by eddyb

eddyb added a commit to eddyb/rust that referenced this pull request Nov 11, 2016
change the `box_free` lang item to accept pointers to unsized types

in miri we use the `box_free` lang item as the destructor for `Box` objects, since the function's api matches that of an `fn drop(&mut self)` in a hypothetical `impl<T: ?Sized> Drop for Box<T>` exactly.

This works fine except if we insert a check in the `size_of` intrinsic to ensure that it is only called with sized types, since the `box_free` lang item calls that intrinsic.

cc @eddyb

no clue who to r? here, probably lang team?
eddyb added a commit to eddyb/rust that referenced this pull request Nov 12, 2016
change the `box_free` lang item to accept pointers to unsized types

in miri we use the `box_free` lang item as the destructor for `Box` objects, since the function's api matches that of an `fn drop(&mut self)` in a hypothetical `impl<T: ?Sized> Drop for Box<T>` exactly.

This works fine except if we insert a check in the `size_of` intrinsic to ensure that it is only called with sized types, since the `box_free` lang item calls that intrinsic.

cc @eddyb

no clue who to r? here, probably lang team?
bors added a commit that referenced this pull request Nov 12, 2016
@bors bors merged commit 323c20c into rust-lang:master Nov 12, 2016
@oli-obk oli-obk deleted the box_free branch June 15, 2020 15:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants