Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 32 additions & 14 deletions crates/perry-codegen/src/collectors/hir_facts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,20 +601,6 @@ pub(crate) fn collect_type_facts(
// is a consequence of the range proof, not an additional assumption.
integer_locals.extend(loop_bounded_i32_locals.iter().copied());
let not_bigint_locals = not_bigint.into_locals();
// #8105: locals that hold a JS Number by construction. Computed here, not
// inside the `Ptr<Shape>` pass, so the fact does not vanish under
// `PERRY_PTR_SHAPE_LOCALS=0` — `is_numeric_expr` is not a repsel consumer.
let number_by_construction_locals = super::collect_number_by_construction_locals(
stmts,
params,
boxed_vars,
module_globals,
binding_types,
spec_ta_lens,
spec_numeric_params,
&not_bigint_locals,
module_global_proven_types,
);
let (mut array_facts, effect_facts, materialization_hazards) =
collect_array_facts(stmts, params, module_globals, binding_types);
// #7469: at-allocation all-pointer element-layout declaration candidates.
Expand Down Expand Up @@ -736,6 +722,38 @@ pub(crate) fn collect_type_facts(
spec_numeric_params,
);
array_facts.exact_numeric_element_fields = exact_numeric_element_fields;

// #8105 / #10777: locals that hold a JS Number by construction.
//
// MOVED here from before `collect_shape_proven_ptr_locals`. The old
// position asked "is `h` Number-producing?" for `h = h + o.a` BEFORE `o`'s
// receiver proof existed, so `expr_numeric_by_construction`'s `PropertyGet`
// arm — gated on a tracked member — could never fire, and the accumulator
// was never admitted however completely `o`'s shape was proven. A probe on
// the `+` routing decision reported `left=LocalGet(num=false)
// right=PropertyGet(num=true)`: the slot was proven and the local was not.
//
// The move is a pure reordering — nothing between the two positions
// consumes this fact and the computation has no side effects. The comment
// it replaces claimed the early position kept the fact alive under
// `PERRY_PTR_SHAPE_LOCALS=0`; that still holds, because an empty
// `shape_proven_ptr_locals` yields empty inputs below and the fixpoint then
// computes exactly what it computed before.
let (nbc_shape_members, nbc_shape_numeric_fields) =
super::number_by_construction::shape_numeric_inputs(&shape_proven_ptr_locals);
let number_by_construction_locals = super::collect_number_by_construction_locals(
stmts,
params,
boxed_vars,
module_globals,
binding_types,
spec_ta_lens,
spec_numeric_params,
&not_bigint_locals,
module_global_proven_types,
&nbc_shape_members,
&nbc_shape_numeric_fields,
);
let guarded_argument_route_locals = if module_dispatch.has_argument_shape_routes() {
super::ptr_shape::collect_guarded_argument_route_locals(
stmts,
Expand Down
71 changes: 71 additions & 0 deletions crates/perry-codegen/src/collectors/number_by_construction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ pub(crate) fn collect_number_by_construction_locals(
spec_numeric_params: &HashSet<u32>,
not_bigint_locals: &HashSet<u32>,
module_global_proven_types: &HashMap<u32, HirType>,
// #10777: shape-proven receivers and the property names numeric on all of
// them, from `collect_shape_proven_ptr_locals`. Empty reproduces the
// pre-fix behaviour exactly.
shape_members: &HashSet<u32>,
shape_numeric_fields: &HashSet<String>,
) -> HashSet<u32> {
if !enabled() {
return HashSet::new();
Expand Down Expand Up @@ -155,6 +160,8 @@ pub(crate) fn collect_number_by_construction_locals(
not_bigint_locals,
&HashMap::new(),
&numeric_ta_views,
shape_members,
shape_numeric_fields,
);
numeric.extend(collect_number_at_read_after_undefined(
stmts,
Expand Down Expand Up @@ -648,6 +655,8 @@ mod tests {
&HashSet::new(),
&HashMap::new(),
ta_views,
&HashSet::new(),
&HashSet::new(),
)
}

Expand Down Expand Up @@ -743,3 +752,65 @@ mod tests {
assert!(!numeric.contains(&acc));
}
}

// ── #10777: shape inputs for the function-scope walk ──────────────────────

/// `PERRY_L14_NBC_ORDER` gate. **Default OFF.** When off this returns empty
/// sets, the fixpoint sees exactly what it saw before, and every emitted byte
/// is identical to the pre-fix build — the reorder in `hir_facts.rs` is pure,
/// so the knob gates the INPUTS, not the position. Keyed into the object cache
/// so a warm cache cannot serve the other arm's object.
pub(crate) fn nbc_order_enabled() -> bool {
use std::sync::OnceLock;
static CACHED: OnceLock<bool> = OnceLock::new();
*CACHED.get_or_init(|| {
matches!(
std::env::var("PERRY_L14_NBC_ORDER").as_deref(),
Ok("1") | Ok("on") | Ok("true")
)
})
}

/// Turn the receiver proofs into the `(members, numeric_fields)` pair the
/// function-scope fixpoint needs.
///
/// ## Why an INTERSECTION, and why that is sound
///
/// `expr_numeric_by_construction` takes ONE `numeric_fields` set for ONE
/// receiver class, because its other caller proves one receiver at a time. A
/// function-scope walk may see several shape-proven receivers of different
/// classes, and the arm it feeds asks only "is `members.contains(recv)` and
/// `numeric_fields.contains(prop)`" — it does not re-check which receiver the
/// property belongs to.
///
/// So the set passed must be numeric on **every** admitted receiver, which is
/// the intersection: if `prop` is numeric on all of them, it is numeric on
/// whichever one the expression names. Under-approximates when receivers
/// disagree; exact for a single shape-proven receiver.
///
/// A union would be a WRONG ANSWER, not a weaker one: `a` numeric on `C` and
/// not on `D` would license a bare `fadd` on `D.a`.
pub(crate) fn shape_numeric_inputs(
shape_proven: &HashMap<u32, crate::collectors::ptr_shape::PtrShapeLocal>,
) -> (HashSet<u32>, HashSet<String>) {
if !nbc_order_enabled() || shape_proven.is_empty() {
return (HashSet::new(), HashSet::new());
}
let mut members: HashSet<u32> = HashSet::new();
let mut fields: Option<HashSet<String>> = None;
for (id, fact) in shape_proven {
members.insert(*id);
fields = Some(match fields {
None => fact.numeric_fields.clone(),
Some(acc) => acc
.intersection(&fact.numeric_fields)
.cloned()
.collect::<HashSet<String>>(),
});
}
let fields = fields.unwrap_or_default();
if fields.is_empty() {
return (HashSet::new(), HashSet::new());
}
(members, fields)
}
5 changes: 5 additions & 0 deletions crates/perry-codegen/src/collectors/ptr_shape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,11 @@ fn collect_shape_proven_ptr_locals_impl(
// not the local rooting proof; it has no specialized `TaPtr` context, so
// no view binding is spec-proven here.
&HashSet::new(),
// #10777: the per-receiver proof supplies its own `members` /
// `numeric_fields`; this locals fixpoint feeds it, so it must stay
// empty here or the two would be mutually recursive.
&HashSet::new(),
&HashSet::new(),
);
// A spec entry has validated these parameters before entering this body.
// Unlike a TypeScript annotation, that is runtime evidence, so derived
Expand Down
12 changes: 10 additions & 2 deletions crates/perry-codegen/src/collectors/ptr_shape_numeric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,14 @@ pub(in crate::collectors) fn collect_numeric_by_construction_locals<'a>(
// #8619: view bindings proven to hold a numeric-kind typed array (spec-ABI
// `TaPtr` params). Empty for the `Ptr<Shape>` type-analysis caller.
numeric_ta_views: &HashSet<u32>,
// #10777: shape-proven receivers visible to THIS walk, and the property
// names numeric on all of them. Both were hardcoded empty here, so
// `expr_numeric_by_construction`'s `PropertyGet` arm — gated on
// `members.contains(id)` — could never fire for a function-scope walk. An
// accumulator written `h = h + o.a` was therefore never admitted, however
// completely `o`'s shape was proven. Empty for every pre-existing caller.
shape_members: &HashSet<u32>,
shape_numeric_fields: &HashSet<String>,
) -> HashSet<u32> {
// ONE write walker for both fixpoints (`collect_not_bigint_locals` and
// this one) — see its doc for why sharing is load-bearing. `None` = a
Expand All @@ -460,8 +468,8 @@ pub(in crate::collectors) fn collect_numeric_by_construction_locals<'a>(
stable_local_inits.entry(id).or_insert(Some(*init));
}
}
let empty_members: HashSet<u32> = HashSet::new();
let empty_fields: HashSet<String> = HashSet::new();
let empty_members: HashSet<u32> = shape_members.clone();
let empty_fields: HashSet<String> = shape_numeric_fields.clone();
let mut numeric: HashSet<u32> = let_bound
.into_iter()
.filter(|id| !boxed_vars.contains(id) && !module_globals.contains_key(id))
Expand Down
9 changes: 9 additions & 0 deletions crates/perry/src/commands/compile/object_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,15 @@ fn compute_object_cache_key_with_env(
.unwrap_or(""),
);

// #10777 — numeric-provenance fact ordering. `=1` lets the function-scope
// `number_by_construction` fixpoint see the `Ptr<Shape>` receiver proofs
// computed before it, which flips `both_numeric` and with it the `+`
// lowering. Different IR, different .o bytes.
h.field(
"env_l14_nbc_order",
env_var("PERRY_L14_NBC_ORDER").as_deref().unwrap_or(""),
);

h.finish()
}

Expand Down
Loading