From ccea232543eb850bac8921e9f69d76546893d5b7 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Wed, 29 Jun 2022 18:53:15 -0500 Subject: [PATCH] Adjust style and clarify some unsafe operations --- boa_engine/src/bytecompiler.rs | 2 +- boa_interner/src/fixed_string.rs | 8 +++++++- boa_interner/src/lib.rs | 24 ++++++++++++++++++------ boa_interner/src/sym.rs | 6 +++--- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/boa_engine/src/bytecompiler.rs b/boa_engine/src/bytecompiler.rs index 30ae6c8e765..1a2bc509fc3 100644 --- a/boa_engine/src/bytecompiler.rs +++ b/boa_engine/src/bytecompiler.rs @@ -2554,7 +2554,7 @@ impl<'b> ByteCompiler<'b> { ConstructorKind::Base }), ); - code.computed_field_names = Some(boa_gc::Cell::new(vec![])); + code.computed_field_names = Some(boa_gc::Cell::new(Vec::new())); let mut compiler = ByteCompiler { code_block: code, literals_map: FxHashMap::default(), diff --git a/boa_interner/src/fixed_string.rs b/boa_interner/src/fixed_string.rs index 895328e5a6d..659ab27f2eb 100644 --- a/boa_interner/src/fixed_string.rs +++ b/boa_interner/src/fixed_string.rs @@ -13,11 +13,17 @@ impl FixedString { } } - /// Get the maximum capacity of the [`FixedString`]. + /// Gets the maximum capacity of the [`FixedString`]. pub(super) fn capacity(&self) -> usize { self.inner.capacity() } + /// Returns `true` if the [`FixedString`] has length zero, + /// and `false` otherwise. + pub(super) fn is_empty(&self) -> bool { + self.inner.is_empty() + } + /// Tries to push `string` to the [`FixedString`], and returns /// an [`InternedStr`] pointer to the stored `string`, or /// `None` if the capacity is not enough to store `string`. diff --git a/boa_interner/src/lib.rs b/boa_interner/src/lib.rs index 4a96aeac578..984a4538fac 100644 --- a/boa_interner/src/lib.rs +++ b/boa_interner/src/lib.rs @@ -113,6 +113,7 @@ impl Interner { } /// Creates a new [`Interner`] with the specified capacity. + #[inline] pub fn with_capacity(capacity: usize) -> Self { Self { symbols: FxHashMap::default(), @@ -128,8 +129,8 @@ impl Interner { COMMON_STRINGS.len() + self.spans.len() } - #[inline] /// Returns `true` if the [`Interner`] contains no interned strings. + #[inline] pub fn is_empty(&self) -> bool { COMMON_STRINGS.is_empty() && self.spans.is_empty() } @@ -193,7 +194,16 @@ impl Interner { (usize::max(self.head.capacity(), string.len()) + 1).next_power_of_two(); let new_head = FixedString::new(new_cap); let old_head = std::mem::replace(&mut self.head, new_head); - self.full.push(old_head); + + // If the user creates an `Interner` + // with `Interner::with_capacity(BIG_NUMBER)` and + // the first interned string's length is bigger than `BIG_NUMBER`, + // `self.full.push(old_head)` would push a big, empty string of + // allocated size `BIG_NUMBER` into `full`. + // This prevents that case. + if !old_head.is_empty() { + self.full.push(old_head); + } self.head.push_unchecked(string) }) }; @@ -251,13 +261,15 @@ impl Interner { /// Gets the symbol of the common string if one of them fn get_common(string: &str) -> Option { COMMON_STRINGS.get_index(string).map(|idx| - // SAFETY: `idx >= 0`, since it's an `usize`, - // and `idx + 1 > 0` (considering no overflows, but - // an overflow would panic on debug anyways). + // SAFETY: `idx >= 0`, since it's an `usize`, and `idx + 1 > 0`. + // In this case, we don't need to worry about overflows + // because `COMMON_STRINGS` would need to be of considerable + // size to cause an overflow, even on machines with `usize = u32`. unsafe { Sym::new_unchecked(idx + 1) }) } + /// Generates a new symbol for the provided [`str`] pointer. /// /// # Safety @@ -266,7 +278,7 @@ impl Interner { /// memory inside `head` and that it won't be invalidated /// by allocations and deallocations. unsafe fn generate_symbol(&mut self, string: InternedStr) -> Sym { - let next = Sym::new(self.len() + 1).expect("Adding one makes `self.len()` always `> 0`"); + let next = Sym::new(self.len() + 1).expect("cannot get interner symbol: integer overflow"); self.spans.push(string.clone()); self.symbols.insert(string, next); next diff --git a/boa_interner/src/sym.rs b/boa_interner/src/sym.rs index ba68cdf4007..51000e80600 100644 --- a/boa_interner/src/sym.rs +++ b/boa_interner/src/sym.rs @@ -82,18 +82,18 @@ impl Sym { /// Symbol for the `"public"` string. pub const PUBLIC: Self = unsafe { Self::new_unchecked(22) }; - #[inline] /// Creates a new [`Sym`] from the provided `value`, or returns `None` if `index` is zero. + #[inline] pub(super) fn new(value: usize) -> Option { NonZeroUsize::new(value).map(|value| Self { value }) } - #[inline] /// Creates a new [`Sym`] from the provided `value`, without checking if `value` is not zero /// /// # Safety /// /// `value` must not be zero. + #[inline] pub(super) const unsafe fn new_unchecked(value: usize) -> Self { Self { value: @@ -104,8 +104,8 @@ impl Sym { } } - #[inline] /// Returns the internal value of the [`Sym`] + #[inline] pub(super) const fn get(self) -> usize { self.value.get() }