Skip to content

Commit ce0f7ba

Browse files
committed
Auto merge of #91512 - scottmcm:array-intoiter-advance, r=Mark-Simulacrum
Override `Iterator::advance(_back)_by` for `array::IntoIter` Because I happened to notice that `nth` is currently getting codegen'd as a loop even for `Copy` types: <https://rust.godbolt.org/z/fPqv7Gvs7> <details> <summary>LLVM before and after</summary> Rust: ```rust #[no_mangle] pub fn array_intoiter_nth(it: &mut std::array::IntoIter<i32, 100>, n: usize) -> Option<i32> { it.nth(n) } ``` Current nightly: ```llvmir define { i32, i32 } `@array_intoiter_nth(%"core::array::iter::IntoIter<i32,` 100_usize>"* noalias nocapture align 8 dereferenceable(416) %it, i64 %n) unnamed_addr #0 personality i32 (i32, i32, i64, %"unwind::libunwind::_Unwind_Exception"*, %"unwind::libunwind::_Unwind_Context"*)* `@rust_eh_personality` !dbg !6 { start: %_3.i.i.i4.i.i = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 0, i32 0 %_4.i.i.i5.i.i = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 0, i32 1 %_4.i.i.i.i.i.i = load i64, i64* %_4.i.i.i5.i.i, align 8, !alias.scope !10 %.not.i.i = icmp eq i64 %n, 0, !dbg !15 %_3.i.i.i.i.pre.i = load i64, i64* %_3.i.i.i4.i.i, align 8, !dbg !40, !alias.scope !41 br i1 %.not.i.i, label %bb4.i, label %bb4.preheader.i.i, !dbg !42 bb4.preheader.i.i: ; preds = %start %umax.i = tail call i64 `@llvm.umax.i64(i64` %_3.i.i.i.i.pre.i, i64 %_4.i.i.i.i.i.i) #3, !dbg !43 %0 = sub i64 %umax.i, %_3.i.i.i.i.pre.i, !dbg !43 br label %bb4.i.i, !dbg !43 bb4.i.i: ; preds = %bb3.i.i.i.i, %bb4.preheader.i.i %_3.i.i.i.i.i.i = phi i64 [ %2, %bb3.i.i.i.i ], [ %_3.i.i.i.i.pre.i, %bb4.preheader.i.i ], !dbg !52 %iter.sroa.0.016.i.i = phi i64 [ %1, %bb3.i.i.i.i ], [ 0, %bb4.preheader.i.i ] %1 = add nuw i64 %iter.sroa.0.016.i.i, 1, !dbg !54 %exitcond.not.i = icmp eq i64 %iter.sroa.0.016.i.i, %0, !dbg !52 br i1 %exitcond.not.i, label %core::iter::traits::iterator::Iterator::nth.exit, label %bb3.i.i.i.i, !dbg !43 bb3.i.i.i.i: ; preds = %bb4.i.i %2 = add nuw i64 %_3.i.i.i.i.i.i, 1, !dbg !63 store i64 %2, i64* %_3.i.i.i4.i.i, align 8, !dbg !66, !alias.scope !75 %exitcond.not.i.i = icmp eq i64 %1, %n, !dbg !15 br i1 %exitcond.not.i.i, label %bb4.i, label %bb4.i.i, !dbg !42 bb4.i: ; preds = %bb3.i.i.i.i, %start %_3.i.i.i.i.i = phi i64 [ %_3.i.i.i.i.pre.i, %start ], [ %2, %bb3.i.i.i.i ], !dbg !84 %3 = icmp ult i64 %_3.i.i.i.i.i, %_4.i.i.i.i.i.i, !dbg !84 br i1 %3, label %bb3.i.i.i, label %core::iter::traits::iterator::Iterator::nth.exit, !dbg !89 bb3.i.i.i: ; preds = %bb4.i %4 = add nuw i64 %_3.i.i.i.i.i, 1, !dbg !90 store i64 %4, i64* %_3.i.i.i4.i.i, align 8, !dbg !93, !alias.scope !96 %5 = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 1, i64 %_3.i.i.i.i.i, !dbg !105 %6 = load i32, i32* %5, align 4, !dbg !131, !alias.scope !141, !noalias !144 br label %core::iter::traits::iterator::Iterator::nth.exit, !dbg !149 core::iter::traits::iterator::Iterator::nth.exit: ; preds = %bb4.i.i, %bb4.i, %bb3.i.i.i %.sroa.3.0.i = phi i32 [ %6, %bb3.i.i.i ], [ undef, %bb4.i ], [ undef, %bb4.i.i ], !dbg !40 %.sroa.0.0.i = phi i32 [ 1, %bb3.i.i.i ], [ 0, %bb4.i ], [ 0, %bb4.i.i ], !dbg !40 %7 = insertvalue { i32, i32 } undef, i32 %.sroa.0.0.i, 0, !dbg !150 %8 = insertvalue { i32, i32 } %7, i32 %.sroa.3.0.i, 1, !dbg !150 ret { i32, i32 } %8, !dbg !151 } ``` With this PR: ```llvmir define { i32, i32 } `@array_intoiter_nth(%"core::array::iter::IntoIter<i32,` 100_usize>"* noalias nocapture align 8 dereferenceable(416) %it, i64 %n) unnamed_addr #0 personality i32 (...)* `@__CxxFrameHandler3` { start: %0 = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 0, i32 1 %_2.i.i.i.i = load i64, i64* %0, align 8, !alias.scope !6, !noalias !13 %1 = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 0, i32 0 %_3.i.i.i.i = load i64, i64* %1, align 8, !alias.scope !16 %2 = sub i64 %_2.i.i.i.i, %_3.i.i.i.i %3 = icmp ult i64 %2, %n %.0.sroa.speculated.i.i.i.i.i = select i1 %3, i64 %2, i64 %n %_10.i.i = add i64 %.0.sroa.speculated.i.i.i.i.i, %_3.i.i.i.i store i64 %_10.i.i, i64* %1, align 8, !alias.scope !16 %.not.i = xor i1 %3, true %4 = icmp ult i64 %_10.i.i, %_2.i.i.i.i %or.cond.i = select i1 %.not.i, i1 %4, i1 false br i1 %or.cond.i, label %bb3.i.i.i, label %_ZN4core4iter6traits8iterator8Iterator3nth17hcbc727011e9e2a3bE.exit bb3.i.i.i: ; preds = %start %5 = add nuw i64 %_10.i.i, 1 store i64 %5, i64* %1, align 8, !alias.scope !17 %6 = getelementptr inbounds %"core::array::iter::IntoIter<i32, 100_usize>", %"core::array::iter::IntoIter<i32, 100_usize>"* %it, i64 0, i32 1, i64 %_10.i.i %7 = load i32, i32* %6, align 4, !alias.scope !26, !noalias !29 br label %_ZN4core4iter6traits8iterator8Iterator3nth17hcbc727011e9e2a3bE.exit _ZN4core4iter6traits8iterator8Iterator3nth17hcbc727011e9e2a3bE.exit: ; preds = %start, %bb3.i.i.i %.sroa.3.0.i = phi i32 [ undef, %start ], [ %7, %bb3.i.i.i ] %.sroa.0.0.i = phi i32 [ 0, %start ], [ 1, %bb3.i.i.i ] %8 = insertvalue { i32, i32 } undef, i32 %.sroa.0.0.i, 0 %9 = insertvalue { i32, i32 } %8, i32 %.sroa.3.0.i, 1 ret { i32, i32 } %9 } ``` </details>
2 parents abba5ed + eb846db commit ce0f7ba

