Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation for targets without AtomicU64 #3399

Merged
merged 5 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion boa_engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@

// Add temporarily - Needs addressing
clippy::missing_panics_doc,
clippy::arc_with_non_send_sync,
)]

#[cfg(not(target_has_atomic = "ptr"))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future we might want to create a symbol feature, that requires this, so that users in platforms without this option can have most of JS working :)

compile_error!("Boa requires a lock free `AtomicUsize` in order to work properly.");

HalidOdat marked this conversation as resolved.
Show resolved Hide resolved
extern crate static_assertions as sa;

pub mod bigint;
Expand Down
33 changes: 16 additions & 17 deletions boa_engine/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use num_enum::{IntoPrimitive, TryFromPrimitive};
use std::{
hash::{Hash, Hasher},
sync::{
atomic::{AtomicU64, Ordering},
atomic::{AtomicUsize, Ordering},
jedel1043 marked this conversation as resolved.
Show resolved Hide resolved
Arc,
},
};
Expand All @@ -43,13 +43,13 @@ use std::{
///
/// 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<u64> {
fn get_id() -> Option<usize> {
// 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| {
Expand Down Expand Up @@ -114,11 +114,7 @@ impl WellKnown {
}
}

const fn hash(self) -> u64 {
self as u64
}

const fn tag(self) -> usize {
const fn hash(self) -> usize {
self as usize
}

Expand All @@ -127,12 +123,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<JsString>,
hash: usize,
// must be a `Vec`, since this needs to be shareable between many threads.
description: Option<Vec<u16>>,
jedel1043 marked this conversation as resolved.
Show resolved Hide resolved
}

/// This represents a JavaScript symbol primitive.
Expand All @@ -158,7 +154,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()),
}
}
)+
Expand All @@ -173,7 +169,10 @@ impl JsSymbol {
#[must_use]
pub fn new(description: Option<JsString>) -> Option<Self> {
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.
Expand All @@ -188,8 +187,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
Expand Down Expand Up @@ -223,7 +222,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
Expand Down
33 changes: 17 additions & 16 deletions boa_gc/src/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<T: Trace, const N: usize> Finalize for [T; N] {}
// SAFETY:
// All elements inside the array are correctly marked.
Expand Down
Loading