From 15333833e60a40c3d29ecc8f95b6905b1070a38c Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 19 Oct 2023 03:53:56 -0600 Subject: [PATCH 1/5] Fix compilation for targets without `AtomicU64` --- boa_engine/src/lib.rs | 1 - boa_engine/src/symbol.rs | 38 ++++++++++++++++++-------------------- boa_gc/src/trace.rs | 33 +++++++++++++++++---------------- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/boa_engine/src/lib.rs b/boa_engine/src/lib.rs index a0e0b483837..28b9678baa7 100644 --- a/boa_engine/src/lib.rs +++ b/boa_engine/src/lib.rs @@ -123,7 +123,6 @@ // Add temporarily - Needs addressing clippy::missing_panics_doc, - clippy::arc_with_non_send_sync, )] extern crate static_assertions as sa; diff --git a/boa_engine/src/symbol.rs b/boa_engine/src/symbol.rs index cd087d31ed0..137ba9babde 100644 --- a/boa_engine/src/symbol.rs +++ b/boa_engine/src/symbol.rs @@ -33,23 +33,22 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::{ hash::{Hash, Hasher}, - sync::{ - atomic::{AtomicU64, Ordering}, - Arc, - }, + sync::{atomic::Ordering, Arc}, }; +use portable_atomic::AtomicUsize; + /// Reserved number of symbols. /// /// This is where the well known symbol live /// and internal engine symbols. -const RESERVED_SYMBOL_HASHES: u64 = 127; +const RESERVED_SYMBOL_HASHES: usize = 127; -fn get_id() -> Option { +fn get_id() -> Option { // Symbol hash. // // For now this is an incremented u64 number. - static SYMBOL_HASH_COUNT: AtomicU64 = AtomicU64::new(RESERVED_SYMBOL_HASHES + 1); + static SYMBOL_HASH_COUNT: AtomicUsize = AtomicUsize::new(RESERVED_SYMBOL_HASHES + 1); SYMBOL_HASH_COUNT .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |value| { @@ -114,11 +113,7 @@ impl WellKnown { } } - const fn hash(self) -> u64 { - self as u64 - } - - const fn tag(self) -> usize { + const fn hash(self) -> usize { self as usize } @@ -127,12 +122,12 @@ impl WellKnown { } } -// TODO: Address below clippy::arc_with_non_send_sync below. /// The inner representation of a JavaScript symbol. #[derive(Debug, Clone)] struct Inner { - hash: u64, - description: Option, + hash: usize, + // must be a `Vec`, since this needs to be shareable between many threads. + description: Option>, } /// This represents a JavaScript symbol primitive. @@ -158,7 +153,7 @@ macro_rules! well_known_symbols { $( $(#[$attr])* pub(crate) const fn $name() -> JsSymbol { JsSymbol { - repr: Tagged::from_tag($variant.tag()), + repr: Tagged::from_tag($variant.hash()), } } )+ @@ -173,7 +168,10 @@ impl JsSymbol { #[must_use] pub fn new(description: Option) -> Option { let hash = get_id()?; - let arc = Arc::new(Inner { hash, description }); + let arc = Arc::new(Inner { + hash, + description: description.map(|s| Vec::from(&*s)), + }); Some(Self { // SAFETY: Pointers returned by `Arc::into_raw` must be non-null. @@ -188,8 +186,8 @@ impl JsSymbol { match self.repr.unwrap() { UnwrappedTagged::Ptr(ptr) => { // SAFETY: `ptr` comes from `Arc`, which ensures the validity of the pointer - // as long as we corrently call `Arc::from_raw` on `Drop`. - unsafe { ptr.as_ref().description.clone() } + // as long as we correctly call `Arc::from_raw` on `Drop`. + unsafe { ptr.as_ref().description.as_ref().map(|v| js_string!(&**v)) } } UnwrappedTagged::Tag(tag) => { // SAFETY: All tagged reprs always come from `WellKnown` itself, making @@ -223,7 +221,7 @@ impl JsSymbol { /// The hash is guaranteed to be unique. #[inline] #[must_use] - pub fn hash(&self) -> u64 { + pub fn hash(&self) -> usize { match self.repr.unwrap() { UnwrappedTagged::Ptr(ptr) => { // SAFETY: `ptr` comes from `Arc`, which ensures the validity of the pointer diff --git a/boa_gc/src/trace.rs b/boa_gc/src/trace.rs index ba5704bed74..070a93ffc58 100644 --- a/boa_gc/src/trace.rs +++ b/boa_gc/src/trace.rs @@ -11,10 +11,7 @@ use std::{ }, path::{Path, PathBuf}, rc::Rc, - sync::atomic::{ - AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicU16, AtomicU32, - AtomicU64, AtomicU8, AtomicUsize, - }, + sync::atomic, }; /// Substitute for the [`Drop`] trait for garbage collected types. @@ -161,20 +158,24 @@ simple_empty_finalize_trace![ NonZeroI64, NonZeroU64, NonZeroI128, - NonZeroU128, - AtomicBool, - AtomicIsize, - AtomicUsize, - AtomicI8, - AtomicU8, - AtomicI16, - AtomicU16, - AtomicI32, - AtomicU32, - AtomicI64, - AtomicU64 + NonZeroU128 ]; +#[cfg(target_has_atomic = "8")] +simple_empty_finalize_trace![atomic::AtomicBool, atomic::AtomicI8, atomic::AtomicU8]; + +#[cfg(target_has_atomic = "16")] +simple_empty_finalize_trace![atomic::AtomicI16, atomic::AtomicU16]; + +#[cfg(target_has_atomic = "32")] +simple_empty_finalize_trace![atomic::AtomicI32, atomic::AtomicU32]; + +#[cfg(target_has_atomic = "64")] +simple_empty_finalize_trace![atomic::AtomicI64, atomic::AtomicU64]; + +#[cfg(target_has_atomic = "ptr")] +simple_empty_finalize_trace![atomic::AtomicIsize, atomic::AtomicUsize]; + impl Finalize for [T; N] {} // SAFETY: // All elements inside the array are correctly marked. From a9e0c760d532277ef35647e205d4971647b8a86f Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 19 Oct 2023 10:07:58 -0600 Subject: [PATCH 2/5] Throw compilation error if `AtomicUsize` is not available --- boa_engine/src/lib.rs | 3 +++ boa_engine/src/symbol.rs | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/boa_engine/src/lib.rs b/boa_engine/src/lib.rs index 28b9678baa7..6a3d6bcbded 100644 --- a/boa_engine/src/lib.rs +++ b/boa_engine/src/lib.rs @@ -125,6 +125,9 @@ clippy::missing_panics_doc, )] +#[cfg(not(target_has_atomic = "ptr"))] +compile_error!("Boa requires a lock free `AtomicUsize` in order to work properly."); + extern crate static_assertions as sa; pub mod bigint; diff --git a/boa_engine/src/symbol.rs b/boa_engine/src/symbol.rs index 137ba9babde..472b1bf7fdd 100644 --- a/boa_engine/src/symbol.rs +++ b/boa_engine/src/symbol.rs @@ -33,11 +33,12 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::{ hash::{Hash, Hasher}, - sync::{atomic::Ordering, Arc}, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, }; -use portable_atomic::AtomicUsize; - /// Reserved number of symbols. /// /// This is where the well known symbol live From 5170efb03b7b6ade0061aaf08a6d8b5e0bc36403 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 19 Oct 2023 17:14:42 -0600 Subject: [PATCH 3/5] Apply review pt. 1 --- boa_engine/src/symbol.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boa_engine/src/symbol.rs b/boa_engine/src/symbol.rs index 472b1bf7fdd..8b637a336eb 100644 --- a/boa_engine/src/symbol.rs +++ b/boa_engine/src/symbol.rs @@ -128,7 +128,7 @@ impl WellKnown { struct Inner { hash: usize, // must be a `Vec`, since this needs to be shareable between many threads. - description: Option>, + description: Option>, } /// This represents a JavaScript symbol primitive. @@ -171,7 +171,7 @@ impl JsSymbol { let hash = get_id()?; let arc = Arc::new(Inner { hash, - description: description.map(|s| Vec::from(&*s)), + description: description.map(|s| Box::from(&*s)), }); Some(Self { From 99c121e3092c3dae3a1e3bb3d8bbac15542d3692 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 19 Oct 2023 18:24:36 -0600 Subject: [PATCH 4/5] Apply review pt. 2 --- boa_engine/src/symbol.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/boa_engine/src/symbol.rs b/boa_engine/src/symbol.rs index 8b637a336eb..42e3720be3a 100644 --- a/boa_engine/src/symbol.rs +++ b/boa_engine/src/symbol.rs @@ -33,23 +33,22 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::{ hash::{Hash, Hasher}, - sync::{ - atomic::{AtomicUsize, Ordering}, - Arc, - }, + sync::{atomic::Ordering, Arc}, }; +use portable_atomic::AtomicU64; + /// Reserved number of symbols. /// -/// This is where the well known symbol live -/// and internal engine symbols. -const RESERVED_SYMBOL_HASHES: usize = 127; +/// This is the maximum number of well known and internal engine symbols +/// that can be defined. +const RESERVED_SYMBOL_HASHES: u64 = 127; -fn get_id() -> Option { +fn get_id() -> Option { // Symbol hash. // // For now this is an incremented u64 number. - static SYMBOL_HASH_COUNT: AtomicUsize = AtomicUsize::new(RESERVED_SYMBOL_HASHES + 1); + static SYMBOL_HASH_COUNT: AtomicU64 = AtomicU64::new(RESERVED_SYMBOL_HASHES + 1); SYMBOL_HASH_COUNT .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |value| { @@ -114,8 +113,8 @@ impl WellKnown { } } - const fn hash(self) -> usize { - self as usize + const fn hash(self) -> u64 { + self as u64 } fn from_tag(tag: usize) -> Option { @@ -126,7 +125,7 @@ impl WellKnown { /// The inner representation of a JavaScript symbol. #[derive(Debug, Clone)] struct Inner { - hash: usize, + hash: u64, // must be a `Vec`, since this needs to be shareable between many threads. description: Option>, } @@ -154,7 +153,8 @@ macro_rules! well_known_symbols { $( $(#[$attr])* pub(crate) const fn $name() -> JsSymbol { JsSymbol { - repr: Tagged::from_tag($variant.hash()), + // the cast shouldn't matter since we only have 127 const symbols + repr: Tagged::from_tag($variant.hash() as usize), } } )+ @@ -222,7 +222,7 @@ impl JsSymbol { /// The hash is guaranteed to be unique. #[inline] #[must_use] - pub fn hash(&self) -> usize { + pub fn hash(&self) -> u64 { match self.repr.unwrap() { UnwrappedTagged::Ptr(ptr) => { // SAFETY: `ptr` comes from `Arc`, which ensures the validity of the pointer From bff44bbe5382c12c58199c4e8ea2ef44382e6423 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 19 Oct 2023 18:25:52 -0600 Subject: [PATCH 5/5] fix comment --- boa_engine/src/symbol.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa_engine/src/symbol.rs b/boa_engine/src/symbol.rs index 42e3720be3a..c7e33f9b44e 100644 --- a/boa_engine/src/symbol.rs +++ b/boa_engine/src/symbol.rs @@ -126,7 +126,7 @@ impl WellKnown { #[derive(Debug, Clone)] struct Inner { hash: u64, - // must be a `Vec`, since this needs to be shareable between many threads. + // must be a `Box`, since this needs to be shareable between many threads. description: Option>, }