fix: type-check CLI args against entry function params (ILO-517) - #786
Merged
Conversation
parse_cli_arg_for_param now tries f64 parse when expected type is Number. If parse fails, returns NaN sentinel. parse_cli_args_typed checks for NaN on Number-typed params and emits ILO-R005 + exit 1 instead of silently passing Text to a Number param.
A shell string that didn't match its declared param type was bound
as-is: tri n:n>n invoked as 'ilo tri.@ main' gave the n param
Text("main"). Tree and VM printed NaN, the JIT echoed the raw string,
and all exited 0. The CLI was less safe than the language, where
num "main" returns R n t and must be handled.
New check_cli_arg_types, sibling of check_cli_arity, wired into all
four dispatch sites so the error contract can't drift per engine.
Rejects unambiguous mismatches (numeric family, bool, optional inner)
with ILO-R600 naming the parameter and the offending value. Deliberately
permissive elsewhere: _ takes anything, O T takes nil, structural and
user types wave through, so the ILO-182 single-fn pass-through
(ilo greet.@ world) keeps working.
R600 is the first allocation in the hundreds-block runtime range; flat
R0xx codes are historical-only per SPEC and enforced by
error_code_namespaces.
Eight regression tests: rejection on default/--vm/--jit, valid args unchanged, ILO-182 bare-ident-to-t pass-through, bool accept/reject, any-type passthrough, optional nil-vs-text, diagnostic names the param and value. Verified the rejection tests fail with the guard disabled. examples/cli-arg-types.ilo pins the pass-through shapes in the examples harness (rejections can't live there - it asserts on success output).
ce7e865 landed a parallel half-finished approach: non-numeric input to a Number param became Number(NAN), which sails through the type guard (NaN is a Number) and broke the explicit-fn dispatch path - two of the branch's own regression tests failed. The guard-based fix rejects at the boundary with ILO-R600 and needs no sentinel; restore cli_parse.rs to main.
rust 1.97 adds useless_borrows_in_formatting; CI lint runs -D warnings so the pre-existing borrow in this untouched test now blocks every PR.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Second half of the ce7e865 cleanup: with the sentinel reverted, a NaN can no longer reach this block from CLI parsing, and its raw eprintln bypassed the diagnostic system with a code (ILO-R005) that belongs to field-not-found. The type guard covers the case properly with ILO-R600.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A shell string that did not match its declared parameter type was bound as-is.
tri n:n>ninvoked asilo tri.@ mainhanded thenparamText("main"): tree and VM printed NaN, the Cranelift JIT echoed the raw string, and every engine exited 0. A silent wrong answer with a success exit code is worse than a crash for any caller that checks $?. The CLI was less safe than the language it fronts, wherenum "main"returnsR n tand the checker forces the failure to be handled.Manifesto framing: the silent NaN cost a benchmark model four retries on an otherwise-correct program because the repair loop's entire error signal was the string "NaN" (ILO-364 traces). A boundary that fails loudly is cheaper than one that fails quietly.
Repro before/after
What's in the diff (per commit)
check_cli_arg_types, sibling ofcheck_cli_arity, wired into all four dispatch sites (VM, interpreter, JIT, default) so the error contract cannot drift per engine. Rejects unambiguous mismatches (numeric family, bool, optional inner) with ILO-R600 naming the parameter and offending value - first allocation in the hundreds-block runtime range per SPEC (flat R0xx are historical-only, enforced byerror_code_namespaces). Deliberately permissive elsewhere:_takes anything,O Ttakes nil, structural and user-defined types wave through, so the ILO-182 single-fn pass-through (ilo greet.@ world) keeps working. Guard chosen over threading Result throughparse_cli_arg_for_parambecause the AOT C-callableilo_aot_parse_arg_listcannot propagate one.examples/cli-arg-types.ilopins the pass-through shapes in the examples harness.nparam parse toNumber(NAN), which sails straight through a type guard and broke the explicit-fn path; restoredcli_parse.rsto main.Test plan
tests/regression_cli_arg_type_mismatch.rs: 8/8 across three engines, rejection tests verified failing without the guardcargo fmt+cargo clippy --all-targetscleanilo --explain ILO-R600serves the registry entryFollow-ups
-5binds toU32without complaint - range validation vs type validation, separate ticketSum(_)membership is not validated at the CLI boundary (deliberate - the guard is not a second type checker)