diff --git a/crates/perry-hir/src/lower/expr_member.rs b/crates/perry-hir/src/lower/expr_member.rs index 9d8fcd1b2d..1185ade806 100644 --- a/crates/perry-hir/src/lower/expr_member.rs +++ b/crates/perry-hir/src/lower/expr_member.rs @@ -1691,7 +1691,15 @@ fn lower_member_inner(ctx: &mut LoweringContext, member: &ast::MemberExpr) -> Re } = &object_expr { if matches!(inner.as_ref(), Expr::GlobalGet(0)) - && crate::analysis::is_builtin_global_value_name(property) + && (crate::analysis::is_builtin_global_value_name(property) + // #4139: `Math`/`JSON`/`Reflect` bare values now lower to + // `PropertyGet { GlobalGet(0), }` (see lower_expr.rs) so + // reflection sees the real namespace object. But in member-OBJECT + // position (`Math.max(…)`, `JSON.stringify(…)`, `Reflect.get(…)`) + // the intrinsic call / constant-fold paths expect the bare + // `GlobalGet(0)` receiver — undo the reroute here exactly as for + // the built-in constructors, keeping those paths byte-identical. + || matches!(property.as_str(), "Math" | "JSON" | "Reflect")) { if let ast::Expr::Ident(obj_ident) = member.obj.as_ref() { if obj_ident.sym.as_ref() == property.as_str() && property != "globalThis" { diff --git a/crates/perry-hir/src/lower/lower_expr.rs b/crates/perry-hir/src/lower/lower_expr.rs index 94ffc34525..aeff9d63a1 100644 --- a/crates/perry-hir/src/lower/lower_expr.rs +++ b/crates/perry-hir/src/lower/lower_expr.rs @@ -391,6 +391,25 @@ pub(crate) fn lower_expr(ctx: &mut LoweringContext, expr: &ast::Expr) -> Result< } }; Ok(Expr::String(value)) + } else if matches!(name.as_str(), "Math" | "JSON" | "Reflect") { + // #4139: the built-in namespace objects used as VALUES (passed + // to `Object.getOwnPropertyDescriptor(Math, …)`, stored in a + // local, etc.) must resolve to the real + // `populate_global_this_builtins`-installed namespace object — + // not the bare `GlobalGet(0)` sentinel (which IS `globalThis`, + // so `Math === globalThis` and reflection reads the wrong + // object). Reuse the `PropertyGet { GlobalGet(0), }` + // value-form (same as the built-in constructors above). When + // these names appear in member-OBJECT position (`Math.max(…)`, + // `Math.PI`), expr_member.rs's #973 reroute-undo resets the + // receiver back to `GlobalGet(0)`, so the intrinsic call / + // constant-fold paths are unchanged. A shadowing local would + // have matched `ctx.lookup_local` earlier and never reached + // here. + Ok(Expr::PropertyGet { + object: Box::new(Expr::GlobalGet(0)), + property: name, + }) } else { // GlobalGet(0) is a sentinel: codegen routes by name from the // parent PropertyGet/Call/Member context. Bare uses lower to diff --git a/crates/perry-runtime/src/object/global_this.rs b/crates/perry-runtime/src/object/global_this.rs index 23625b47c7..2d0a52fc50 100644 --- a/crates/perry-runtime/src/object/global_this.rs +++ b/crates/perry-runtime/src/object/global_this.rs @@ -2039,9 +2039,16 @@ pub(crate) fn populate_global_this_builtins(singleton: *mut ObjectHeader) { if ns_obj.is_null() { continue; } - if name == "Math" { - install_proto_method(ns_obj, "f16round", math_f16round_thunk as *const u8, 1); - install_proto_method(ns_obj, "random", math_random_thunk as *const u8, 0); + // #4139: reify each namespace's own members as real properties so + // the reflection APIs (`getOwnPropertyDescriptor`, + // `getOwnPropertyNames`) observe them. Call sites (`Math.max(...)`, + // `JSON.stringify(...)`, `Reflect.get(...)`) are codegen intrinsics + // gated on the AST shape and never read these fields. + match name { + "Math" => install_math_namespace_members(ns_obj), + "JSON" => install_json_namespace_members(ns_obj), + "Reflect" => install_reflect_namespace_members(ns_obj), + _ => {} } crate::value::js_nanbox_pointer(ns_obj as i64) }; @@ -2725,6 +2732,125 @@ fn install_proto_method_rest( ); } +/// #4139: reify the `Math` namespace's own members (methods + constants) as +/// real own properties so reflection (`Object.getOwnPropertyDescriptor`, +/// `Object.getOwnPropertyNames`) sees them. The actual `Math.(...)` call +/// sites are codegen intrinsics gated on the AST shape and never read these +/// fields — this is reflection parity only, so the methods are backed by the +/// shared no-op thunk (mirroring `install_proto_method`'s default). Methods +/// are spec'd `{ writable:true, enumerable:false, configurable:true }` +/// (set by `install_proto_method`); constants are `{ writable:false, +/// enumerable:false, configurable:false }`. Member order matches V8/Node's +/// own-key enumeration: methods, then constants, then the newer `f16round`. +fn install_math_namespace_members(ns_obj: *mut ObjectHeader) { + let noop = global_this_builtin_noop_thunk as *const u8; + const METHODS: &[(&str, u32)] = &[ + ("abs", 1), + ("acos", 1), + ("acosh", 1), + ("asin", 1), + ("asinh", 1), + ("atan", 1), + ("atanh", 1), + ("atan2", 2), + ("ceil", 1), + ("cbrt", 1), + ("expm1", 1), + ("clz32", 1), + ("cos", 1), + ("cosh", 1), + ("exp", 1), + ("floor", 1), + ("fround", 1), + ("hypot", 2), + ("imul", 2), + ("log", 1), + ("log1p", 1), + ("log2", 1), + ("log10", 1), + ("max", 2), + ("min", 2), + ("pow", 2), + ("random", 0), + ("round", 1), + ("sign", 1), + ("sin", 1), + ("sinh", 1), + ("sqrt", 1), + ("tan", 1), + ("tanh", 1), + ("trunc", 1), + ]; + for (name, arity) in METHODS.iter().copied() { + // `random` keeps its dedicated thunk so the value-call path + // (`const r = Math.random; r()`) returns a real random number; the + // rest are reflection-only and share the no-op thunk. Installed in + // enumeration position so `getOwnPropertyNames(Math)` order matches V8. + let thunk = if name == "random" { + math_random_thunk as *const u8 + } else { + noop + }; + install_proto_method(ns_obj, name, thunk, arity); + } + let non_writable = super::PropertyAttrs::new(false, false, false); + const CONSTS: &[(&str, f64)] = &[ + ("E", std::f64::consts::E), + ("LN10", std::f64::consts::LN_10), + ("LN2", std::f64::consts::LN_2), + ("LOG10E", std::f64::consts::LOG10_E), + ("LOG2E", std::f64::consts::LOG2_E), + ("PI", std::f64::consts::PI), + ("SQRT1_2", std::f64::consts::FRAC_1_SQRT_2), + ("SQRT2", std::f64::consts::SQRT_2), + ]; + for (name, value) in CONSTS.iter().copied() { + set_intrinsic_data_prop(ns_obj, name, value, non_writable); + } + // `f16round` keeps its dedicated thunk (the only Math member with one). + install_proto_method(ns_obj, "f16round", math_f16round_thunk as *const u8, 1); +} + +/// #4139: reify the `JSON` namespace's own methods for reflection parity. See +/// `install_math_namespace_members` for the rationale (call sites are codegen +/// intrinsics; these no-op-backed fields exist only for reflection). +fn install_json_namespace_members(ns_obj: *mut ObjectHeader) { + let noop = global_this_builtin_noop_thunk as *const u8; + const METHODS: &[(&str, u32)] = &[ + ("parse", 2), + ("stringify", 3), + ("rawJSON", 1), + ("isRawJSON", 1), + ]; + for (name, arity) in METHODS.iter().copied() { + install_proto_method(ns_obj, name, noop, arity); + } +} + +/// #4139: reify the `Reflect` namespace's own methods for reflection parity. +/// See `install_math_namespace_members` for the rationale. +fn install_reflect_namespace_members(ns_obj: *mut ObjectHeader) { + let noop = global_this_builtin_noop_thunk as *const u8; + const METHODS: &[(&str, u32)] = &[ + ("defineProperty", 3), + ("deleteProperty", 2), + ("apply", 3), + ("construct", 2), + ("get", 2), + ("getOwnPropertyDescriptor", 2), + ("getPrototypeOf", 1), + ("has", 2), + ("isExtensible", 1), + ("ownKeys", 1), + ("preventExtensions", 1), + ("set", 3), + ("setPrototypeOf", 2), + ]; + for (name, arity) in METHODS.iter().copied() { + install_proto_method(ns_obj, name, noop, arity); + } +} + /// Install a list of `(method_name, arity)` pairs on a prototype object, /// each backed by `global_this_builtin_noop_thunk`. The shared no-op thunk /// is fine because every method shares the same backing func pointer (the diff --git a/test-parity/node-suite/globals/builtin-namespace-member-descriptors.ts b/test-parity/node-suite/globals/builtin-namespace-member-descriptors.ts new file mode 100644 index 0000000000..6252bf3d4c --- /dev/null +++ b/test-parity/node-suite/globals/builtin-namespace-member-descriptors.ts @@ -0,0 +1,71 @@ +// #4139 — the built-in namespace objects (`Math`, `JSON`, `Reflect`) expose +// their own members (methods + constants) to the reflection APIs, and the +// namespace identifier used as a VALUE resolves to the real namespace object +// rather than `globalThis`. +// +// Before #4139 a bare `Math` lowered to the `globalThis` sentinel, so +// `Object.getOwnPropertyDescriptor(Math, "abs")` reflected `globalThis` +// (returning `undefined`) and `Math === globalThis` held. The intrinsic call +// sites (`Math.max(...)`, `JSON.stringify(...)`, `Reflect.get(...)`) are +// AST-gated codegen paths and are exercised here too to prove they still work. + +// Identity: the namespace value is the real object, not globalThis. +console.log("typeof", typeof Math, typeof JSON, typeof Reflect); +console.log("Math===globalThis.Math", Math === globalThis.Math); +console.log("JSON===globalThis.JSON", JSON === globalThis.JSON); +console.log("Reflect===globalThis.Reflect", Reflect === globalThis.Reflect); +console.log("Math===globalThis", (Math as any) === (globalThis as any)); + +// Namespaces are plain objects: no own `name`/`prototype`. +console.log("Math.name", (Math as any).name); +console.log("Math.prototype", (Math as any).prototype); + +function memberDesc(o: any, label: string, keys: string[]) { + console.log("== " + label + " =="); + for (const k of keys) { + const d = Object.getOwnPropertyDescriptor(o, k); + if (d === undefined) { + console.log(k, "undefined"); + } else if (typeof d.value === "function") { + console.log(k, "fn", d.value.length, d.writable, d.enumerable, d.configurable); + } else { + console.log(k, "data", d.value, d.writable, d.enumerable, d.configurable); + } + } +} + +memberDesc(Math, "Math", ["abs", "max", "atan2", "random", "PI", "E", "SQRT2", "f16round", "nope"]); +memberDesc(JSON, "JSON", ["parse", "stringify", "rawJSON", "isRawJSON", "nope"]); +memberDesc(Reflect, "Reflect", ["get", "set", "has", "ownKeys", "defineProperty", "nope"]); + +// Reflection enumerates own members; they are non-enumerable so Object.keys +// stays empty and the `in` operator sees them. +console.log("Math names count", Object.getOwnPropertyNames(Math).length); +console.log("JSON names", Object.getOwnPropertyNames(JSON).join(",")); +console.log("Reflect names count", Object.getOwnPropertyNames(Reflect).length); +console.log("Object.keys(Math)", JSON.stringify(Object.keys(Math))); +console.log("'abs' in Math", "abs" in Math, "'PI' in Math", "PI" in Math); +console.log("globalThis own has abs", Object.getOwnPropertyNames(globalThis).includes("abs")); + +// The intrinsic call / constant-fold paths are unchanged. +console.log("Math.max(1,5,3)", Math.max(1, 5, 3)); +console.log("Math.abs(-7)", Math.abs(-7)); +console.log("Math.floor(3.7)", Math.floor(3.7)); +console.log("Math.PI", Math.PI); +console.log("JSON.stringify", JSON.stringify({ a: 1, b: [2, 3] })); +console.log("JSON.parse", JSON.parse("[1,2,3]")[2]); +const o = { x: 42 }; +console.log("Reflect.get", Reflect.get(o, "x")); +console.log("Reflect.has", Reflect.has(o, "x")); +console.log("Reflect.ownKeys", JSON.stringify(Reflect.ownKeys(o))); + +// A namespace value passed through a local binding keeps reflecting the object. +const M = Math; +console.log("aliased getOwnPropertyDescriptor", JSON.stringify(Object.getOwnPropertyDescriptor(M, "PI"))); + +// Local shadowing wins over the global namespace. +{ + const Math = { custom: 1 }; + console.log("shadowed custom", (Math as any).custom); + console.log("shadowed max desc", Object.getOwnPropertyDescriptor(Math, "max")); +}