Skip to content

Commit 9d0f8ee

Browse files
committed
Fix in-place collect not reallocating when necessary
1 parent df0295f commit 9d0f8ee

File tree

2 files changed

+34
-7
lines changed

2 files changed

+34
-7
lines changed

library/alloc/src/vec/in_place_collect.rs

+26-7
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ const fn in_place_collectible<DEST, SRC>(
168168
step_merge: Option<NonZeroUsize>,
169169
step_expand: Option<NonZeroUsize>,
170170
) -> bool {
171-
if DEST::IS_ZST || mem::align_of::<SRC>() < mem::align_of::<DEST>() {
171+
if const { SRC::IS_ZST || DEST::IS_ZST || mem::align_of::<SRC>() < mem::align_of::<DEST>() } {
172172
return false;
173173
}
174174

@@ -186,6 +186,26 @@ const fn in_place_collectible<DEST, SRC>(
186186
}
187187
}
188188

189+
const fn needs_realloc<SRC, DEST>(src_cap: usize, dst_cap: usize) -> bool {
190+
if const { mem::align_of::<SRC>() != mem::align_of::<DEST>() } {
191+
return src_cap > 0;
192+
}
193+
194+
// If src size is an integer multiple of the destination size
195+
// the resulting capacity will also always be an integer multiple without remainder
196+
if const {
197+
let src_sz = mem::size_of::<SRC>();
198+
let dest_sz = mem::size_of::<DEST>();
199+
dest_sz != 0 && src_sz % dest_sz == 0
200+
} {
201+
return false;
202+
}
203+
204+
// type layouts don't guarantee a fit, so do a runtime check to see if
205+
// the allocations happen to match
206+
return src_cap > 0 && src_cap * mem::size_of::<SRC>() != dst_cap * mem::size_of::<DEST>();
207+
}
208+
189209
/// This provides a shorthand for the source type since local type aliases aren't a thing.
190210
#[rustc_specialization_trait]
191211
trait InPlaceCollect: SourceIter<Source: AsVecIntoIter> + InPlaceIterable {
@@ -259,13 +279,10 @@ where
259279
// that wasn't a multiple of the destination type size.
260280
// Since the discrepancy should generally be small this should only result in some
261281
// bookkeeping updates and no memmove.
262-
if (const {
263-
let src_sz = mem::size_of::<I::Src>();
264-
src_sz > 0 && mem::size_of::<T>() % src_sz != 0
265-
} && src_cap * mem::size_of::<I::Src>() != dst_cap * mem::size_of::<T>())
266-
|| const { mem::align_of::<T>() != mem::align_of::<I::Src>() }
267-
{
282+
if needs_realloc::<I::Src, T>(src_cap, dst_cap) {
268283
let alloc = Global;
284+
debug_assert_ne!(src_cap, 0);
285+
debug_assert_ne!(dst_cap, 0);
269286
unsafe {
270287
// The old allocation exists, therefore it must have a valid layout.
271288
let src_align = mem::align_of::<I::Src>();
@@ -286,6 +303,8 @@ where
286303
let Ok(reallocated) = result else { handle_alloc_error(new_layout) };
287304
dst_buf = reallocated.as_ptr() as *mut T;
288305
}
306+
} else {
307+
debug_assert_eq!(src_cap * mem::size_of::<I::Src>(), dst_cap * mem::size_of::<T>());
289308
}
290309

291310
mem::forget(dst_guard);

library/alloc/tests/vec.rs

+8
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,14 @@ fn test_in_place_specialization_step_up_down() {
12131213
assert_ne!(src_bytes, sink_bytes);
12141214
assert_eq!(sink.len(), 2);
12151215

1216+
let mut src: Vec<[u8; 3]> = Vec::with_capacity(17);
1217+
src.resize( 8, [0; 3]);
1218+
let iter = src.into_iter().map(|[a, b, _]| [a, b]);
1219+
assert_in_place_trait(&iter);
1220+
let sink: Vec<[u8; 2]> = iter.collect();
1221+
assert_eq!(sink.len(), 8);
1222+
assert!(sink.capacity() <= 25);
1223+
12161224
let src = vec![[0u8; 4]; 256];
12171225
let srcptr = src.as_ptr();
12181226
let iter = src

0 commit comments

Comments
 (0)