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

Remove alignment-changing in-place collect #120116

Merged
merged 2 commits into from
Jan 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ const fn in_place_collectible<DEST, SRC>(
step_merge: Option<NonZeroUsize>,
step_expand: Option<NonZeroUsize>,
) -> bool {
if const { SRC::IS_ZST || DEST::IS_ZST || mem::align_of::<SRC>() < mem::align_of::<DEST>() } {
// Require matching alignments because an alignment-changing realloc is inefficient on many
// system allocators and better implementations would require the unstable Allocator trait.
if const { SRC::IS_ZST || DEST::IS_ZST || mem::align_of::<SRC>() != mem::align_of::<DEST>() } {
return false;
}

Expand All @@ -188,7 +190,8 @@ const fn in_place_collectible<DEST, SRC>(

const fn needs_realloc<SRC, DEST>(src_cap: usize, dst_cap: usize) -> bool {
if const { mem::align_of::<SRC>() != mem::align_of::<DEST>() } {
return src_cap > 0;
// FIXME: use unreachable! once that works in const
panic!("in_place_collectible() prevents this");
}

// If src type size is an integer multiple of the destination type size then
Expand Down Expand Up @@ -276,8 +279,8 @@ where
let dst_guard = InPlaceDstBufDrop { ptr: dst_buf, len, cap: dst_cap };
src.forget_allocation_drop_remaining();

// Adjust the allocation if the alignment didn't match or the source had a capacity in bytes
// that wasn't a multiple of the destination type size.
// Adjust the allocation if the source had a capacity in bytes that wasn't a multiple
// of the destination type size.
// Since the discrepancy should generally be small this should only result in some
// bookkeeping updates and no memmove.
if needs_realloc::<I::Src, T>(src_cap, dst_cap) {
Expand All @@ -290,7 +293,7 @@ where
let src_size = mem::size_of::<I::Src>().unchecked_mul(src_cap);
let old_layout = Layout::from_size_align_unchecked(src_size, src_align);

// The must be equal or smaller for in-place iteration to be possible
// The allocation must be equal or smaller for in-place iteration to be possible
// therefore the new layout must be ≤ the old one and therefore valid.
let dst_align = mem::align_of::<T>();
let dst_size = mem::size_of::<T>().unchecked_mul(dst_cap);
Expand Down
14 changes: 7 additions & 7 deletions library/alloc/src/vec/spec_from_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ use super::{IntoIter, SpecExtend, SpecFromIterNested, Vec};
/// +-+-----------+
/// |
/// v
/// +-+-------------------------------+ +---------------------+
/// |SpecFromIter +---->+SpecFromIterNested |
/// |where I: | | |where I: |
/// | Iterator (default)----------+ | | Iterator (default) |
/// | vec::IntoIter | | | TrustedLen |
/// | SourceIterMarker---fallback-+ | +---------------------+
/// +---------------------------------+
/// +-+---------------------------------+ +---------------------+
/// |SpecFromIter +---->+SpecFromIterNested |
/// |where I: | | |where I: |
/// | Iterator (default)------------+ | | Iterator (default) |
/// | vec::IntoIter | | | TrustedLen |
/// | InPlaceCollect--(fallback to)-+ | +---------------------+
/// +-----------------------------------+
/// ```
pub(super) trait SpecFromIter<T, I> {
fn from_iter(iter: I) -> Self;
Expand Down
Loading