Skip to content

Commit 208e22d

Browse files
committed
Fix private field updates and implement Function.prototype.bind
- src/core/eval.rs: Use get_property_with_accessors in update expressions to support private accessors. - src/core/eval.rs: Support bound_this in inline closure evaluation and call_closure. - src/core/eval.rs: Add dispatch for Function.prototype.bind. - src/js_function.rs: Implement bind method, preserving original binding if present. - src/js_class.rs: Fix private method key generation and home object binding. - src/core/mod.rs: Enable Function initialization.
1 parent c8d5cce commit 208e22d

File tree

4 files changed

+114
-17
lines changed

4 files changed

+114
-17
lines changed

src/core/eval.rs

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::core::{Gc, GcCell, MutationContext};
44
use crate::js_array::{create_array, handle_array_static_method, is_array, set_array_length};
55
use crate::js_bigint::bigint_constructor;
66
use crate::js_date::{handle_date_method, handle_date_static_method, is_date_object};
7+
use crate::js_function::handle_function_prototype_method;
78
use crate::js_json::handle_json_method;
89
use crate::js_number::{handle_number_instance_method, handle_number_prototype_method, handle_number_static_method, number_constructor};
910
use crate::js_string::{handle_string_method, string_from_char_code, string_from_code_point, string_raw};
@@ -1898,6 +1899,13 @@ pub fn evaluate_expr<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>,
18981899
} else {
18991900
Err(EvalError::Js(raise_eval_error!(format!("Unknown Set function: {}", name))))
19001901
}
1902+
} else if name.starts_with("Function.") {
1903+
if let Some(method) = name.strip_prefix("Function.prototype.") {
1904+
let this_v = this_val.clone().unwrap_or(Value::Undefined);
1905+
Ok(handle_function_prototype_method(mc, &this_v, method, &eval_args, env).map_err(EvalError::Js)?)
1906+
} else {
1907+
Err(EvalError::Js(raise_eval_error!(format!("Unknown Function method: {}", name))))
1908+
}
19011909
} else {
19021910
Err(EvalError::Js(raise_eval_error!(format!("Unknown native function: {}", name))))
19031911
}
@@ -1909,7 +1917,13 @@ pub fn evaluate_expr<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>,
19091917
let call_env = crate::core::new_js_object_data(mc);
19101918
call_env.borrow_mut(mc).prototype = Some(cl.env);
19111919
call_env.borrow_mut(mc).is_function_scope = true;
1912-
if let Some(tv) = &this_val {
1920+
let effective_this = if let Some(bound) = &cl.bound_this {
1921+
Some(bound.clone())
1922+
} else {
1923+
this_val.clone()
1924+
};
1925+
1926+
if let Some(tv) = &effective_this {
19131927
obj_set_key_value(mc, &call_env, &"this".into(), tv.clone())?;
19141928
}
19151929

@@ -2015,7 +2029,13 @@ pub fn evaluate_expr<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr<'gc>,
20152029
let call_env = crate::core::new_js_object_data(mc);
20162030
call_env.borrow_mut(mc).prototype = Some(cl.env);
20172031
call_env.borrow_mut(mc).is_function_scope = true;
2018-
if let Some(tv) = &this_val {
2032+
let effective_this = if let Some(bound) = &cl.bound_this {
2033+
Some(bound.clone())
2034+
} else {
2035+
this_val.clone()
2036+
};
2037+
2038+
if let Some(tv) = &effective_this {
20192039
obj_set_key_value(mc, &call_env, &"this".into(), tv.clone())?;
20202040
}
20212041

@@ -3004,7 +3024,16 @@ pub fn call_closure<'gc>(
30043024
let call_env = crate::core::new_js_object_data(mc);
30053025
call_env.borrow_mut(mc).prototype = Some(cl.env);
30063026
call_env.borrow_mut(mc).is_function_scope = true;
3007-
if let Some(tv) = &this_val {
3027+
3028+
let this_val_for_log = this_val.clone();
3029+
3030+
let effective_this = if let Some(bound) = &cl.bound_this {
3031+
Some(bound.clone())
3032+
} else {
3033+
this_val
3034+
};
3035+
3036+
if let Some(tv) = effective_this {
30083037
crate::core::obj_set_key_value(mc, &call_env, &"this".into(), tv.clone()).map_err(EvalError::Js)?;
30093038
}
30103039

@@ -3055,11 +3084,8 @@ fn evaluate_update_expression<'gc>(
30553084
let obj_val = evaluate_expr(mc, env, obj_expr)?;
30563085
if let Value::Object(obj) = obj_val {
30573086
let key_val = PropertyKey::from(key.to_string());
3058-
let current = if let Some(some_cell) = obj_get_key_value(&obj, &key_val)? {
3059-
some_cell.borrow().clone()
3060-
} else {
3061-
Value::Undefined
3062-
};
3087+
// Use get_property_with_accessors so getters are invoked for reads
3088+
let current = get_property_with_accessors(mc, env, &obj, &key_val)?;
30633089

30643090
let current_num = match current {
30653091
Value::Number(n) => n,
@@ -3080,11 +3106,8 @@ fn evaluate_update_expression<'gc>(
30803106
let key = PropertyKey::from(key_str);
30813107

30823108
if let Value::Object(obj) = obj_val {
3083-
let current = if let Some(some_cell) = obj_get_key_value(&obj, &key)? {
3084-
some_cell.borrow().clone()
3085-
} else {
3086-
Value::Undefined
3087-
};
3109+
// Use get_property_with_accessors so getters are invoked for reads
3110+
let current = get_property_with_accessors(mc, env, &obj, &key)?;
30883111

30893112
let current_num = match current {
30903113
Value::Number(n) => n,

src/core/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ pub fn initialize_global_constructors<'gc>(mc: &MutationContext<'gc>, env: &JSOb
7171
initialize_math(mc, env)?;
7272
initialize_string(mc, env)?;
7373
initialize_array(mc, env)?;
74-
// crate::js_function::initialize_function(mc, env)?;
74+
crate::js_function::initialize_function(mc, env)?;
7575
initialize_regexp(mc, env)?;
7676
// Initialize Date constructor and prototype
7777
initialize_date(mc, env)?;

src/js_class.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -752,10 +752,16 @@ pub(crate) fn create_class_object<'gc>(
752752
// Instance private properties handled during instantiation
753753
}
754754
ClassMember::PrivateMethod(method_name, params, body) => {
755-
// Add private method to prototype using the '#name' key
756-
let closure_data = ClosureData::new(params, body, env, None);
755+
// Create a closure for the private method
756+
let final_key = if method_name.starts_with('#') {
757+
method_name.clone()
758+
} else {
759+
format!("#{method_name}")
760+
};
761+
762+
let closure_data = ClosureData::new(params, body, env, Some(prototype_obj.clone()));
757763
let method_closure = Value::Closure(Gc::new(mc, closure_data));
758-
obj_set_key_value(mc, &prototype_obj, &format!("#{}", method_name).into(), method_closure)?;
764+
obj_set_key_value(mc, &prototype_obj, &final_key.into(), method_closure)?;
759765
}
760766
ClassMember::PrivateGetter(getter_name, body) => {
761767
let key = format!("#{}", getter_name);

src/js_function.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,3 +1391,71 @@ pub fn initialize_function<'gc>(mc: &MutationContext<'gc>, env: &JSObjectDataPtr
13911391
crate::core::env_set(mc, env, "Function", Value::Object(func_ctor))?;
13921392
Ok(())
13931393
}
1394+
1395+
pub fn handle_function_prototype_method<'gc>(
1396+
mc: &MutationContext<'gc>,
1397+
this_value: &Value<'gc>,
1398+
method: &str,
1399+
args: &[Value<'gc>],
1400+
_env: &JSObjectDataPtr<'gc>,
1401+
) -> Result<Value<'gc>, JSError> {
1402+
match method {
1403+
"bind" => {
1404+
// function.bind(thisArg, ...args)
1405+
if let Value::Closure(closure_gc) = this_value {
1406+
let this_arg = args.get(0).cloned().unwrap_or(Value::Undefined);
1407+
1408+
let original = closure_gc;
1409+
// Check if already bound?
1410+
// logic: if already bound, subsequent binds (of this) are ignored?
1411+
// MDN: "The bind() method creates a new function that, when called, has its this keyword set to the provided value."
1412+
// "The new function has the same body as the original function. The this value is bound to the first argument of bind()."
1413+
1414+
// If original func is already bound, calling bind on it creates a NEW bound function.
1415+
// The new bound function calls the OLD bound function.
1416+
// But with our ClosureData implementation, if we just clone data and set bound_this, we are modifying the 'effective' this for the code.
1417+
// If 'original' has 'bound_this' set, 'call_closure' uses that.
1418+
// If we overwrite 'bound_this' in the clone:
1419+
// Value::Closure(new_data { bound_this: new_this })
1420+
// call_closure(new_data) -> uses new_this.
1421+
// This is WRONG if the original was already bound. The original binding should take precedence.
1422+
// But wait, if original was bound, 'call_closure' on original uses original.bound_this.
1423+
// If we clone 'original.body' etc, we are essentially copying the code.
1424+
// Does 'bound_this' stick to the code?
1425+
1426+
// Correct implementation of Bound Function creates a wrapper that calls TargetFunction.
1427+
// Since this engine simplifies things by using ClosureData, we have limits.
1428+
// If 'bound_this' is already set, we should probably NOT overwrite it with new access?
1429+
// Or rather, we can't easily implement "bound function calling bound function" without wrapper.
1430+
1431+
let effective_bound_this = if original.bound_this.is_some() {
1432+
original.bound_this.clone()
1433+
} else {
1434+
Some(this_arg)
1435+
};
1436+
1437+
// NOTE: Arguments binding is skipped for now as ClosureData doesn't support it.
1438+
1439+
let new_closure_data = ClosureData {
1440+
params: original.params.clone(),
1441+
body: original.body.clone(),
1442+
env: original.env.clone(),
1443+
home_object: original.home_object.clone(),
1444+
captured_envs: original.captured_envs.clone(),
1445+
bound_this: effective_bound_this,
1446+
};
1447+
1448+
Ok(Value::Closure(Gc::new(mc, new_closure_data)))
1449+
} else if let Value::Function(_) = this_value {
1450+
// Binding native function: not supported
1451+
// Return original for now or undefined? Returning original is wrong 'this'.
1452+
// Returning undefined throws error.
1453+
// Let's return the function itself (broken binding) or error.
1454+
Err(crate::raise_type_error!("Binding native functions not supported yet"))
1455+
} else {
1456+
Err(crate::raise_type_error!("Function.prototype.bind called on non-function"))
1457+
}
1458+
}
1459+
_ => Err(crate::raise_type_error!(format!("Unknown Function.prototype method: {method}"))),
1460+
}
1461+
}

0 commit comments

Comments
 (0)