Skip to content
Merged
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ For the release process and tag conventions, see [RELEASING.md](RELEASING.md).

### Fixed

- **CLI arguments are type-checked against the entry function's parameters (ILO-517).** A shell string that didn't match its declared parameter type used to be bound as-is, because `parse_cli_arg`'s ladder falls through to `Text` for anything non-numeric. So `tri n:n>n` invoked as `ilo tri.@ main` bound `Value::Text("main")` to a `n` parameter: the tree-walker and VM printed `NaN`, the Cranelift JIT echoed `main`, and every engine exited **0** — a silent wrong answer with a success exit code, which is worse than a crash for any caller that checks `$?`. The CLI was therefore less safe than the language it fronts, where `num "main"` returns `R n t` and the type checker forces the failure to be handled. A new CLI-boundary guard (`check_cli_arg_types`, sibling of the existing `check_cli_arity`) now rejects unambiguously mismatched values with `ILO-R600`, naming the parameter and showing the offending literal, and exits 1. Wired into all four dispatch sites (VM, interpreter, JIT, default) so the error contract can't drift per engine the way ILO-177 did. The guard is deliberately permissive — `_` accepts anything, `O T` still accepts `nil`, and structural / user-defined types are waved through — so the ILO-182 single-fn pass-through keeps working (`ilo greet.@ world` against `s:t` is legitimate usage, not a typo'd function name). Also closes the same hole for `b` parameters. Found while running the ILO-364 closed-loop benchmark, where it cost a model retries on an otherwise-correct program.

- **`get-stream` yields lines on newline, not buffer-fill (ILO-489).** The client-side streaming builtins (`get-stream`, `get-stream-h`, `pst-stream`, `pst-stream-h`, ILO-448) wrapped minreq's `ResponseLazy` in `BufReader::lines()`, which blocks on a full ~8 KiB read-buffer fill before surfacing any line. So a slow SSE upstream that flushes one short event then idles had its lines batched until the buffer filled or the connection closed - functionally correct but latency was buffer-bound, not event-bound. The line splitter now consumes `ResponseLazy`'s byte iterator incrementally and emits each line the instant its `\n` arrives (trailing `\r` stripped for CRLF / chunked encoding), so each event surfaces promptly. EOF still yields a trailing newline-less partial line; mid-stream errors still surface as `ILO-R009 http-stream read error: ...`. Unblocks ILO-482's previously flaky end-to-end streaming test.

### Added
Expand Down
32 changes: 32 additions & 0 deletions examples/cli-arg-types.ilo
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
-- CLI arguments are checked against the entry function's declared parameter
-- types before any engine runs (ILO-517). A mismatch is `ILO-R600` with a
-- non-zero exit, not a silently-bound wrong value.
--
-- Previously `ilo file.ilo main` against `tri n:n>n` bound the text "main"
-- to a `n` parameter: tree/VM printed `NaN`, the JIT echoed `main`, and all
-- of them exited 0.
--
-- The guard is deliberately permissive, and this file pins the shapes that
-- must keep working:
-- * `n` accepts a numeric literal
-- * `t` accepts a bare ident (the ILO-182 single-fn pass-through)
-- * `_` accepts anything
-- * `O n` accepts `nil`
--
-- The rejection cases can't live here — the examples harness asserts on
-- successful output — so they're covered by
-- tests/regression_cli_arg_type_mismatch.rs, which runs each one across
-- the default, --vm and --jit engines.

tri n:n>n;s=+n 1;p=*n s;/p 2

greet s:t>t;+"hello " s

anything x:_>t;"got"

maybe x:O n>t;??x 0;"ok"

main>t;fmt "{} {} {} {}" (tri 10) (greet "world") (anything "main") (maybe nil)

-- run: main
-- out: 55 hello world got ok
28 changes: 28 additions & 0 deletions src/diagnostic/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,34 @@ A division operation (`/`) was performed with a zero divisor.

An operation was applied to a value of the wrong type at runtime.
This may indicate a verifier gap for a dynamic code path.
"#,
},
ErrorEntry {
code: "ILO-R600",
phase: Phase::Runtime,
short: "CLI argument does not match the declared parameter type",
long: r#"## ILO-R600: CLI argument does not match the declared parameter type

A value passed on the command line could not be interpreted as the type
the entry function declares for that parameter.

```
tri n:n>n;s=+n 1;p=*n s;/p 2

$ ilo tri.@ main
ILO-R600: argument 1 (`n`) expects n, got text `main`
```

Before this check existed the mismatched value was bound as-is, so a `n`
parameter could receive text. Arithmetic on it produced `NaN` and the
process still exited 0 — a silent wrong answer rather than an error.

This mirrors the guarantee the language already makes in-band: `num "main"`
returns `R n t`, so the type checker forces the failure to be handled.
The CLI boundary now behaves the same way.

**Fix:** pass a value of the declared type, or widen the parameter type
(`_` accepts anything, `O n` accepts `nil`).
"#,
},
ErrorEntry {
Expand Down
120 changes: 120 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4771,6 +4771,11 @@ fn dispatch_run(
{
return code;
}
if let Err(code) =
check_cli_arg_types(&program, func_name, &run_args, &source, mode)
{
return code;
}
let compiled = match vm::compile(&program) {
Ok(c) => c,
Err(e) => {
Expand Down Expand Up @@ -4810,6 +4815,11 @@ fn dispatch_run(
{
return code;
}
if let Err(code) =
check_cli_arg_types(&program, func_name, &run_args, &source, mode)
{
return code;
}
run_interp_with_provider(
&program,
func_name,
Expand Down Expand Up @@ -5005,6 +5015,9 @@ fn run_cranelift_engine(
if let Err(code) = check_cli_arity(program, func_name, run_args.len(), source, mode) {
return code;
}
if let Err(code) = check_cli_arg_types(program, func_name, &run_args, source, mode) {
return code;
}
let suppress = program_result_should_suppress(program, func_name);

#[cfg(feature = "cranelift")]
Expand Down Expand Up @@ -5466,6 +5479,9 @@ fn run_default(
if let Err(code) = check_cli_arity(program, func_name, args.len(), source, mode) {
return code;
}
if let Err(code) = check_cli_arg_types(program, func_name, &args, source, mode) {
return code;
}
let suppress = program_result_should_suppress(program, func_name);
// Default engine is the bytecode register VM: it supports every opcode
// (closures, listview, len-has-k-count, every modern shape), and avoids
Expand Down Expand Up @@ -6532,6 +6548,110 @@ fn check_cli_arity(
Err(1)
}

/// Describe a parsed CLI value for a diagnostic message: the ilo type sigil
/// plus the offending literal, so the reader can see both what arrived and
/// what was expected.
fn describe_cli_value(v: &interpreter::Value) -> String {
match v {
interpreter::Value::Number(n) => format!("number `{n}`"),
interpreter::Value::Text(s) => format!("text `{s}`"),
interpreter::Value::Bool(b) => format!("bool `{b}`"),
interpreter::Value::Nil => "nil".to_string(),
interpreter::Value::List(_) => "a list".to_string(),
_ => "a value of a different type".to_string(),
}
}

/// Whether a parsed CLI value is compatible with a declared parameter type.
///
/// Deliberately permissive: this guard exists to catch values that are
/// *unambiguously* wrong (text bound to `n`), not to re-implement the type
/// checker at the CLI boundary. Anything structural or user-defined is
/// waved through and left to the engines.
fn cli_value_matches_type(v: &interpreter::Value, ty: &ast::Type) -> bool {
match ty {
// The numeric family. This is the case ILO-R600 exists for: binding
// text here yielded NaN with a zero exit code.
ast::Type::Number | ast::Type::U32 | ast::Type::U64 | ast::Type::I64 => {
matches!(v, interpreter::Value::Number(_))
}
ast::Type::Bool => matches!(v, interpreter::Value::Bool(_)),
// `parse_cli_arg_for_param` guarantees Text for `t` params, so there
// is nothing left to check.
ast::Type::Text => true,
// `nil` is a legitimate value for an optional; anything else must
// satisfy the inner type.
ast::Type::Optional(inner) => {
matches!(v, interpreter::Value::Nil) || cli_value_matches_type(v, inner)
}
// `parse_cli_args_typed` already wraps non-lists for `L _` params.
// Everything below is either "don't care" or too structural to judge
// from a shell string without guessing.
ast::Type::Any
| ast::Type::List(_)
| ast::Type::Map(_, _)
| ast::Type::Result(_, _)
| ast::Type::Sum(_)
| ast::Type::Fn(_, _)
| ast::Type::Named(_) => true,
}
}

/// CLI-boundary type guard. Sibling of `check_cli_arity`: that one checks how
/// many args arrived, this one checks that each is the declared type.
///
/// Without it, a `n:n` parameter could receive `Value::Text` (a shell string
/// that isn't numeric falls through `parse_cli_arg`'s ladder to `Text`).
/// Downstream arithmetic then produced `NaN` and the process exited 0, so a
/// caller passing an unvalidated string got a silent wrong answer instead of
/// an error — the CLI was less safe than the language it fronts, where
/// `num "main"` returns `R n t` and must be handled.
///
/// Returns `Err(1)` on the first mismatch so callers can short-circuit before
/// handing off to any engine, keeping the diagnostic identical across
/// tree / VM / JIT dispatch.
fn check_cli_arg_types(
program: &ast::Program,
func_name: Option<&str>,
args: &[interpreter::Value],
source: &str,
mode: OutputMode,
) -> Result<(), i32> {
let target = match resolve_entry_func_name(program, func_name) {
Some(t) => t,
None => return Ok(()),
};
let params = match lookup_param_types(program, Some(target)) {
Some(p) => p,
None => return Ok(()),
};
for (i, param) in params.iter().enumerate() {
let Some(v) = args.get(i) else { break };
if cli_value_matches_type(v, &param.ty) {
continue;
}
let err = interpreter::RuntimeError {
code: "ILO-R600",
message: format!(
"argument {} (`{}`) expects {}, got {}",
i + 1,
param.name,
ilo::codegen::fmt::type_str(&param.ty),
describe_cli_value(v)
),
span: None,
call_stack: Vec::new(),
propagate_value: None,
};
report_diagnostic(
&Diagnostic::from(&err).with_source(source.to_string()),
mode,
);
return Err(1);
}
Ok(())
}

/// Parse and coerce CLI args against a function's declared parameter types.
///
/// - For `Type::Text` params, the raw CLI string is preserved as `Text`
Expand Down
2 changes: 1 addition & 1 deletion tests/regression_bench_silent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn bench_silent_suppresses_program_stdout_under_json() {
non_json_lines.is_empty(),
"expected only JSON envelopes on stdout under --silent; saw {} non-JSON lines (first few: {:?})",
non_json_lines.len(),
&non_json_lines.iter().take(3).collect::<Vec<_>>()
non_json_lines.iter().take(3).collect::<Vec<_>>()
);

// ...and we still got bench numbers — at least one envelope per
Expand Down
Loading
Loading