diff --git a/crates/ogar-loco/src/statements.rs b/crates/ogar-loco/src/statements.rs index 8d5322d1..718866cb 100644 --- a/crates/ogar-loco/src/statements.rs +++ b/crates/ogar-loco/src/statements.rs @@ -285,12 +285,15 @@ mod tests { ), Err(StatementError::DanglingOperands { depth: 2 }) ); - // Uncovered: WAIT has no shared-core tables — refuse, never guess. + // Uncovered: RETURN has no shared-core arity — refuse, never guess. + // (Re-pinned from WAIT when the coverage sweep landed: WAIT is now + // covered, RETURN stays refused because it is genuinely variadic — + // void vs value. See `shared_core::stack_arity`'s doc table.) assert_eq!( - statement_bounds(&v, &body(&[Call::with_value(FnIndex::WAIT, 3)])), + statement_bounds(&v, &body(&[Call::with_value(FnIndex::RETURN, 3)])), Err(StatementError::Uncovered { index: 0, - f: FnIndex::WAIT + f: FnIndex::RETURN }) ); } diff --git a/crates/ogar-loco/src/vocabulary.rs b/crates/ogar-loco/src/vocabulary.rs index c3d863f4..68dc24b1 100644 --- a/crates/ogar-loco/src/vocabulary.rs +++ b/crates/ogar-loco/src/vocabulary.rs @@ -13,9 +13,10 @@ //! - Bytes **at/above the floor** belong to the vocabulary. A [`Vocabulary`] //! implementation answers for exactly that range via its `domain_*` hooks; //! the composed methods route each byte to the right table. -//! - A shared-core byte the core does not cover (e.g. `WAIT` today) is -//! **refused everywhere** — a vocabulary does not get to guess for it. -//! Coverage grows in the core, once, for everyone. +//! - A shared-core byte the core does not cover (today: the five genuinely +//! variadic entries listed on [`shared_core::stack_arity`]) is **refused +//! everywhere** — a vocabulary does not get to guess for it. Coverage +//! grows in the core, once, for everyone. //! //! [`conformance::check`] is the mechanical enforcement: run it in every //! vocabulary crate's tests. It catches a composed-method override that @@ -51,8 +52,26 @@ pub mod shared_core { /// `None` means the core does not cover this function — refused rather /// than guessed, because a wrong arity does not produce a slightly-wrong /// result: it desynchronizes the stack and reattributes every later - /// operand. (`WAIT`, `STOP`, `RETURN`, `TERNARY` and others are real - /// palette entries deliberately not yet covered.) + /// operand. + /// + /// # What is still uncovered, and why it is not an oversight + /// + /// Exactly five entries answer `None`, and every one is **genuinely + /// variadic** — it has no single correct arity to record, so covering it + /// would mean choosing a number that is wrong for some legitimate call: + /// + /// | function | why it has no fixed arity | + /// |---|---| + /// | `RETURN` | a void return evaluates nothing; a value return evaluates one | + /// | `NUMBER_PROPERTY` | `even`/`odd`/`prime`/… take the number alone, `divisible by` takes a divisor too | + /// | `PROMPT` | `text_prompt` carries its message as a field, `text_prompt_ext` as an operand | + /// | `LIST_WITH` | a list literal holds as many items as its mutator says | + /// | `PROC_CALL` | a call passes as many arguments as its procedure declares | + /// + /// These want an arity that is a property of the CALL rather than of the + /// function — a per-call count carried in the call's own bytes. That is a + /// deliberate design question, not a table row, so they stay refused + /// until it is answered. Everything else the palette names is covered. /// /// For control flow this counts **only** the evaluated operands. A loop /// body is not among them; see [`body_refs`]. @@ -115,6 +134,84 @@ pub mod shared_core { | FnIndex::AND | FnIndex::OR | FnIndex::JOIN => 2, + + // ── control: the suspend/stop family. + // A duration or a condition, evaluated first; `STOP`'s target + // (this script / all / others) is a selector immediate, not an + // operand — both frontends agree (`control_wait` declares one + // input, `control_stop` declares none). + FnIndex::WAIT | FnIndex::WAIT_UNTIL => 1, + FnIndex::STOP => 0, + + // ── expressions with a fixed shape. + // Corroborated by BOTH frontends where both have the block, and + // by the surviving one where only one does: `logic_ternary` and + // `math_constrain` declare three inputs, `math_atan2` and + // `math_random_int`/`operator_random` two, `math_random_float` + // none, `text_isEmpty` one. + FnIndex::TERNARY | FnIndex::CONSTRAIN => 3, + FnIndex::ATAN2 | FnIndex::RANDOM_INT => 2, + FnIndex::RANDOM_FLOAT => 0, + + // ── text. + // The selector-style ops take the subject only; the dropdown + // (which case, which end to trim) is a value byte, never an + // operand — the same split `math_single[ROOT]` already uses. + FnIndex::IS_EMPTY + | FnIndex::CHANGE_CASE + | FnIndex::TRIM + | FnIndex::REVERSE + | FnIndex::PRINT => 1, + // `APPEND` follows the `VAR_SET` shape below: the variable is an + // immediate, so only the appended value is evaluated + // (`text_append` declares one input). + FnIndex::APPEND => 1, + FnIndex::CHAR_AT | FnIndex::INDEX_OF | FnIndex::CONTAINS | FnIndex::COUNT => 2, + FnIndex::SUBSTRING | FnIndex::REPLACE => 3, + + // ── variables. + // `VAR_GET` is a leaf (above): the variable id rides as an + // immediate value byte and nothing is popped. Its writing twins + // therefore evaluate only the incoming value — which is exactly + // what both frontends declare (`data_setvariableto` and + // `variables_set` one input each, the variable itself a field). + FnIndex::VAR_SET | FnIndex::VAR_CHANGE => 1, + + // ── lists. + // + // The list is an ORDINARY OPERAND here, not an immediate, and + // that is what lets one table serve two frontends that model + // lists differently. Blockly passes a list *expression* + // (`lists_length` takes a LIST input); Scratch names a global + // list in a *field* (`data_lengthoflist` declares zero inputs). + // On a stack machine the two converge: Scratch's field lowers to + // an explicit push of the list reference — the same shape + // `VAR_GET` already has — and the op then pops it. So the arity + // below counts the list, and a Scratch lowering supplies it + // rather than needing a second opcode family. + FnIndex::LIST_EMPTY => 0, + FnIndex::LIST_LENGTH + | FnIndex::LIST_IS_EMPTY + | FnIndex::LIST_DELETE_ALL + | FnIndex::LIST_SORT + | FnIndex::ON_LIST => 1, + FnIndex::LIST_REPEAT + | FnIndex::LIST_GET + | FnIndex::LIST_ADD + | FnIndex::LIST_DELETE + | FnIndex::LIST_INDEX_OF + | FnIndex::LIST_CONTAINS + | FnIndex::LIST_SPLIT => 2, + FnIndex::LIST_SET | FnIndex::LIST_INSERT | FnIndex::LIST_SUBLIST => 3, + + // ── procedures. + // `PROC_DEF` evaluates nothing — its body is a REFERENCE, counted + // by `body_refs`, which is the two-quantity split this module + // exists to keep apart. `PROC_ARG` is a leaf like `VAR_GET`. + FnIndex::PROC_DEF | FnIndex::PROC_ARG => 0, + + // Deliberately still uncovered — see the module note on the five + // variadic entries. Refused rather than guessed. _ => return None, }) } @@ -134,7 +231,13 @@ pub mod shared_core { | FnIndex::REPEAT_UNTIL | FnIndex::FOREVER | FnIndex::FOR_EACH - | FnIndex::FOR_RANGE => 1, + | FnIndex::FOR_RANGE + // A function definition references the body it defines. It + // evaluates nothing (`stack_arity` 0), which is the two-quantity + // split in its purest form — and both frontends declare exactly + // this shape: `procedures_definition` has one statement input and + // zero value inputs. + | FnIndex::PROC_DEF => 1, FnIndex::IF_ELSE => 2, _ => 0, } @@ -944,17 +1047,29 @@ mod tests { ); assert!(shared_core::branches(f), "{f:?} should reference a body"); } - // Silence twin: uncovered control flow stays uncovered rather than - // being quietly assigned a plausible shape. WAIT/STOP/RETURN are real - // palette entries the core does not yet model. + // Silence twin: what the core does NOT model stays refused rather + // than being quietly assigned a plausible shape. + // + // Re-pinned when the coverage sweep landed: WAIT / WAIT_UNTIL / STOP + // are now covered (both frontends declare their shape, so there was + // nothing left to guess). What remains refused is the genuinely + // VARIADIC set — an arity that belongs to the call, not the function + // — which is a design question and not a table row. See + // `shared_core::stack_arity`'s doc table. for f in [ - FnIndex::WAIT, - FnIndex::WAIT_UNTIL, - FnIndex::STOP, FnIndex::RETURN, + FnIndex::NUMBER_PROPERTY, + FnIndex::PROMPT, + FnIndex::LIST_WITH, + FnIndex::PROC_CALL, ] { assert_eq!(shared_core::stack_arity(f), None, "{f:?} must stay refused"); } + // …and the newly covered half, so this test cannot pass by refusing + // everything: a silence twin with no can-fire half proves nothing. + assert_eq!(shared_core::stack_arity(FnIndex::WAIT), Some(1)); + assert_eq!(shared_core::stack_arity(FnIndex::STOP), Some(0)); + assert_eq!(shared_core::stack_arity(FnIndex::LIST_SET), Some(3)); } #[test] @@ -1081,7 +1196,7 @@ mod tests { // uncovered stays None. assert_eq!(checked.pushes_result(FnIndex::ADD), Some(true)); assert_eq!(checked.pushes_result(FnIndex::REPEAT), Some(false)); - assert_eq!(checked.pushes_result(FnIndex::WAIT), None); + assert_eq!(checked.pushes_result(FnIndex::RETURN), None); // Domain half sampled from the hooks; the undeclared pushes column // defaults to None (refused by segmentation, never guessed). assert_eq!(checked.table().stack_arity(FnIndex(0x90)), Some(1)); @@ -1156,8 +1271,8 @@ mod tests { // W-2: names are not coverage — refused-but-real entries are named // so a legend can say "exists, refused" rather than omitting them. assert_eq!(shared_core::name(FnIndex::ADD), Some("ADD")); - assert_eq!(shared_core::name(FnIndex::WAIT), Some("WAIT")); - assert_eq!(shared_core::stack_arity(FnIndex::WAIT), None); + assert_eq!(shared_core::name(FnIndex::RETURN), Some("RETURN")); + assert_eq!(shared_core::stack_arity(FnIndex::RETURN), None); assert_eq!(shared_core::name(FnIndex::NOP), None, "NOP is not an op"); assert_eq!(shared_core::name(FnIndex(0x1F)), None, "reserved = unnamed"); @@ -1241,3 +1356,69 @@ mod tests { ); } } + +#[cfg(test)] +mod coverage { + use super::*; + + /// Every function the palette NAMES has an arity, except the five + /// documented variadic entries. + /// + /// The core used to name 96 operations and give arities for 50 — so a + /// consumer could resolve an opcode, get a legal `FnIndex`, and then find + /// the substrate unable to say what it consumed. That gap was invisible + /// from inside `ogar-loco` (nothing here enumerated it) and surfaced only + /// when a consumer mapped a second frontend onto the same core. + /// + /// This test is the enumeration that was missing. It walks the whole byte + /// range rather than a hand-listed set, so a NEWLY MINTED constant with no + /// arity row fails here immediately instead of years later in a consumer. + #[test] + fn every_named_function_has_an_arity_except_the_documented_variadics() { + const VARIADIC: [FnIndex; 5] = [ + FnIndex::RETURN, + FnIndex::NUMBER_PROPERTY, + FnIndex::PROMPT, + FnIndex::LIST_WITH, + FnIndex::PROC_CALL, + ]; + + let mut named = 0usize; + let mut missing = Vec::new(); + for b in 0..DOMAIN_FLOOR { + let f = FnIndex(b); + if shared_core::name(f).is_none() { + // Not a minted operation — nothing is owed for it. But it + // must not answer either, or the range would be lying. + assert_eq!( + shared_core::stack_arity(f), + None, + "{f:?} is unnamed yet reports an arity" + ); + continue; + } + named += 1; + if shared_core::stack_arity(f).is_none() && !VARIADIC.contains(&f) { + missing.push((b, shared_core::name(f))); + } + } + assert!( + missing.is_empty(), + "named operations with no arity and no documented reason: {missing:?}" + ); + + // Can-stay-silent: the variadics really are still refused, so the + // assertion above cannot be satisfied by covering everything. + for f in VARIADIC { + assert_eq!(shared_core::stack_arity(f), None, "{f:?} must stay refused"); + assert!( + shared_core::name(f).is_some(), + "{f:?} must still be NAMED — refused is not the same as absent" + ); + } + + // And the census itself, pinned: 95 named operations (96 constants + // minus NOP, which is not an operation), 5 refused, 90 covered. + assert_eq!(named, 95, "the named census moved — re-pin deliberately"); + } +}