Skip to content

Commit

Permalink
fix Object.defineProperty with symbols and fix some clippy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Oct 29, 2024
1 parent ba7c2f9 commit a94d64b
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 19 deletions.
22 changes: 10 additions & 12 deletions crates/dash_node_impl/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,34 @@ pub fn init_module(sc: &mut LocalScope<'_>) -> Result<Value, Value> {
let EventsState {
event_emitter_prototype,
event_emitter_constructor,
} = &State::from_vm(cx.scope).store[EventsKey];
} = State::from_vm(cx.scope).store[EventsKey];

let emitter = EventEmitter {
object: NamedObject::with_prototype_and_constructor(
event_emitter_prototype.clone(),
event_emitter_constructor.clone(),
event_emitter_prototype,
event_emitter_constructor,
),
handlers: RefCell::new(FxHashMap::default()),
};
Ok(cx.scope.register(emitter).into())
}),
);
event_emitter_ctor.set_fn_prototype(event_emitter_prototype.clone());
event_emitter_ctor.set_fn_prototype(event_emitter_prototype);
sc.register(event_emitter_ctor)
};

State::from_vm_mut(sc).store.insert(
EventsKey,
EventsState {
event_emitter_constructor: event_emitter_ctor.clone(),
event_emitter_constructor: event_emitter_ctor,
event_emitter_prototype,
},
);

event_emitter_ctor.set_property(
sc,
event_emitter_sym.into(),
PropertyValue::static_default(event_emitter_ctor.clone().into()),
PropertyValue::static_default(event_emitter_ctor.into()),
)?;

Ok(Value::object(event_emitter_ctor))
Expand Down Expand Up @@ -111,7 +111,7 @@ impl Object for EventEmitter {
}

fn on(cx: CallContext) -> Result<Value, Value> {
let [name, cb] = &*cx.args else {
let [name, cb] = *cx.args else {
throw!(cx.scope, Error, "expected an event name and callback function");
};
let name = name.to_js_string(cx.scope)?;
Expand All @@ -123,8 +123,8 @@ fn on(cx: CallContext) -> Result<Value, Value> {
throw!(cx.scope, TypeError, "on can only be called on EventEmitter instances")
};
match this.handlers.borrow_mut().entry(name.sym()) {
Entry::Occupied(mut entry) => entry.get_mut().push(cb.clone()),
Entry::Vacant(entry) => drop(entry.insert(vec![cb.clone()])),
Entry::Occupied(mut entry) => entry.get_mut().push(cb),
Entry::Vacant(entry) => drop(entry.insert(vec![cb])),
};
Ok(cx.this)
}
Expand All @@ -141,9 +141,7 @@ fn emit(cx: CallContext) -> Result<Value, Value> {
let mut did_emit = false;
if let Some(handlers) = this.handlers.borrow().get(&name.sym()) {
for handler in handlers {
handler
.apply(cx.scope, cx.this.clone(), args.to_owned())
.root_err(cx.scope)?;
handler.apply(cx.scope, cx.this, args.to_owned()).root_err(cx.scope)?;
did_emit = true;
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/dash_node_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,13 @@ fn execute_node_module(
}));
let key = scope.intern("exports");
module
.set_property(scope, key.into(), PropertyValue::static_default(exports.clone()))
.set_property(scope, key.into(), PropertyValue::static_default(exports))
.unwrap();

global_state
.ongoing_requires
.borrow_mut()
.insert(file_path.to_owned(), module.clone());
.insert(file_path.to_owned(), module);

let mut code = String::from("(function(exports, module, require) {\n");
code += source;
Expand All @@ -195,7 +195,7 @@ fn execute_node_module(
Err(err) => return Err((err, code)),
};

fun.apply(scope, Value::undefined(), vec![exports, module.clone(), require])
fun.apply(scope, Value::undefined(), vec![exports, module, require])
.map_err(|err| (EvalError::Exception(err), code))?;

Ok(module)
Expand Down
8 changes: 7 additions & 1 deletion crates/dash_vm/src/js_std/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,13 @@ pub fn define_property(cx: CallContext) -> Result<Value, Value> {
};

let property = match cx.args.get(1) {
Some(other) => PropertyKey::from(other.to_js_string(cx.scope)?),
Some(other) => {
if let ValueKind::Symbol(sym) = other.unpack() {
PropertyKey::Symbol(sym)
} else {
PropertyKey::from(other.to_js_string(cx.scope)?)
}
}
_ => throw!(cx.scope, TypeError, "Property must be a string or symbol"),
};
let descriptor = match cx.args.get(2).unpack() {
Expand Down
4 changes: 1 addition & 3 deletions crates/dash_vm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg_attr(dash_lints, feature(register_tool))]
#![cfg_attr(dash_lints, register_tool(dash_lints))]
#![allow(clippy::clone_on_copy)] // TODO: Requires large amounts of changes
#![allow(clippy::clone_on_copy, clippy::needless_borrow)] // TODO: Requires large amounts of changes
#![warn(clippy::redundant_clone)]
#![deny(clippy::disallowed_methods)]

Expand Down Expand Up @@ -1476,11 +1476,9 @@ impl Vm {

// All reachable roots are marked.
debug!("rss before sweep: {}", self.alloc.rss());
let before = self.alloc.rss();
let sweep = span!(Level::TRACE, "gc sweep");
sweep.in_scope(|| unsafe { self.alloc.sweep() });
debug!("rss after sweep: {}", self.alloc.rss());
println!("[GC] {} -> {}", before, self.alloc.rss());

debug!("sweep interner");
self.interner.sweep();
Expand Down

0 comments on commit a94d64b

Please sign in to comment.