fix(codegen): class-typed nullish field read throws TypeError (#7153)

changelog.d/7430-class-typed-nullish-field-read.md

@@ -0,0 +1,8 @@
+Fixed a field read through a class-annotated binding holding `undefined`/`null`
+silently answering `undefined` instead of throwing `TypeError` (#7153) — e.g.
+`short[5].id` on a `Row[]` where index 5 is out of bounds, which Node aborts
+with `Cannot read properties of undefined (reading 'id')`. The class-field
+guard diamond's fallback (value-context and raw-f64 number-context lowerings,
+plus the outlined `js_class_field_get_ic`) now mirrors the generic dispatch
+path's nullish-receiver check on the cold fallback arm; the fast path is
+unchanged. Covered by `test_gap_7153_class_typed_nullish_field_read.ts`.

crates/perry-codegen/src/expr/property_get.rs

@@ -1602,6 +1602,44 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                         }
 
                         ctx.current_block = fallback_idx;
+                        // #7153: the guard rejects a nullish receiver along with
+                        // every other shape miss, but a nullish field read must
+                        // throw TypeError per spec — the by-name lookup below
+                        // answers `undefined` and the program keeps running on a
+                        // silent wrong value. Mirror the generic path's check
+                        // (generic_dispatch.rs); the cost sits on the cold
+                        // fallback arm only.
+                        let (is_null, is_nullish) = {
+                            let blk = ctx.block();
+                            let is_undef =
+                                blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64);
+                            let is_null = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64);
+                            let is_nullish = blk.or(I1, &is_undef, &is_null);
+                            (is_null, is_nullish)
+                        };
+                        let throw_idx = ctx.new_block("class_field_get.throw_nullish");
+                        let lookup_idx = ctx.new_block("class_field_get.fallback_lookup");
+                        let throw_label = ctx.block_label(throw_idx);
+                        let lookup_label = ctx.block_label(lookup_idx);
+                        ctx.block()
+                            .cond_br(&is_nullish, &throw_label, &lookup_label);
+
+                        ctx.current_block = throw_idx;
+                        let prop_entry = ctx.strings.entry(key_idx);
+                        let prop_bytes_global = format!("@{}", prop_entry.bytes_global);
+                        let prop_len_str = prop_entry.byte_len.to_string();
+                        let is_null_i32 = ctx.block().zext(I1, &is_null, I32);
+                        ctx.block().call_void(
+                            "js_throw_type_error_property_access",
+                            &[
+                                (I32, &is_null_i32),
+                                (PTR, &prop_bytes_global),
+                                (I64, &prop_len_str),
+                            ],
+                        );
+                        ctx.block().unreachable();
+
+                        ctx.current_block = lookup_idx;
                         let blk = ctx.block();
                         blk.call_void("js_typed_feedback_record_fallback_call", &[(I64, &site_id)]);
                         let val_fallback_js = blk.call(

crates/perry-codegen/src/expr/property_get/helpers.rs

@@ -13,7 +13,7 @@ use crate::native_value::{
     BoundsState, BufferAccessMode, LoweredValue, MaterializationReason, NativeRep, SemanticKind,
 };
 use crate::type_analysis::receiver_class_name;
-use crate::types::{DOUBLE, I32, I64, I8, PTR};
+use crate::types::{DOUBLE, I1, I32, I64, I8, PTR};
 
 pub(crate) fn class_has_computed_runtime_members(ctx: &FnCtx<'_>, class_name: &str) -> bool {
     ctx.classes
@@ -695,6 +695,39 @@ pub(crate) fn lower_raw_f64_class_field_get_for_number_context(
     );
 
     ctx.current_block = fallback_idx;
+    // #7153: same nullish-receiver check as the value-context diamond in
+    // property_get.rs — a nullish field read must throw TypeError, not coerce
+    // `undefined` to NaN and keep running.
+    let (is_null, is_nullish) = {
+        let blk = ctx.block();
+        let is_undef = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_UNDEFINED_I64);
+        let is_null = blk.icmp_eq(I64, &obj_bits, crate::nanbox::TAG_NULL_I64);
+        let is_nullish = blk.or(I1, &is_undef, &is_null);
+        (is_null, is_nullish)
+    };
+    let throw_idx = ctx.new_block("class_field_get_number.throw_nullish");
+    let lookup_idx = ctx.new_block("class_field_get_number.fallback_lookup");
+    let throw_label = ctx.block_label(throw_idx);
+    let lookup_label = ctx.block_label(lookup_idx);
+    ctx.block()
+        .cond_br(&is_nullish, &throw_label, &lookup_label);
+
+    ctx.current_block = throw_idx;
+    let prop_entry = ctx.strings.entry(key_idx);
+    let prop_bytes_global = format!("@{}", prop_entry.bytes_global);
+    let prop_len_str = prop_entry.byte_len.to_string();
+    let is_null_i32 = ctx.block().zext(I1, &is_null, I32);
+    ctx.block().call_void(
+        "js_throw_type_error_property_access",
+        &[
+            (I32, &is_null_i32),
+            (PTR, &prop_bytes_global),
+            (I64, &prop_len_str),
+        ],
+    );
+    ctx.block().unreachable();
+
+    ctx.current_block = lookup_idx;
     let blk = ctx.block();
     blk.call_void("js_typed_feedback_record_fallback_call", &[(I64, &site_id)]);
     let val_fallback_js = blk.call(

crates/perry-runtime/src/typed_feedback/guards.rs

@@ -761,7 +761,24 @@ pub extern "C" fn js_class_field_get_ic(
 
     crate::typed_feedback::js_typed_feedback_record_fallback_call(site_id);
     let obj_bits = receiver.to_bits();
+    // #7153: this function is the full-outline of the codegen class-field-get
+    // diamond (#5391), so it must mirror the diamond's nullish-receiver check —
+    // a field read on undefined/null throws TypeError instead of answering
+    // `undefined` through the by-name lookup.
     let key_raw = key as u64 & crate::value::POINTER_MASK;
+    if obj_bits == crate::value::TAG_UNDEFINED || obj_bits == crate::value::TAG_NULL {
+        let name = unsafe {
+            crate::object::has_own_helpers::str_from_string_header(
+                key_raw as *const crate::StringHeader,
+            )
+        }
+        .unwrap_or("");
+        crate::error::js_throw_type_error_property_access(
+            (obj_bits == crate::value::TAG_NULL) as u32,
+            name.as_ptr(),
+            name.len(),
+        );
+    }
     crate::object::js_object_get_field_by_name_f64(
         obj_bits as *const ObjectHeader,
         key_raw as *const crate::StringHeader,

test-files/test_gap_7153_class_typed_nullish_field_read.ts

@@ -0,0 +1,71 @@
+// #7153: a field read through a CLASS-ANNOTATED binding whose runtime value is
+// nullish must throw TypeError, exactly like Node — not answer `undefined`.
+//
+// The bug: `const missing = short[5]` on a `Row[]` refines the binding to
+// `Named("Row")`, so the read takes the class-field guard diamond
+// (property_get.rs / property_get/helpers.rs). The guard correctly rejects a
+// non-pointer receiver, but the fallback funneled straight into
+// `js_object_get_field_by_name_f64`, which answers `undefined` for any
+// unrecognized receiver — the program kept running on a silent wrong value.
+// An `any`-typed binding took the generic dispatch path, which has thrown
+// correctly since #462. The reads below are DIRECT (no closure wrapper — a
+// capture could reroute the lowering) and every case prints the caught
+// error's name and message so the file is byte-exact against Node.
+
+class Row {
+  id: number;
+  name: string;
+  score: number;
+  constructor(id: number, name: string, score: number) {
+    this.id = id;
+    this.name = name;
+    this.score = score;
+  }
+}
+
+const short: Row[] = [];
+short.push(new Row(9, "s", 9));
+
+// 1. Value context: the plain class-field guard diamond.
+const missing = short[5];
+try {
+  console.log("oob value:", missing.id);
+} catch (e) {
+  console.log("oob value:", (e as Error).constructor.name + ": " + (e as Error).message);
+}
+
+// 2. Number context: the raw-f64 number-context variant of the same diamond
+//    (`missing.score * 2` routes through the ToNumber-flavored lowering).
+try {
+  console.log("oob number:", missing.score * 2);
+} catch (e) {
+  console.log("oob number:", (e as Error).constructor.name + ": " + (e as Error).message);
+}
+
+// 3. String-typed field on the same nullish receiver.
+try {
+  console.log("oob string field:", missing.name);
+} catch (e) {
+  console.log("oob string field:", (e as Error).constructor.name + ": " + (e as Error).message);
+}
+
+// 4. A null element behind the same class annotation: the message must say
+//    "null", not "undefined".
+const holed: Row[] = [];
+holed.push(null as unknown as Row);
+const nullish = holed[0];
+try {
+  console.log("null value:", nullish.id);
+} catch (e) {
+  console.log("null value:", (e as Error).constructor.name + ": " + (e as Error).message);
+}
+try {
+  console.log("null number:", nullish.score + 1);
+} catch (e) {
+  console.log("null number:", (e as Error).constructor.name + ": " + (e as Error).message);
+}
+
+// 5. Control: an in-bounds element still reads normally through the same
+//    lowering (the fix must not disturb the fast path or valid fallbacks).
+const present = short[0];
+console.log("in bounds:", present.id, present.name, present.score * 2);

test-files/test_gap_repsel_ptr_shape_elements.ts

@@ -26,13 +26,12 @@
 //     mutated with `pop`, one with mixed element classes, one read out of
 //     bounds — each still has to produce Node's answer.
 //
-// NOT covered, deliberately: `short[5].id` where the binding is annotated
-// `Row`. Node throws `TypeError: Cannot read properties of undefined`; Perry
-// prints `undefined` — on `main`, at the base commit, and with
-// `PERRY_PTR_SHAPE_LOCALS=0`, so it is an unrelated pre-existing gap and NOT
-// this pass's OOB hazard (which E5's in-bounds conjunct is what rules out).
-// Asserting `typeof` instead keeps the out-of-bounds read exercised without
-// making this file red for someone else's bug.
+// NOT covered here: `short[5].id` where the binding is annotated `Row` —
+// that was the pre-existing #7153 gap (Perry printed `undefined` where Node
+// throws TypeError), fixed and covered by
+// `test_gap_7153_class_typed_nullish_field_read.ts`. The `typeof` assert
+// below stays: it exercises the out-of-bounds read without a throw, which is
+// this pass's concern (E5's in-bounds conjunct).
 
 class Row {
   id: number;