From 96ab7ccc8f41d5e0f81e2c27bf8f0654950b1a3e Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Sun, 27 Oct 2019 17:42:49 +0100 Subject: [PATCH 1/7] Replace each_allocated_chunk function with iter_allocated_chunks function --- src/lib.rs | 69 ++++++++++++++++++++++++++++++++++---------------- tests/tests.rs | 33 ++++++++++++------------ 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 77827e8..d55d5fb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,6 +122,8 @@ mod imports { pub use std::cell::{Cell, UnsafeCell}; pub use std::cmp; pub use std::fmt; + pub use std::iter; + pub use std::marker::PhantomData; pub use std::mem; pub use std::ptr::{self, NonNull}; pub use std::slice; @@ -134,6 +136,8 @@ mod imports { pub use core::cell::{Cell, UnsafeCell}; pub use core::cmp; pub use core::fmt; + pub use core::iter; + pub use core::marker::PhantomData; pub use core::mem; pub use core::ptr::{self, NonNull}; pub use core::slice; @@ -663,10 +667,10 @@ impl Bump { } } - /// Call `f` on each chunk of allocated memory that this arena has bump - /// allocated into. + /// Returns an iterator over the each chunk of allocated memory that + /// from an arena has bump allocated into. /// - /// `f` is invoked in order of allocation: oldest chunks first, newest + /// Chunks are returned in order of allocation: oldest chunks first, newest /// chunks last. /// /// ## Safety @@ -678,8 +682,8 @@ impl Bump { /// don't need to worry about here. /// /// However, there could be regions of uninitialized memory used as padding - /// between allocations. Reading uninitialized memory is big time undefined - /// behavior! + /// between allocations, which is why this iterator has items of type + /// `[MaybeUninit]`, instead of simply `[u8]`. /// /// The only way to guarantee that there is no padding between allocations /// or within allocated objects is if all of these properties hold: @@ -689,8 +693,9 @@ impl Bump { /// 3. None of the objects allocated in this arena contain any internal /// padding. /// - /// If you want to use this `each_allocated_chunk` method, it is *your* - /// responsibility to ensure that these properties hold! + /// If you want to use this `iter_allocated_chunks` method, it is *your* + /// responsibility to ensure that these properties hold before calling + /// `MaybeUninit::assume_init` or otherwise reading the returned values. /// /// ## Example /// @@ -705,20 +710,39 @@ impl Bump { /// /// // Iterate over each chunk we've bump allocated into. This is safe /// // because we have only allocated `i32` objects in this arena. - /// unsafe { - /// bump.each_allocated_chunk(|ch| { - /// println!("Used a chunk that is {} bytes long", ch.len()); - /// }); + /// for ch in bump.iter_allocated_chunks() { + /// println!("Used a chunk that is {} bytes long", ch.len()); + /// println!("The first byte is {:?}", unsafe { ch.get(0).unwrap().assume_init() }); /// } /// ``` - pub unsafe fn each_allocated_chunk(&mut self, mut f: F) - where - F: for<'a> FnMut(&'a [u8]), - { - let mut footer = Some(self.all_chunk_footers.get()); - while let Some(foot) = footer { - let foot = foot.as_ref(); + pub fn iter_allocated_chunks(&mut self) -> ChunkIter<'_> { + ChunkIter { + footer: Some(self.all_chunk_footers.get()), + bump: PhantomData, + } + } +} +/// An iterator over the each chunk of allocated memory that +/// from an arena has bump allocated into. +/// +/// Chunks are returned in order of allocation: oldest chunks first, newest +/// chunks last. +/// +/// This struct is created by the [`iter_allocated_chunks`] method on +/// [Bump]. See that function for a safety description regarding. +#[derive(Debug)] +pub struct ChunkIter<'a> { + footer: Option>, + bump: PhantomData<&'a mut Bump>, +} + +impl<'a> Iterator for ChunkIter<'a> { + type Item = &'a [mem::MaybeUninit]; + fn next(&mut self) -> Option<&'a [mem::MaybeUninit]> { + unsafe { + let foot = self.footer?; + let foot = foot.as_ref(); let start = foot.data.as_ptr() as usize; let end_of_allocated_region = foot.ptr.get().as_ptr() as usize; debug_assert!(end_of_allocated_region <= foot as *const _ as usize); @@ -730,14 +754,15 @@ impl Bump { ); let len = end_of_allocated_region - start; - let slice = slice::from_raw_parts(start as *const u8, len); - f(slice); - - footer = foot.next.get(); + let slice = slice::from_raw_parts(start as *const mem::MaybeUninit, len); + self.footer = foot.next.get(); + Some(slice) } } } +impl<'a> iter::FusedIterator for ChunkIter<'a> {} + #[inline(never)] #[cold] fn oom() -> ! { diff --git a/tests/tests.rs b/tests/tests.rs index a750f46..3264348 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -40,23 +40,22 @@ fn can_iterate_over_allocated_things() { // Safe because we always allocated objects of the same type in this arena, // and their size >= their align. - unsafe { - bump.each_allocated_chunk(|ch| { - let ch_usize = ch.as_ptr() as usize; - println!("iter chunk @ 0x{:x}", ch_usize); - assert_eq!( - chunks.pop().unwrap(), - ch_usize, - "should iterate over each chunk once, in order they were allocated in" - ); - - let ch: &[u64] = - slice::from_raw_parts(ch.as_ptr() as *mut u64, ch.len() / mem::size_of::()); - for i in ch { - assert!(*i < MAX, "{} < {} (aka {:x} < {:x})", i, MAX, i, MAX); - seen[*i as usize] = true; - } - }); + for ch in bump.iter_allocated_chunks() { + let ch_usize = ch.as_ptr() as usize; + println!("iter chunk @ 0x{:x}", ch_usize); + assert_eq!( + chunks.pop().unwrap(), + ch_usize, + "should iterate over each chunk once, in order they were allocated in" + ); + + let ch: &[u64] = unsafe { + slice::from_raw_parts(ch.as_ptr() as *mut u64, ch.len() / mem::size_of::()) + }; + for i in ch { + assert!(*i < MAX, "{} < {} (aka {:x} < {:x})", i, MAX, i, MAX); + seen[*i as usize] = true; + } } assert!(seen.iter().all(|s| *s)); From cfd01aed9f5d70e06661cbe36e4ffb2936c18995 Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:28:41 +0100 Subject: [PATCH 2/7] Update src/lib.rs Co-Authored-By: Nick Fitzgerald --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index d55d5fb..91bd69f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -667,7 +667,7 @@ impl Bump { } } - /// Returns an iterator over the each chunk of allocated memory that + /// Returns an iterator over each chunk of allocated memory that /// from an arena has bump allocated into. /// /// Chunks are returned in order of allocation: oldest chunks first, newest From 75bdf167aeb8e0f745452655e7a9ba2910867681 Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:30:01 +0100 Subject: [PATCH 3/7] Update src/lib.rs Co-Authored-By: Nick Fitzgerald --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 91bd69f..e9306ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -730,7 +730,7 @@ impl Bump { /// chunks last. /// /// This struct is created by the [`iter_allocated_chunks`] method on -/// [Bump]. See that function for a safety description regarding. +/// [Bump]. See that function for a safety description regarding reading from the returned items. #[derive(Debug)] pub struct ChunkIter<'a> { footer: Option>, From c89d03d284d9f51dfc38c5985206f3951e82a811 Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:30:08 +0100 Subject: [PATCH 4/7] Update src/lib.rs Co-Authored-By: Nick Fitzgerald --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e9306ac..7d232dd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -724,7 +724,7 @@ impl Bump { } /// An iterator over the each chunk of allocated memory that -/// from an arena has bump allocated into. +/// an arena has bump allocated into. /// /// Chunks are returned in order of allocation: oldest chunks first, newest /// chunks last. From 68c1e4df5cd6206501eb684c33c4f16dd1fdf59b Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:30:14 +0100 Subject: [PATCH 5/7] Update src/lib.rs Co-Authored-By: Nick Fitzgerald --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 7d232dd..d55bda3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -723,7 +723,7 @@ impl Bump { } } -/// An iterator over the each chunk of allocated memory that +/// An iterator over each chunk of allocated memory that /// an arena has bump allocated into. /// /// Chunks are returned in order of allocation: oldest chunks first, newest From 3b42fd827e99090707352a00d8f5dce439479c97 Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:30:20 +0100 Subject: [PATCH 6/7] Update src/lib.rs Co-Authored-By: Nick Fitzgerald --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index d55bda3..75aeb14 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -668,7 +668,7 @@ impl Bump { } /// Returns an iterator over each chunk of allocated memory that - /// from an arena has bump allocated into. + /// this arena has bump allocated into. /// /// Chunks are returned in order of allocation: oldest chunks first, newest /// chunks last. From 9f2d3758da5095cb5696333614604e554d8b4922 Mon Sep 17 00:00:00 2001 From: Mathias Svensson Date: Tue, 29 Oct 2019 09:38:58 +0100 Subject: [PATCH 7/7] Re-introduce each_allocated_chunk for backwards compatibility --- src/lib.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 75aeb14..eb9e149 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -721,6 +721,64 @@ impl Bump { bump: PhantomData, } } + + /// Call `f` on each chunk of allocated memory that this arena has bump + /// allocated into. + /// + /// `f` is invoked in order of allocation: oldest chunks first, newest + /// chunks last. + /// + /// ## Safety + /// + /// Because this method takes `&mut self`, we know that the bump arena + /// reference is unique and therefore there aren't any active references to + /// any of the objects we've allocated in it either. This potential aliasing + /// of exclusive references is one common footgun for unsafe code that we + /// don't need to worry about here. + /// + /// However, there could be regions of uninitialized memory used as padding + /// between allocations. Reading uninitialized memory is big time undefined + /// behavior! + /// + /// The only way to guarantee that there is no padding between allocations + /// or within allocated objects is if all of these properties hold: + /// + /// 1. Every object allocated in this arena has the same alignment. + /// 2. Every object's size is a multiple of its alignment. + /// 3. None of the objects allocated in this arena contain any internal + /// padding. + /// + /// If you want to use this `each_allocated_chunk` method, it is *your* + /// responsibility to ensure that these properties hold! + /// + /// ## Example + /// + /// ``` + /// let mut bump = bumpalo::Bump::new(); + /// + /// // Allocate a bunch of things in this bump arena, potentially causing + /// // additional memory chunks to be reserved. + /// for i in 0..10000 { + /// bump.alloc(i); + /// } + /// + /// // Iterate over each chunk we've bump allocated into. This is safe + /// // because we have only allocated `i32` objects in this arena. + /// unsafe { + /// bump.each_allocated_chunk(|ch| { + /// println!("Used a chunk that is {} bytes long", ch.len()); + /// }); + /// } + /// ``` + #[deprecated(note = "deprecated in favor of iter_allocated_chunks")] + pub unsafe fn each_allocated_chunk(&mut self, mut f: F) + where + F: for<'a> FnMut(&'a [u8]), + { + for chunk in self.iter_allocated_chunks() { + f(slice::from_raw_parts(chunk.as_ptr() as *const u8, chunk.len())); + } + } } /// An iterator over each chunk of allocated memory that