From 14fbb45895b766b71b6f9628262744c96150136e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 20:59:49 +0000 Subject: [PATCH] Re-pin the arity-gap assertions: upstream closed the gap Both pins fired exactly as designed when OGAR#265 landed the shared-core arity sweep, and both are re-pinned deliberately rather than relaxed. `scratch.rs` pinned the borrowed-opcode arity gap at 21 -- opcodes the core NAMED but gave no `stack_arity` row. The comment said it would "fail LOUDLY when upstream fixes it, at which point the number drops and this demands a deliberate re-pin". That is what happened. Now 1: `PROC_CALL`, which stays refused upstream because it is genuinely variadic (a call passes as many arguments as its procedure declares), not because anyone forgot it. `projection.rs`'s uncovered-function test is re-pinned for the SECOND time (first off `IF`, now off `PROC_DEF`/`WAIT`, both since covered) -- onto `PROC_CALL`/`RETURN`, which are still genuinely outside the table. It also gains a can-fire half asserting the previously-named functions ARE covered now, so "uncovered" keeps discriminating instead of becoming a label for whatever happens to be missing. Depends on OGAR#265; the path dep means CI here needs it merged first. --- crates/blockly-abi/src/projection.rs | 30 ++++++++++++++++++---------- crates/blockly-abi/src/scratch.rs | 21 ++++++++++--------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/crates/blockly-abi/src/projection.rs b/crates/blockly-abi/src/projection.rs index 7e03b3c..d31dd07 100644 --- a/crates/blockly-abi/src/projection.rs +++ b/crates/blockly-abi/src/projection.rs @@ -607,22 +607,30 @@ mod tests { // A wrong arity does not produce a wrong-looking render — it // desynchronizes the stack and silently reattributes every later // operand. So the table refuses. - // RE-PINNED. This used to name `IF`, which gained a stack arity when - // control flow was covered (now in the shared core's tables) — so - // asserting it is uncovered became wrong. The PROPERTY did not - // change, so it is re-pinned against functions that are still - // genuinely outside the table rather than weakened. - assert_eq!(arity(FnIndex::PROC_DEF), None); - assert_eq!(arity(FnIndex::WAIT), None); - let body = FunctionBody::from_calls(S, &[Call::new(FnIndex::PROC_DEF)]).unwrap(); + // RE-PINNED TWICE. It first named `IF`, which gained an arity when + // control flow was covered; then `PROC_DEF`/`WAIT`, which gained one + // in the upstream coverage sweep. The PROPERTY never changed, so each + // time it is re-pinned onto something still genuinely outside the + // table rather than weakened. What remains uncovered upstream is the + // GENUINELY VARIADIC set — an arity belonging to the call, not the + // function — which is a design question, not a missing row. + assert_eq!(arity(FnIndex::PROC_CALL), None); + assert_eq!(arity(FnIndex::RETURN), None); + // …and the can-fire half: the ops this test used to name ARE covered + // now, so "uncovered" still discriminates instead of being a label + // for whatever happens to be missing. + assert_eq!(arity(FnIndex::PROC_DEF), Some(0)); + assert_eq!(arity(FnIndex::WAIT), Some(1)); + let body = FunctionBody::from_calls(S, &[Call::new(FnIndex::PROC_CALL)]).unwrap(); assert_eq!( render_text(&body), Err(ProjectionError::Uncovered { - function: FnIndex::PROC_DEF.0 + function: FnIndex::PROC_CALL.0 }) ); - // …and `IF` really is covered now, so this test cannot quietly become - // a claim about a function that no longer exists in the gap. + // …and the previously-named functions really are covered now, so this + // test cannot quietly become a claim about a function that no longer + // exists in the gap. assert_eq!(arity(FnIndex::IF), Some(1)); // Two-sided: the covered core still renders, so the refusal is targeted // rather than a blanket failure. diff --git a/crates/blockly-abi/src/scratch.rs b/crates/blockly-abi/src/scratch.rs index 990e6f7..0ded45f 100644 --- a/crates/blockly-abi/src/scratch.rs +++ b/crates/blockly-abi/src/scratch.rs @@ -755,17 +755,16 @@ mod tests { // ── A MEASURED SUBSTRATE GAP, pinned rather than papered over ── // - // Some borrowed opcodes are NAMED by the core (the constant exists) - // but carry no row in `shared_core::stack_arity`. That is a gap in - // `ogar-loco`, not in this mapping, and it PREDATES Scratch: 37 - // Blockly block types land on the same arity-less bytes - // (`cargo run -p blockly-abi --example core_gap`). Per Core-First the - // fix belongs upstream — extend the core deliberately, never hack a - // local arity table in here. + // This pin did its job. It was 21: that many borrowed opcodes were + // NAMED by the core but carried no row in `shared_core::stack_arity` + // — a gap in `ogar-loco` that predated Scratch (37 Blockly types hit + // the same bytes). Per Core-First the fix went upstream rather than + // into a local table here, and when it landed this assertion failed + // exactly as designed, demanding a deliberate re-pin. // - // Pinned as an exact count so it cannot grow silently, and so it - // fails LOUDLY when upstream fixes it — at which point the number - // drops and this demands a deliberate re-pin rather than drifting. + // Now 1: `PROC_CALL`, which stays refused upstream because it is + // genuinely variadic (a call passes as many arguments as its + // procedure declares), not because anyone forgot it. let arity_gap = SCRATCH_CORE .iter() .filter(|&&(_, f)| { @@ -773,7 +772,7 @@ mod tests { }) .count(); assert_eq!( - arity_gap, 21, + arity_gap, 1, "the shared core's arity coverage moved; re-measure with the \ core_gap example and re-pin deliberately" );