Skip to content

Commit 2e95813

Browse files
committed
Refactor symbol propertykey and adjust usages
- Change PropertyKey::Symbol to hold Gc<SymbolData> - Update PartialEq/Hash/Display implementations - Adjust has_own_property_value to use SymbolData directly - Update multiple call sites in js_object.rs to use new Symbol key form - Update js_proxy property_key_to_value to return Value::Symbol
1 parent 4480e0b commit 2e95813

File tree

12 files changed

+69
-67
lines changed

12 files changed

+69
-67
lines changed

src/core.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@ use crate::js_regexp::initialize_regexp;
77
use crate::js_string::initialize_string;
88
use crate::raise_eval_error;
99
use crate::unicode::utf8_to_utf16;
10+
pub(crate) use gc_arena::GcWeak;
1011
pub(crate) use gc_arena::Mutation as MutationContext;
12+
pub(crate) use gc_arena::collect::Trace as GcTrace;
1113
pub(crate) use gc_arena::lock::RefLock as GcCell;
1214
pub(crate) use gc_arena::{Collect, Gc};
15+
pub(crate) type GcPtr<'gc, T> = Gc<'gc, GcCell<T>>;
1316
use std::collections::HashMap;
1417

1518
mod gc;
@@ -41,7 +44,7 @@ pub use js_error::*;
4144
#[collect(no_drop)]
4245
pub struct JsRoot<'gc> {
4346
pub global_env: JSObjectDataPtr<'gc>,
44-
pub well_known_symbols: Gc<'gc, GcCell<HashMap<String, gc::GcPtr<'gc, Value<'gc>>>>>,
47+
pub well_known_symbols: Gc<'gc, GcCell<HashMap<String, GcPtr<'gc, Value<'gc>>>>>,
4548
}
4649

4750
pub type JsArena = gc_arena::Arena<gc_arena::Rootable!['gc => JsRoot<'gc>]>;

src/core/eval.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![allow(warnings)]
22

3+
use crate::core::{Gc, GcCell, MutationContext};
34
use crate::js_array::handle_array_static_method;
45
use crate::js_date::{handle_date_method, handle_date_static_method};
56
use crate::js_string::{string_from_char_code, string_from_code_point, string_raw};
@@ -15,9 +16,6 @@ use crate::{
1516
unicode::{utf8_to_utf16, utf16_to_utf8},
1617
};
1718
use crate::{Token, parse_statements, tokenize};
18-
use gc_arena::Gc;
19-
use gc_arena::Mutation as MutationContext;
20-
use gc_arena::lock::RefLock as GcCell;
2119

