-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Unify stable and unstable sort implementations in same core module #104672
Conversation
This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`.
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
There were several unsafe blocks in the existing implementation that were not documented with a SAFETY comment.
As a nice little side-effect, I'm seeing a small binary size decrease (eg. 363 -> 361kb) with this change for sort instantiations. |
@joshtriplett friendly ping, is there something blocking review of this PR? |
r? @thomcc |
This seems to be true, so I don't have any comments really. I will say that this will be a problem if we don't perform the other sort-related changes, and it may constrain future sort implementations to share components in this way, but that's probably fine. @bors r+ |
Ah, actually, I need some more time to think about a couple parts of this, sorry for the false start. @bors r- |
Can I somehow help? This PR seems stuck again :( |
Yeah, this seems fine. @bors r+ |
…homcc Unify stable and unstable sort implementations in same core module This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`. This is in preparation of rust-lang#100856 and rust-lang#104116. It only moves code, it *doesn't* change any of the sort related logic. This unlocks the ability to share `insert_head`, `insert_tail`, `swap_if_less` `merge` and more. Tagging ``@Mark-Simulacrum`` I hope this allows progress on rust-lang#100856, by moving `merge_sort` here I hope future changes will be easier to review.
…homcc Unify stable and unstable sort implementations in same core module This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`. This is in preparation of rust-lang#100856 and rust-lang#104116. It only moves code, it *doesn't* change any of the sort related logic. This unlocks the ability to share `insert_head`, `insert_tail`, `swap_if_less` `merge` and more. Tagging ```@Mark-Simulacrum``` I hope this allows progress on rust-lang#100856, by moving `merge_sort` here I hope future changes will be easier to review.
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#104154 (Change `bindings_with_variant_name` to deny-by-default) - rust-lang#104347 (diagnostics: suggest changing `s@self::{macro}`@::macro`` for exported) - rust-lang#104672 (Unify stable and unstable sort implementations in same core module) - rust-lang#107048 (check for x version updates) - rust-lang#107061 (Implement some more new solver candidates and fix some bugs) - rust-lang#107095 (rustdoc: remove redundant CSS selector `.sidebar .current`) - rust-lang#107112 (Fix typo in opaque_types.rs) - rust-lang#107124 (fix check macro expansion) - rust-lang#107131 (rustdoc: use CSS inline layout for radio line instead of flexbox) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access
Vec
. The twoVec
used by merge sort,buf
andruns
, are modelled as custom types that implement the very limited requiredVec
interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such asinsert_head
.This is in preparation of #100856 and #104116. It only moves code, it doesn't change any of the sort related logic. This unlocks the ability to share
insert_head
,insert_tail
,swap_if_less
merge
and more.Tagging @Mark-Simulacrum I hope this allows progress on #100856, by moving
merge_sort
here I hope future changes will be easier to review.