fix(codegen): provenance-based transitive disqualification for integer locals (#4785 bug class)

crates/perry-codegen/src/codegen/closure.rs

@@ -233,6 +233,7 @@ pub(super) fn compile_closure(
         body,
         &flat_const_ids,
         &clamp_fn_ids,
+        &cross_module.clamp3_functions,
         &closure_boxed_vars,
         module_globals,
         classes,

crates/perry-codegen/src/codegen/entry.rs

@@ -238,6 +238,7 @@ pub(super) fn compile_module_entry(
             &hir.init,
             &flat_const_ids,
             &clamp_fn_ids,
+            &cross_module.clamp3_functions,
             &main_boxed_vars,
             module_globals,
             classes,
@@ -668,6 +669,7 @@ pub(super) fn compile_module_entry(
             &hir.init,
             &flat_const_ids,
             &clamp_fn_ids,
+            &cross_module.clamp3_functions,
             &init_boxed_vars,
             module_globals,
             classes,

crates/perry-codegen/src/codegen/function.rs

@@ -142,6 +142,7 @@ pub(super) fn compile_function(
         &f.body,
         &flat_const_ids,
         &clamp_fn_ids,
+        &cross_module.clamp3_functions,
         &boxed_vars,
         module_globals,
         classes,

crates/perry-codegen/src/codegen/method.rs

@@ -126,6 +126,7 @@ pub(super) fn compile_method(
         &method.body,
         &flat_const_ids,
         &clamp_fn_ids,
+        &cross_module.clamp3_functions,
         &method_boxed_vars,
         module_globals,
         classes,
@@ -616,6 +617,7 @@ pub(super) fn compile_static_method(
         &f.body,
         &flat_const_ids,
         &clamp_fn_ids,
+        &cross_module.clamp3_functions,
         &static_boxed_vars,
         module_globals,
         classes,

crates/perry-codegen/src/codegen/mod.rs

@@ -2153,7 +2153,18 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
             }
             walks(s, &module_globals)
         });
-        if crate::collectors::is_integer_specializable(f) && !uses_module_globals {
+        // Skip clamp-shaped functions: their FuncRef call sites with provably
+        // i32 arguments are intrinsified to smax/smin and never call this
+        // symbol, so the only remaining callers are exactly the ones whose
+        // arguments are NOT integers (fractional doubles, NaN-boxed pointers)
+        // — and clamp3 returns an argument verbatim, so the wrapper's
+        // unconditional `fptosi` miscompiles every one of them (#4785 bug
+        // class: `(number).method is not a function`). Those callers need
+        // the real f64 body.
+        let is_clamp_shape =
+            crate::collectors::detect_clamp3(f).is_some() || crate::collectors::detect_clamp_u8(f);
+        if crate::collectors::is_integer_specializable(f) && !uses_module_globals && !is_clamp_shape
+        {
             if let Some(llvm_name) = func_names.get(&f.id) {
                 let i64_name = format!("{}_i64", llvm_name);
                 crate::collectors::emit_i64_function(&mut llmod, f, &i64_name);

crates/perry-codegen/src/collectors/clamp_detect.rs

@@ -76,6 +76,13 @@ pub fn detect_clamp3(f: &Function) -> Option<(u32, u32, u32)> {
 }
 
 /// Detect a 1-param clampU8 pattern: `if (v < 0) return 0; if (v > 255) return 255; return v|0;`
+///
+/// The third statement must actually coerce (`v | 0`, another bitwise op, or
+/// an integer literal). A body ending in bare `return v;` passes a fractional
+/// in-range `v` through unchanged, so treating its callers' results as
+/// int-producing (clamp_u8_functions feeds `clamp_fn_ids` as an
+/// argument-INdependent admission in `collect_integer_locals`) would put a
+/// truncating i32 shadow slot on a non-integer value.
 pub fn detect_clamp_u8(f: &Function) -> bool {
     if f.is_async || f.is_generator || f.params.len() != 1 {
         return false;
@@ -136,7 +143,8 @@ pub fn detect_clamp_u8(f: &Function) -> bool {
     } else {
         return false;
     }
-    true
+    // [2] Return of an int-coercing expression (`v | 0`, bitwise, Integer).
+    matches!(&f.body[2], Stmt::Return(Some(e)) if returns_int_expr(e))
 }
 
 /// A function is i64-specializable if it's a pure numeric recursive fn.

crates/perry-codegen/src/collectors/hir_facts.rs

@@ -138,17 +138,23 @@ impl NativeRegionFactGraph {
 /// Some subgraphs still delegate to established focused collectors; this
 /// function is the single contract used by codegen entry points so new native
 /// consumers do not need to rediscover facts independently.
+#[allow(clippy::too_many_arguments)]
 pub(crate) fn collect_native_region_fact_graph(
     stmts: &[Stmt],
     flat_const_ids: &HashSet<u32>,
     clamp_fn_ids: &HashSet<u32>,
+    arg_dependent_clamp_fn_ids: &HashSet<u32>,
     boxed_vars: &HashSet<u32>,
     module_globals: &HashMap<u32, String>,
     classes: &HashMap<String, &perry_hir::Class>,
     compile_time_constants: &HashMap<u32, f64>,
 ) -> NativeRegionFactGraph {
-    let integer_locals =
-        super::integer_locals::collect_integer_locals(stmts, flat_const_ids, clamp_fn_ids);
+    let integer_locals = super::integer_locals::collect_integer_locals(
+        stmts,
+        flat_const_ids,
+        clamp_fn_ids,
+        arg_dependent_clamp_fn_ids,
+    );
     let unsigned_i32_locals = super::i32_locals::collect_unsigned_i32_locals(stmts);
     let index_used_locals = super::index_uses::collect_index_used_locals(stmts);
     let strictly_i32_bounded_locals = super::i32_locals::collect_strictly_i32_bounded_locals(
@@ -226,6 +232,7 @@ pub(crate) fn collect_hir_facts(
         flat_const_ids,
         clamp_fn_ids,
         &HashSet::new(),
+        &HashSet::new(),
         &HashMap::new(),
         &HashMap::new(),
         &HashMap::new(),
@@ -458,6 +465,7 @@ mod tests {
             &HashSet::new(),
             &pure_helpers,
             &HashSet::new(),
+            &HashSet::new(),
             &HashMap::new(),
             &HashMap::new(),
             &constants,
@@ -490,6 +498,7 @@ mod tests {
             &HashSet::new(),
             &HashSet::new(),
             &HashSet::new(),
+            &HashSet::new(),
             &HashMap::new(),
             &HashMap::new(),
             &HashMap::new(),
@@ -538,6 +547,7 @@ mod tests {
             &stmts,
             &HashSet::new(),
             &HashSet::new(),
+            &HashSet::new(),
         );
 
         assert!(
@@ -575,6 +585,7 @@ mod tests {
             &stmts,
             &HashSet::new(),
             &HashSet::new(),
+            &HashSet::new(),
         );
 
         assert!(ints.contains(&1), "live |0 accumulator must stay integer");
@@ -612,6 +623,7 @@ mod tests {
             &stmts,
             &HashSet::new(),
             &HashSet::new(),
+            &HashSet::new(),
         );
 
         assert!(
@@ -627,4 +639,150 @@ mod tests {
             "const copy of the mutable param binding must not be integer"
         );
     }
+
+    // Provenance hole #1 (the clamp_fn_ids bypass): a local admitted via a
+    // clamp3-shaped call must be pruned when an *argument* of that call is
+    // disqualified — clamp3 returns one of its arguments verbatim, so the
+    // result is only an integer if the arguments are. Previously
+    // `is_int32_producing_expr` accepted any clamp call unconditionally, so
+    // the candidate kept its i32 slot forever.
+    #[test]
+    fn clamp_admitted_local_is_pruned_when_arg_source_is_disqualified() {
+        let clamp_call = |arg: Expr| Expr::Call {
+            callee: Box::new(Expr::FuncRef(7)),
+            args: vec![arg, Expr::Integer(0), Expr::Integer(100)],
+            type_args: vec![],
+        };
+        // let src = undefined; src = obj.value;       (disqualified seed)
+        // const xx = clamp3(src, 0, 100);             (clamp-admitted)
+        // const yy = xx;                              (downstream copy)
+        let stmts = vec![
+            mutable_number_let(1, Expr::Undefined),
+            Stmt::Expr(Expr::LocalSet(
+                1,
+                Box::new(Expr::PropertyGet {
+                    object: Box::new(Expr::LocalGet(99)),
+                    property: "value".to_string(),
+                }),
+            )),
+            const_let(2, clamp_call(Expr::LocalGet(1))),
+            const_let(3, Expr::LocalGet(2)),
+        ];
+        let clamp_ids: HashSet<u32> = [7].into_iter().collect();
+
+        let ints = super::super::integer_locals::collect_integer_locals(
+            &stmts,
+            &HashSet::new(),
+            &clamp_ids,
+            &clamp_ids,
+        );
+        assert!(!ints.contains(&1), "non-int-written seed must be pruned");
+        assert!(
+            !ints.contains(&2),
+            "clamp3-admitted local must follow its disqualified argument"
+        );
+        assert!(
+            !ints.contains(&3),
+            "copy of the clamp3-admitted local must be pruned transitively"
+        );
+
+        // Same shape with integer-stable arguments keeps the optimization.
+        let ok_stmts = vec![
+            mutable_number_let(1, Expr::Integer(5)),
+            const_let(2, clamp_call(Expr::LocalGet(1))),
+            const_let(3, Expr::LocalGet(2)),
+        ];
+        let ints = super::super::integer_locals::collect_integer_locals(
+            &ok_stmts,
+            &HashSet::new(),
+            &clamp_ids,
+            &clamp_ids,
+        );
+        assert!(ints.contains(&2), "int-arg clamp3 result must stay integer");
+        assert!(ints.contains(&3), "copy of live clamp3 result must stay");
+
+        // Argument-INdependent clamp functions (clampU8 / returns_integer —
+        // they coerce internally) must keep admitting double-valued args.
+        let coercing_stmts = vec![const_let(2, clamp_call(Expr::LocalGet(98)))];
+        let ints = super::super::integer_locals::collect_integer_locals(
+            &coercing_stmts,
+            &HashSet::new(),
+            &clamp_ids,
+            &HashSet::new(),
+        );
+        assert!(
+            ints.contains(&2),
+            "internally-coercing clamp result must stay integer regardless of args"
+        );
+    }
+
+    // Provenance hole #2 (init bypass on written locals): a candidate WITH
+    // `LocalSet` writes was never re-validated through its init, so
+    // `let b = a; …use b…; b = 1` kept b integer after `a` was disqualified
+    // — reads between the init and the int write saw a truncated pointer.
+    #[test]
+    fn written_local_is_still_revalidated_through_its_init() {
+        // let a = undefined; a = obj.value;   (disqualified seed)
+        // let b = a;                          (init copies disqualified a)
+        // b = 1;                              (later int write)
+        let stmts = vec![
+            mutable_number_let(1, Expr::Undefined),
+            Stmt::Expr(Expr::LocalSet(
+                1,
+                Box::new(Expr::PropertyGet {
+                    object: Box::new(Expr::LocalGet(99)),
+                    property: "value".to_string(),
+                }),
+            )),
+            mutable_number_let(2, Expr::LocalGet(1)),
+            Stmt::Expr(Expr::LocalSet(2, Box::new(Expr::Integer(1)))),
+        ];
+
+        let ints = super::super::integer_locals::collect_integer_locals(
+            &stmts,
+            &HashSet::new(),
+            &HashSet::new(),
+            &HashSet::new(),
+        );
+        assert!(
+            !ints.contains(&2),
+            "a written local whose init copies a disqualified source must be pruned"
+        );
+    }
+
+    // Provenance hole #3 (Update bypass): `const y = x++` was unconditionally
+    // int-producing even when `x` never was (or stopped being) an integer.
+    #[test]
+    fn update_admitted_local_follows_its_target() {
+        // let x = undefined; x = obj.value; const y = x++;
+        let stmts = vec![
+            mutable_number_let(1, Expr::Undefined),
+            Stmt::Expr(Expr::LocalSet(
+                1,
+                Box::new(Expr::PropertyGet {
+                    object: Box::new(Expr::LocalGet(99)),
+                    property: "value".to_string(),
+                }),
+            )),
+            const_let(
+                2,
+                Expr::Update {
+                    id: 1,
+                    op: perry_hir::UpdateOp::Increment,
+                    prefix: false,
+                },
+            ),
+        ];
+
+        let ints = super::super::integer_locals::collect_integer_locals(
+            &stmts,
+            &HashSet::new(),
+            &HashSet::new(),
+            &HashSet::new(),
+        );
+        assert!(
+            !ints.contains(&2),
+            "`x++` over a disqualified local must not stay integer"
+        );
+    }
 }

crates/perry-codegen/src/collectors/i32_locals.rs

@@ -777,27 +777,6 @@ pub fn collect_integer_let_ids(
 /// `collect_ref_ids_in_expr`: any new HIR Expr variant must recurse into its
 /// sub-expressions here, or the walker may miss a LocalSet hidden inside it
 /// and wrongly mark its target as integer-valued.
-/// Walks the HIR and records LocalIds that have at least one LocalSet whose
-/// rhs is NOT int32-producing. `collect_integer_locals` uses this to remove
-/// locals that lose their integer invariant somewhere in the function.
-pub fn collect_non_int_localset_ids_in_stmts(
-    stmts: &[perry_hir::Stmt],
-    out: &mut HashSet<u32>,
-    known_int_locals: &HashSet<u32>,
-    flat_const_ids: &HashSet<u32>,
-    flat_row_alias_ids: &HashSet<u32>,
-    clamp_fn_ids: &HashSet<u32>,
-) {
-    collect_localset_ids_in_stmts_filtered(
-        stmts,
-        out,
-        Some(known_int_locals),
-        flat_const_ids,
-        flat_row_alias_ids,
-        clamp_fn_ids,
-    );
-}
-
 pub fn collect_localset_ids_in_stmts(stmts: &[perry_hir::Stmt], out: &mut HashSet<u32>) {
     let empty = HashSet::new();
     collect_localset_ids_in_stmts_filtered(stmts, out, None, &empty, &empty, &empty);