2220
#[derive(Clone, Debug)]
2321
pub enum ControlFlow<'gc> {
@@ -291,7 +289,7 @@ fn eval_res<'gc>(
291289
};
292290
Ok(Some(ControlFlow::Return(val)))
293291
}
294-
StatementKind::FunctionDeclaration(name, params, body, _) => {
292+
StatementKind::FunctionDeclaration(_name, _params, _body, _) => {
295293
// Function declarations are hoisted, so they are already defined.
296294
Ok(None)
297295
}

src/core/gc.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1+
use crate::core::{Collect, GcTrace};
12
use crate::core::{DestructuringElement, Expr, Statement, StatementKind};
2-
use gc_arena::collect::Trace;
3-
use gc_arena::lock::RefLock as GcCell;
4-
use gc_arena::{Collect, Gc};
5-
6-
pub type GcPtr<'gc, T> = Gc<'gc, GcCell<T>>;
73

84
// Manual implementation of trace to avoid cycles in derive
9-
pub fn trace_expr<'gc, T: Trace<'gc>>(context: &mut T, expr: &Expr) {
5+
pub fn trace_expr<'gc, T: GcTrace<'gc>>(context: &mut T, expr: &Expr) {
106
// helper to find and trace any Expr defaults nested in destructuring elements
11-
fn trace_destructuring<'gc, T: Trace<'gc>>(cc: &mut T, d: &DestructuringElement) {
7+
fn trace_destructuring<'gc, T: GcTrace<'gc>>(cc: &mut T, d: &DestructuringElement) {
128
match d {
139
DestructuringElement::Variable(_, Some(e)) => trace_expr(cc, e),
1410
DestructuringElement::Property(_, inner) => trace_destructuring(cc, inner),
@@ -75,7 +71,7 @@ pub fn trace_expr<'gc, T: Trace<'gc>>(context: &mut T, expr: &Expr) {
7571
}
7672
}
7773

78-
pub fn trace_stmt<'gc, T: Trace<'gc>>(context: &mut T, stmt: &Statement) {
74+
pub fn trace_stmt<'gc, T: GcTrace<'gc>>(context: &mut T, stmt: &Statement) {
7975
match &stmt.kind {
8076
StatementKind::Expr(e) => trace_expr(context, e),
8177
StatementKind::Let(decls) | StatementKind::Var(decls) => {

src/core/property_key.rs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
use crate::core::Value;
2-
use crate::core::gc::GcPtr;
3-
use gc_arena::Collect;
1+
use crate::core::SymbolData;
2+
use crate::core::{Collect, Gc};
43

54
#[derive(Clone, Debug, Collect)]
65
#[collect(no_drop)]
76
pub enum PropertyKey<'gc> {
87
String(String),
9-
Symbol(GcPtr<'gc, Value<'gc>>),
8+
Symbol(Gc<'gc, SymbolData>),
109
}
1110

1211
impl<'gc> From<&str> for PropertyKey<'gc> {
@@ -31,13 +30,7 @@ impl<'gc> PartialEq for PropertyKey<'gc> {
3130
fn eq(&self, other: &Self) -> bool {
3231
match (self, other) {
3332
(PropertyKey::String(s1), PropertyKey::String(s2)) => s1 == s2,
34-
(PropertyKey::Symbol(sym1), PropertyKey::Symbol(sym2)) => {
35-
if let (Value::Symbol(s1), Value::Symbol(s2)) = (&*sym1.borrow(), &*sym2.borrow()) {
36-
gc_arena::Gc::ptr_eq(*s1, *s2)
37-
} else {
38-
false
39-
}
40-
}
33+
(PropertyKey::Symbol(sym1), PropertyKey::Symbol(sym2)) => Gc::ptr_eq(*sym1, *sym2),
4134
_ => false,
4235
}
4336
}
@@ -54,9 +47,7 @@ impl<'gc> std::hash::Hash for PropertyKey<'gc> {
5447
}
5548
PropertyKey::Symbol(sym) => {
5649
1u8.hash(state);
57-
if let Value::Symbol(s) = &*sym.borrow() {
58-
gc_arena::Gc::as_ptr(*s).hash(state);
59-
}
50+
Gc::as_ptr(*sym).hash(state);
6051
}
6152
}
6253
}
@@ -66,7 +57,7 @@ impl<'gc> std::fmt::Display for PropertyKey<'gc> {
6657
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
6758
match self {
6859
PropertyKey::String(s) => write!(f, "{}", s),
69-
PropertyKey::Symbol(ptr) => write!(f, "[symbol {:p}]", gc_arena::Gc::as_ptr(*ptr)),
60+
PropertyKey::Symbol(sym) => write!(f, "[symbol {:p}]", Gc::as_ptr(*sym)),
7061
}
7162
}
7263
}
@@ -75,7 +66,7 @@ impl<'gc> AsRef<str> for PropertyKey<'gc> {
7566
fn as_ref(&self) -> &str {
7667
match self {
7768
PropertyKey::String(s) => s,
78-
PropertyKey::Symbol(_ptr) => todo!("Cannot convert Symbol to &str"),
69+
PropertyKey::Symbol(_sym) => todo!("Cannot convert Symbol to &str"),
7970
}
8071
}
8172
}

src/core/statement.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::core::TemplatePart;
2+
use crate::core::{Collect, GcTrace};
23

34
#[derive(Clone, Debug)]
45
pub struct Statement {
@@ -48,14 +49,14 @@ impl From<StatementKind> for Statement {
4849
}
4950
}
5051

51-
unsafe impl<'gc> gc_arena::Collect<'gc> for Statement {
52-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, cc: &mut T) {
52+
unsafe impl<'gc> Collect<'gc> for Statement {
53+
fn trace<T: GcTrace<'gc>>(&self, cc: &mut T) {
5354
crate::core::gc::trace_stmt(cc, self);
5455
}
5556
}
5657

57-
unsafe impl<'gc> gc_arena::Collect<'gc> for StatementKind {
58-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, _cc: &mut T) {
58+
unsafe impl<'gc> Collect<'gc> for StatementKind {
59+
fn trace<T: GcTrace<'gc>>(&self, _cc: &mut T) {
5960
// Handled via Statement trace
6061
}
6162
}
@@ -136,8 +137,8 @@ pub enum Expr {
136137
ValuePlaceholder,
137138
}
138139

139-
unsafe impl<'gc> gc_arena::Collect<'gc> for Expr {
140-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, cc: &mut T) {
140+
unsafe impl<'gc> Collect<'gc> for Expr {
141+
fn trace<T: GcTrace<'gc>>(&self, cc: &mut T) {
141142
crate::core::gc::trace_expr(cc, self);
142143
}
143144
}
@@ -169,8 +170,8 @@ pub enum BinaryOp {
169170
Pow,
170171
}
171172

172-
unsafe impl<'gc> gc_arena::Collect<'gc> for BinaryOp {
173-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, _cc: &mut T) {}
173+
unsafe impl<'gc> Collect<'gc> for BinaryOp {
174+
fn trace<T: GcTrace<'gc>>(&self, _cc: &mut T) {}
174175
}
175176

176177
#[derive(Debug, Clone)]
@@ -238,8 +239,8 @@ pub enum DestructuringElement {
238239
NestedObject(Vec<DestructuringElement>),
239240
}
240241

241-
unsafe impl<'gc> gc_arena::Collect<'gc> for DestructuringElement {
242-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, cc: &mut T) {
242+
unsafe impl<'gc> Collect<'gc> for DestructuringElement {
243+
fn trace<T: GcTrace<'gc>>(&self, cc: &mut T) {
243244
match self {
244245
DestructuringElement::Variable(_, e) => {
245246
if let Some(e) = e {

src/core/token.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
use crate::core::{Collect, GcTrace};
12
use crate::{JSError, raise_tokenize_error};
2-
use gc_arena::{Collect, collect::Trace};
33
use num_bigint::BigInt;
44
use num_traits::{Num, ToPrimitive};
55

@@ -195,7 +195,7 @@ pub struct TokenData {
195195
unsafe impl<'gc> Collect<'gc> for TokenData {
196196
const NEEDS_TRACE: bool = false;
197197

198-
fn trace<'a, T: Trace<'gc>>(&self, _cc: &mut T) {
198+
fn trace<'a, T: GcTrace<'gc>>(&self, _cc: &mut T) {
199199
// do not trace token to break cycle
200200
}
201201
}

src/core/value.rs

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
11
#![allow(clippy::collapsible_if, clippy::collapsible_match, dead_code)]
22

3-
use gc_arena::Mutation as MutationContext;
4-
use gc_arena::collect::Trace;
5-
use gc_arena::lock::RefLock as GcCell;
6-
use gc_arena::{Collect, Gc};
7-
use num_bigint::BigInt;
8-
use std::sync::{Arc, Mutex};
9-
3+
use crate::core::{Collect, Gc, GcCell, GcPtr, GcTrace, GcWeak, MutationContext};
104
use crate::unicode::utf16_to_utf8;
115
use crate::{
126
JSError,
137
core::{DestructuringElement, PropertyKey, Statement, is_error},
148
raise_type_error,
159
};
16-
17-
pub type GcPtr<'gc, T> = Gc<'gc, GcCell<T>>;
10+
use num_bigint::BigInt;
11+
use std::sync::{Arc, Mutex};
1812

1913
#[derive(Clone, Collect)]
2014
#[collect(no_drop)]
@@ -31,13 +25,13 @@ pub struct JSSet<'gc> {
3125
#[derive(Clone, Collect)]
3226
#[collect(no_drop)]
3327
pub struct JSWeakMap<'gc> {
34-
pub entries: Vec<(gc_arena::GcWeak<'gc, GcCell<JSObjectData<'gc>>>, Value<'gc>)>,
28+
pub entries: Vec<(GcWeak<'gc, GcCell<JSObjectData<'gc>>>, Value<'gc>)>,
3529
}
3630

3731
#[derive(Clone, Collect)]
3832
#[collect(no_drop)]
3933
pub struct JSWeakSet<'gc> {
40-
pub values: Vec<gc_arena::GcWeak<'gc, GcCell<JSObjectData<'gc>>>>,
34+
pub values: Vec<GcWeak<'gc, GcCell<JSObjectData<'gc>>>>,
4135
}
4236

4337
#[derive(Clone, Collect)]
@@ -108,7 +102,7 @@ pub enum GeneratorState<'gc> {
108102
}
109103

110104
pub type JSObjectDataPtr<'gc> = GcPtr<'gc, JSObjectData<'gc>>;
111-
pub type JSObjectDataWeakPtr<'gc> = gc_arena::Gc<'gc, GcCell<JSObjectData<'gc>>>;
105+
pub type JSObjectDataWeakPtr<'gc> = Gc<'gc, GcCell<JSObjectData<'gc>>>;
112106

113107
#[inline]
114108
pub fn new_js_object_data<'gc>(mc: &MutationContext<'gc>) -> JSObjectDataPtr<'gc> {
@@ -126,8 +120,8 @@ pub struct JSObjectData<'gc> {
126120
pub is_function_scope: bool,
127121
}
128122

129-
unsafe impl<'gc> gc_arena::Collect<'gc> for JSObjectData<'gc> {
130-
fn trace<T: gc_arena::collect::Trace<'gc>>(&self, cc: &mut T) {
123+
unsafe impl<'gc> Collect<'gc> for JSObjectData<'gc> {
124+
fn trace<T: GcTrace<'gc>>(&self, cc: &mut T) {
131125
for (k, v) in &self.properties {
132126
k.trace(cc);
133127
(*v.borrow()).trace(cc);
@@ -387,7 +381,7 @@ impl From<&String> for Value<'_> {
387381
}
388382

389383
unsafe impl<'gc> Collect<'gc> for Value<'gc> {
390-
fn trace<T: Trace<'gc>>(&self, cc: &mut T) {
384+
fn trace<T: GcTrace<'gc>>(&self, cc: &mut T) {
391385
match self {
392386
Value::Object(obj) => obj.trace(cc),
393387
Value::Closure(cl) => cl.trace(cc),
@@ -582,6 +576,25 @@ pub fn env_set<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>, key:
582576
Ok(())
583577
}
584578

579+
// Helper: Check whether the given object has an own property corresponding to a
580+
// given JS `Value` (as passed to hasOwnProperty / propertyIsEnumerable). This
581+
// centralizes conversion from various `Value` variants (String/Number/Boolean/
582+
// Undefined/Symbol/other) to a `PropertyKey` and calls `get_own_property`.
583+
// Returns true if an own property exists.
584+
pub fn has_own_property_value<'gc>(obj: &JSObjectDataPtr<'gc>, key_val: &Value<'gc>) -> bool {
585+
match key_val {
586+
Value::String(s) => get_own_property(obj, &utf16_to_utf8(s).into()).is_some(),
587+
Value::Number(n) => get_own_property(obj, &n.to_string().into()).is_some(),
588+
Value::Boolean(b) => get_own_property(obj, &b.to_string().into()).is_some(),
589+
Value::Undefined => get_own_property(obj, &"undefined".into()).is_some(),
590+
Value::Symbol(sd) => {
591+
let sym_key = PropertyKey::Symbol(sd.clone());
592+
get_own_property(obj, &sym_key).is_some()
593+
}
594+
other => get_own_property(obj, &value_to_string(other).into()).is_some(),
595+
}
596+
}
597+
585598
pub fn env_set_recursive<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>, key: &str, val: Value<'gc>) -> Result<(), JSError> {
586599
let mut current = *env;
587600
loop {

src/js_array.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#![allow(warnings)]
22

3+
use crate::core::MutationContext;
34
use crate::{
45
core::{JSObjectDataPtr, PropertyKey, env_set, js_error::EvalError, new_js_object_data},
56
error::JSError,
67
raise_eval_error, raise_range_error,
78
unicode::{utf8_to_utf16, utf16_to_utf8},
89
};
9-
use gc_arena::Mutation as MutationContext;
1010

1111
use crate::core::{
1212
Expr, Value, evaluate_expr, evaluate_statements, get_own_property, obj_get_key_value, obj_set_key_value, obj_set_rc,

src/js_math.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
use crate::core::MutationContext;
12
use crate::core::{JSObjectDataPtr, Value, env_set, new_js_object_data, obj_set_key_value};
23
use crate::error::JSError;
3-
use gc_arena::Mutation as MutationContext;
44

55
/// Create the Math object with all mathematical constants and functions
66
pub fn initialize_math<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>) -> Result<(), JSError> {

src/js_number.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#![allow(clippy::collapsible_if, clippy::collapsible_match)]
22

3+
use crate::core::{Collect, Gc, GcCell, GcPtr, MutationContext, Trace};
34
use crate::core::{Expr, JSObjectDataPtr, Value, evaluate_expr, new_js_object_data, obj_get_key_value, obj_set_key_value, to_primitive};
45
use crate::error::JSError;
56
use crate::unicode::{utf8_to_utf16, utf16_to_utf8};
6-
use gc_arena::Mutation as MutationContext;
77

88
/// Create the Number object with all number constants and functions
99
pub fn make_number_object<'gc>(mc: &MutationContext<'gc>) -> Result<JSObjectDataPtr<'gc>, JSError> {

0 commit comments

Comments
 (0)