Skip to content

Add new ThinBox type for 1 stack pointer wide heap allocated trait objects#90066

Merged
bors merged 1 commit intorust-lang:masterfrom
yaahc:thinbox
Apr 9, 2022
Merged

Add new ThinBox type for 1 stack pointer wide heap allocated trait objects#90066
bors merged 1 commit intorust-lang:masterfrom
yaahc:thinbox

Conversation

@yaahc
Copy link
Member

@yaahc yaahc commented Oct 19, 2021 •
edited
Loading

@rust-highfive
Copy link
Contributor

rust-highfive commented Oct 19, 2021

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 19, 2021
@yaahc
Copy link
Member Author

yaahc commented Oct 19, 2021

r? @m-ou-se

cc @eddyb

@rust-highfive rust-highfive assigned m-ou-se and unassigned kennytm Oct 19, 2021
@m-ou-se m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 19, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@camelid

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@yaahc

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@yaahc

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented Oct 20, 2021

☔ The latest upstream changes (presumably #90067) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 20, 2021
matthieu-m
matthieu-m reviewed Oct 20, 2021
View reviewed changes
@matthieu-m

This comment has been minimized.

Sign in to view
@yaahc

This comment has been minimized.

Sign in to view
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 10, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@yaahc yaahc force-pushed the thinbox branch 3 times, most recently from 7b4d93b to e08f43c Compare January 11, 2022 20:31
@yaahc
Copy link
Member Author

yaahc commented Feb 18, 2022

r? rust-lang/libs

@yaahc yaahc added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 18, 2022
@yaahc
Copy link
Member Author

yaahc commented Feb 18, 2022

second attempt!

r? rust-lang/libs

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2022
@bors
Copy link
Collaborator

bors commented Apr 1, 2022

☔ The latest upstream changes (presumably #95552) made this pull request unmergeable. Please resolve the merge conflicts.

joshtriplett
joshtriplett reviewed Apr 7, 2022
View reviewed changes
joshtriplett
joshtriplett reviewed Apr 7, 2022
View reviewed changes
joshtriplett
joshtriplett reviewed Apr 7, 2022
View reviewed changes
joshtriplett
joshtriplett reviewed Apr 7, 2022
View reviewed changes
library/alloc/src/lib.rs Outdated
Copy link
Member

@joshtriplett joshtriplett Apr 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@joshtriplett
Copy link
Member

joshtriplett commented Apr 7, 2022

Posted a few comments, and un-resolved one other comment that didn't get addressed.

LGTM otherwise; r=me with those addressed.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@yaahc yaahc mentioned this pull request Apr 8, 2022
Merged
@bors
Copy link
Collaborator

bors commented Apr 8, 2022

☔ The latest upstream changes (presumably #95798) made this pull request unmergeable. Please resolve the merge conflicts.

@yaahc @joshtriplett
Add ThinBox type for 1 stack pointer sized heap allocated trait objects
a87a0d0 
Relevant commit messages from squashed history in order:

Add initial version of ThinBox

update test to actually capture failure

swap to middle ptr impl based on matthieu-m's design

Fix stack overflow in debug impl

The previous version would take a `&ThinBox<T>` and deref it once, which
resulted in a no-op and the same type, which it would then print causing
an endless recursion. I've switched to calling `deref` by name to let
method resolution handle deref the correct number of times.

I've also updated the Drop impl for good measure since it seemed like it
could be falling prey to the same bug, and I'll be adding some tests to
verify that the drop is happening correctly.

add test to verify drop is behaving

add doc examples and remove unnecessary Pointee bounds

ThinBox: use NonNull

ThinBox: tests for size

Apply suggestions from code review

Co-authored-by: Alphyr <47725341+a1phyr@users.noreply.github.com>

use handle_alloc_error and fix drop signature

update niche and size tests

add cfg for allocating APIs

check null before calculating offset

add test for zst and trial usage

prevent optimizer induced ub in drop and cleanup metadata gathering

account for arbitrary size and alignment metadata

Thank you nika and thomcc!

Update library/alloc/src/boxed/thin.rs

Co-authored-by: Josh Triplett <josh@joshtriplett.org>

Update library/alloc/src/boxed/thin.rs

Co-authored-by: Josh Triplett <josh@joshtriplett.org>
@yaahc
Copy link
Member Author

yaahc commented Apr 9, 2022

@bors r=joshtriplett

@bors
Copy link
Collaborator

bors commented Apr 9, 2022

📌 Commit a87a0d0 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2022
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 9, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 9, 2022
@bors
Auto merge of rust-lang#95835 - Dylan-DPC:rollup-l5mf2ad, r=Dylan-DPC
4bb685e 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#90066 (Add new ThinBox type for 1 stack pointer wide heap allocated trait objects)
 - rust-lang#95374 (assert_uninit_valid: ensure we detect at least arrays of uninhabited types)
 - rust-lang#95599 (Strict provenance lints)
 - rust-lang#95751 (Don't report numeric inference ambiguity when we have previous errors)
 - rust-lang#95764 ([macro_metavar_expr] Add tests to ensure the feature requirement)
 - rust-lang#95787 (reword panic vs result section to remove recoverable vs unrecoverable framing)
 - rust-lang#95797 (Remove explicit delimiter token trees from `Delimited`.)
 - rust-lang#95804 (rustdoc: Fix empty doc comment with backline ICE)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ee8cea8 into rust-lang:master Apr 9, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cuviper cuviper cuviper left review comments

@joshtriplett joshtriplett joshtriplett left review comments

@RalfJung RalfJung RalfJung left review comments

@thomcc thomcc thomcc left review comments

+5 more reviewers

@ben0x539 ben0x539 ben0x539 left review comments

@eddyb eddyb eddyb left review comments

@matthieu-m matthieu-m matthieu-m left review comments

@compiler-errors compiler-errors compiler-errors left review comments

@a1phyr a1phyr a1phyr left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@joshtriplett joshtriplett

Labels

S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.62.0

Development

Successfully merging this pull request may close these issues.