File tree

2 files changed

+149
-1
lines changed

2 files changed

+149
-1
lines changed

library/core/src/array/iter.rs

+43-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Defines the `IntoIter` owned iterator for arrays.
22
33
use crate::{
4-
fmt,
4+
cmp, fmt,
55
iter::{self, ExactSizeIterator, FusedIterator, TrustedLen},
66
mem::{self, MaybeUninit},
77
ops::Range,
@@ -281,6 +281,27 @@ impl<T, const N: usize> Iterator for IntoIter<T, N> {
281281
fn last(mut self) -> Option<Self::Item> {
282282
self.next_back()
283283
}
284+
285+
fn advance_by(&mut self, n: usize) -> Result<(), usize> {
286+
let len = self.len();
287+
288+
// The number of elements to drop. Always in-bounds by construction.
289+
let delta = cmp::min(n, len);
290+
291+
let range_to_drop = self.alive.start..(self.alive.start + delta);
292+
293+
// Moving the start marks them as conceptually "dropped", so if anything
294+
// goes bad then our drop impl won't double-free them.
295+
self.alive.start += delta;
296+
297+
// SAFETY: These elements are currently initialized, so it's fine to drop them.
298+
unsafe {
299+
let slice = self.data.get_unchecked_mut(range_to_drop);
300+
ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
301+
}
302+
303+
if n > len { Err(len) } else { Ok(()) }
304+
}
284305
}
285306

286307
#[stable(feature = "array_value_iter_impls", since = "1.40.0")]
@@ -301,6 +322,27 @@ impl<T, const N: usize> DoubleEndedIterator for IntoIter<T, N> {
301322
unsafe { self.data.get_unchecked(idx).assume_init_read() }
302323
})
303324
}
325+
326+
fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
327+
let len = self.len();
328+
329+
// The number of elements to drop. Always in-bounds by construction.
330+
let delta = cmp::min(n, len);
331+
332+
let range_to_drop = (self.alive.end - delta)..self.alive.end;
333+
334+
// Moving the end marks them as conceptually "dropped", so if anything
335+
// goes bad then our drop impl won't double-free them.
336+
self.alive.end -= delta;
337+
338+
// SAFETY: These elements are currently initialized, so it's fine to drop them.
339+
unsafe {
340+
let slice = self.data.get_unchecked_mut(range_to_drop);
341+
ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
342+
}
343+
344+
if n > len { Err(len) } else { Ok(()) }
345+
}
304346
}
305347

