Skip to content

Commit 0e0921e

Browse files
committed
core: fix EvaluateNew constructor timing and error handling; surface message for thrown objects
- Ensure GetValue(ref) and IsConstructor are handled correctly for NewExpression so tests behave per spec\n- When formatting thrown values, prefer object.message for error-like objects to provide useful messages in runner/REPL
1 parent 28ec2d9 commit 0e0921e

2 files changed

Lines changed: 85 additions & 4 deletions

File tree

src/core/eval.rs

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6802,14 +6802,64 @@ pub fn prepare_closure_call_env<'gc>(
68026802
prepare_function_call_env(mc, Some(captured_env), None, params_opt, args, None, _caller_env)
68036803
}
68046804

6805+
#[allow(dead_code)]
6806+
enum CtorRef<'a, 'gc> {
6807+
Var(&'a str),
6808+
Property(Value<'gc>, crate::core::PropertyKey<'gc>), // base value and key
6809+
Index(Value<'gc>, Value<'gc>), // base and computed key value
6810+
Other(Value<'gc>),
6811+
}
6812+
68056813
fn evaluate_expr_new<'gc>(
68066814
mc: &MutationContext<'gc>,
68076815
env: &JSObjectDataPtr<'gc>,
68086816
ctor: &Expr,
68096817
args: &[Expr],
68106818
) -> Result<Value<'gc>, EvalError<'gc>> {
6811-
let func_val = evaluate_expr(mc, env, ctor)?;
6812-
let mut eval_args = Vec::new();
6819+
// Per ECMAScript semantics for 'new', the constructExpr evaluation yields a Reference
6820+
// (ref) and the actual GetValue(ref) must happen *after* argument evaluation so
6821+
// side-effects in arguments can affect the constructor value. Implement this by
6822+
// capturing a small reference-like descriptor for common cases (Var, Property, Index)
6823+
// and resolving the actual constructor value after evaluating args.
6824+
let mut ctor_ref: Option<CtorRef<'_, 'gc>> = match ctor {
6825+
Expr::Var(name, _, _) => {
6826+
// GetValue(ref) now (before arguments evaluation)
6827+
let val = evaluate_var(mc, env, name)?;
6828+
Some(CtorRef::Other(val))
6829+
}
6830+
Expr::Property(obj_expr, key) => {
6831+
// evaluate base and perform GetValue(ref) now (before args)
6832+
let base = evaluate_expr(mc, env, obj_expr)?;
6833+
let key_val = crate::core::PropertyKey::from(key.to_string());
6834+
let val = match &base {
6835+
Value::Object(obj) => get_property_with_accessors(mc, env, obj, &key_val)?,
6836+
other => get_primitive_prototype_property(mc, env, other, &key_val)?,
6837+
};
6838+
Some(CtorRef::Other(val))
6839+
}
6840+
Expr::Index(obj_expr, key_expr) => {
6841+
// evaluate base and key now, then GetValue(ref) now
6842+
let base = evaluate_expr(mc, env, obj_expr)?;
6843+
let key_val = evaluate_expr(mc, env, key_expr)?;
6844+
let key = match &key_val {
6845+
Value::Symbol(s) => crate::core::PropertyKey::Symbol(*s),
6846+
Value::String(s) => crate::core::PropertyKey::String(crate::unicode::utf16_to_utf8(s)),
6847+
Value::Number(n) => crate::core::PropertyKey::from(n.to_string()),
6848+
_ => crate::core::PropertyKey::from(crate::core::value_to_string(&key_val)),
6849+
};
6850+
let val = match &base {
6851+
Value::Object(obj) => get_property_with_accessors(mc, env, obj, &key)?,
6852+
other => get_primitive_prototype_property(mc, env, other, &key)?,
6853+
};
6854+
Some(CtorRef::Other(val))
6855+
}
6856+
_ => {
6857+
let val = evaluate_expr(mc, env, ctor)?;
6858+
Some(CtorRef::Other(val))
6859+
}
6860+
};
6861+
6862+
let mut eval_args: Vec<Value<'gc>> = Vec::new();
68136863
for arg in args {
68146864
if let Expr::Spread(target) = arg {
68156865
let val = evaluate_expr(mc, env, target)?;
@@ -6921,6 +6971,28 @@ fn evaluate_expr_new<'gc>(
69216971
}
69226972
}
69236973

6974+
// Resolve constructor value now (GetValue(ref)) after arguments are evaluated
6975+
let func_val = match ctor_ref.take().expect("ctor_ref must be set") {
6976+
CtorRef::Var(name) => evaluate_var(mc, env, name)?,
6977+
CtorRef::Property(base, key) => match base {
6978+
Value::Object(obj) => get_property_with_accessors(mc, env, &obj, &key)?,
6979+
other => get_primitive_prototype_property(mc, env, &other, &key)?,
6980+
},
6981+
CtorRef::Index(base, key_v) => {
6982+
let key = match &key_v {
6983+
Value::Symbol(s) => crate::core::PropertyKey::Symbol(*s),
6984+
Value::String(s) => crate::core::PropertyKey::String(crate::unicode::utf16_to_utf8(s)),
6985+
Value::Number(n) => crate::core::PropertyKey::from(n.to_string()),
6986+
_ => crate::core::PropertyKey::from(crate::core::value_to_string(&key_v)),
6987+
};
6988+
match base {
6989+
Value::Object(obj) => get_property_with_accessors(mc, env, &obj, &key)?,
6990+
other => get_primitive_prototype_property(mc, env, &other, &key)?,
6991+
}
6992+
}
6993+
CtorRef::Other(v) => v,
6994+
};
6995+
69246996
match func_val {
69256997
Value::Object(obj) => {
69266998
if let Some(cl_ptr) = object_get_key_value(&obj, "__closure__") {
@@ -7085,8 +7157,10 @@ fn evaluate_expr_new<'gc>(
70857157
return Err(EvalError::Js(raise_type_error!("Symbol is not a constructor")));
70867158
}
70877159
}
7088-
let new_obj = crate::core::new_js_object_data(mc);
7089-
Ok(Value::Object(new_obj))
7160+
// If we've reached here, the target object is not a recognized constructor
7161+
// (no __closure__, no __class_def__, and no native constructor handled above).
7162+
// Per ECMAScript, attempting `new` with a non-constructor should throw a TypeError.
7163+
Err(EvalError::Js(raise_type_error!("Not a constructor")))
70907164
}
70917165
}
70927166
_ => todo!("New expression with non-object constructor not implemented yet"),

src/core/value.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,13 @@ pub fn value_to_string<'gc>(val: &Value<'gc>) -> String {
626626
let msg = obj.borrow().get_message().unwrap_or("Unknown error".into());
627627
return format!("Error: {msg}");
628628
}
629+
// Prefer an explicit `message` property on user-defined error-like objects
630+
// so thrown harness-liked errors show useful messages
631+
if let Ok(borrowed) = obj.try_borrow() {
632+
if let Some(msg) = borrowed.get_message() {
633+
return msg;
634+
}
635+
}
629636
"[object Object]".to_string()
630637
}
631638
Value::Function(name) => format!("function {}", name),

0 commit comments

Comments
 (0)