306348
#[stable(feature = "array_value_iter_impls", since = "1.40.0")]

library/core/tests/array.rs

+106
Original file line numberDiff line numberDiff line change
@@ -474,3 +474,109 @@ fn array_split_array_mut_out_of_bounds() {
474474

475475
v.split_array_mut::<7>();
476476
}
477+
478+
#[test]
479+
fn array_intoiter_advance_by() {
480+
use std::cell::Cell;
481+
struct DropCounter<'a>(usize, &'a Cell<usize>);
482+
impl Drop for DropCounter<'_> {
483+
fn drop(&mut self) {
484+
let x = self.1.get();
485+
self.1.set(x + 1);
486+
}
487+
}
488+
489+
let counter = Cell::new(0);
490+
let a: [_; 100] = std::array::from_fn(|i| DropCounter(i, &counter));
491+
let mut it = IntoIterator::into_iter(a);
492+
493+
let r = it.advance_by(1);
494+
assert_eq!(r, Ok(()));
495+
assert_eq!(it.len(), 99);
496+
assert_eq!(counter.get(), 1);
497+
498+
let r = it.advance_by(0);
499+
assert_eq!(r, Ok(()));
500+
assert_eq!(it.len(), 99);
501+
assert_eq!(counter.get(), 1);
502+
503+
let r = it.advance_by(11);
504+
assert_eq!(r, Ok(()));
505+
assert_eq!(it.len(), 88);
506+
assert_eq!(counter.get(), 12);
507+
508+
let x = it.next();
509+
assert_eq!(x.as_ref().map(|x| x.0), Some(12));
510+
assert_eq!(it.len(), 87);
511+
assert_eq!(counter.get(), 12);
512+
drop(x);
513+
assert_eq!(counter.get(), 13);
514+
515+
let r = it.advance_by(123456);
516+
assert_eq!(r, Err(87));
517+
assert_eq!(it.len(), 0);
518+
assert_eq!(counter.get(), 100);
519+
520+
let r = it.advance_by(0);
521+
assert_eq!(r, Ok(()));
522+
assert_eq!(it.len(), 0);
523+
assert_eq!(counter.get(), 100);
524+
525+
let r = it.advance_by(10);
526+
assert_eq!(r, Err(0));
527+
assert_eq!(it.len(), 0);
528+
assert_eq!(counter.get(), 100);
529+
}
530+
531+
#[test]
532+
fn array_intoiter_advance_back_by() {
533+
use std::cell::Cell;
534+
struct DropCounter<'a>(usize, &'a Cell<usize>);
535+
impl Drop for DropCounter<'_> {
536+
fn drop(&mut self) {
537+
let x = self.1.get();
538+
self.1.set(x + 1);
539+
}
540+
}
541+
542+
let counter = Cell::new(0);
543+
let a: [_; 100] = std::array::from_fn(|i| DropCounter(i, &counter));
544+
let mut it = IntoIterator::into_iter(a);
545+
546+
let r = it.advance_back_by(1);
547+
assert_eq!(r, Ok(()));
548+
assert_eq!(it.len(), 99);
549+
assert_eq!(counter.get(), 1);
550+
551+
let r = it.advance_back_by(0);
552+
assert_eq!(r, Ok(()));
553+
assert_eq!(it.len(), 99);
554+
assert_eq!(counter.get(), 1);
555+
556+
let r = it.advance_back_by(11);
557+
assert_eq!(r, Ok(()));
558+
assert_eq!(it.len(), 88);
559+
assert_eq!(counter.get(), 12);
560+
561+
let x = it.next_back();
562+
assert_eq!(x.as_ref().map(|x| x.0), Some(87));
563+
assert_eq!(it.len(), 87);
564+
assert_eq!(counter.get(), 12);
565+
drop(x);
566+
assert_eq!(counter.get(), 13);
567+
568+
let r = it.advance_back_by(123456);
569+
assert_eq!(r, Err(87));
570+
assert_eq!(it.len(), 0);
571+
assert_eq!(counter.get(), 100);
572+
573+
let r = it.advance_back_by(0);
574+
assert_eq!(r, Ok(()));
575+
assert_eq!(it.len(), 0);
576+
assert_eq!(counter.get(), 100);
577+
578+
let r = it.advance_back_by(10);
579+
assert_eq!(r, Err(0));
580+
assert_eq!(it.len(), 0);
581+
assert_eq!(counter.get(), 100);
582+
}

0 commit comments

Comments
 (0)