7d8792e · knot-legacy · git.iko.soy

git.iko.soy/knot-legacy

Close `#`-argument gaps: loud errors, and a concrete default beats `_`

Three fixes to `#`-argument handling: 1. Non-CLI-settable args error loudly instead of silently no-op'ing. A `#`-arg whose type isn't CLI-parseable (a record, a function, a collection of non-scalar elements like `Vec Mode`) used to silently ignore a passed flag and use the default. Codegen's non-overridable branch now emits a runtime check (`knot_override_forbid`) that aborts with a clear error when the flag is present: with { Cfg {name Text} Cfg #cfg {name "default"} } ./app '--cfg={name "x"}' → Error: argument --cfg has a type that cannot be set from the command line With no flag, the default still applies. A required (`_`) non-settable arg already errored at build time; that path is unchanged. 2. A concrete default beats a required `_` across scopes. `#port _` (required) in one scope + `#port 8080` (default) in another used to abort with "missing required argument" even when the default existed — each ArgDecl emitted its own lookup, and the `_` one always aborted. Now the shadowing/agreement pass returns the merged per-arg defaults, threaded into codegen via `TypeEnv::cli_arg_defaults`; a `_` declaration whose name has a concrete default elsewhere emits the flag-or-default path instead of the abort. The flag still wins when present; a `_` with NO concrete default anywhere still aborts. `check_with_chain_shadowing` now returns `(diagnostics, arg_defaults)` alongside its diagnostics. 3. Collection-of-refined (`Vec Pos`, `Rel Pos`) is non-settable (refined elements aren't CLI-parseable scalars), so it now errors loudly via fix 1 rather than silently no-op'ing. Scalar refined args are unaffected. 4 new tests (record flag abort, collection-of-non-scalar abort, `_`+default in both declaration orders, truly-required `_` still aborts). Full workspace suite green (49 suites).

commit7d8792e5c6c68fd5633bf04f6aff9a99f8fcc60a authoriko <ilyakooo0@gmail.com> dateSun 6 Sep 2026, 09:45 (1 month ago) parent0653e18

5 files changed+244−26

compare with master

unifiedsplit

crates/knot-compiler/src/codegen.rs+81−21 crates/knot-compiler/src/main.rs+2−1 crates/knot-compiler/src/types.rs+21−2 crates/knot-jit/tests/cli_overrides.rs+122 crates/knot-runtime/src/lib.rs+18−2

crates/knot-compiler/src/codegen.rs+81−21

@@ -237,6 +237,11 @@ pub struct Codegen<M: cranelift_module::Module = ObjectModule> {

237237 // Type aliases for resolving response types in OpenAPI descriptors

238238 type_aliases: HashMap<String, ResolvedType>,

239239

240+ // CLI `#`-arg merged defaults: arg name → the concrete literal default any

241+ // scope supplies. A `_` (required) declaration whose name has a concrete

242+ // default elsewhere uses it instead of aborting when the flag is absent.

243+ cli_arg_defaults: HashMap<String, Option<String>>,

244+

240245 // Trampolines for user functions used as values: fn_name -> trampoline_func_id

241246 user_fn_trampolines: HashMap<String, FuncId>,

242247

@@ -1002,6 +1007,7 @@ fn compile_inner<M: cranelift_module::Module>(

10021007 .unwrap_or("knot");

10031008 cg.db_path = format!("{}.db", stem);

10041009 cg.source_schemas = type_env.source_schemas.clone();

1010+ cg.cli_arg_defaults = type_env.cli_arg_defaults.clone();

10051011 for (name, schema) in &type_env.source_schemas {

10061012 if schema.starts_with("_value:") {

10071013 cg.scalar_sources.insert(name.clone());

@@ -1722,6 +1728,7 @@ impl<M: cranelift_module::Module> Codegen<M> {

17221728 route_entries: HashMap::new(),

17231729 fetch_route_entries: HashMap::new(),

17241730 type_aliases: HashMap::new(),

1731+ cli_arg_defaults: HashMap::new(),

17251732 user_fn_trampolines: HashMap::new(),

17261733 capture_env_in_progress: std::collections::HashSet::new(),

17271734 relational_do_spans: HashSet::new(),

@@ -1939,6 +1946,7 @@ impl<M: cranelift_module::Module> Codegen<M> {

19391946

19401947 // CLI constant overrides

19411948 self.declare_rt("knot_override_lookup", &[p, p, types::I32], &[p]);

1949+ self.declare_rt("knot_override_forbid", &[p, p], &[p]);

19421950 self.declare_rt("knot_override_lookup_ex", &[p, p, p, p], &[p]);

19431951 self.declare_rt("knot_override_unwrap_just", &[p], &[p]);

19441952 self.declare_rt("knot_override_check_refined", &[p, p, p, p, p, p], &[]);

@@ -6666,30 +6674,82 @@ ast::ExprKind::Atomic(inner) => {

66666674 builder.block_params(merge_block)[0]

66676675 }

66686676 None => {

6669− // Required: abort if the flag is missing.

6670− let missing_block = builder.create_block();

6671− let ok_block = builder.create_block();

6672− let is_null =

6673− builder.ins().icmp_imm(IntCC::Equal, override_val, 0);

6674− builder

6675− .ins()

6676− .brif(is_null, missing_block, &[], ok_block, &[]);

6677− builder.switch_to_block(missing_block);

6678− builder.seal_block(missing_block);

6679− let (msg_ptr, msg_len) = self.string_ptr(

6680− builder,

6681− &format!("missing required argument --{}", name),

6682− );

6683− let _ = self.call_rt(builder, "knot_todo", &[msg_ptr, msg_len]);

6684− builder.ins().jump(ok_block, &[]);

6685− builder.switch_to_block(ok_block);

6686− builder.seal_block(ok_block);

6687− override_val

6677+ // Required — UNLESS another scope declared this same

6678+ // `#name` with a concrete default (the agreement check

6679+ // merged it). Then the flag-or-default logic applies:

6680+ // the flag wins, else the merged default.

6681+ let merged_default = self

6682+ .cli_arg_defaults

6683+ .get(name.as_str())

6684+ .and_then(|d| d.as_ref())

6685+ .cloned();

6686+ match merged_default {

6687+ Some(lit_src) => {

6688+ // Parse the merged literal default into an

6689+ // expr and use the optional-arg default path.

6690+ let lexer = knot::lexer::Lexer::new(&lit_src);

6691+ let (tokens, _) = lexer.tokenize();

6692+ let parser =

6693+ knot::parser::Parser::new(lit_src.clone(), tokens);

6694+ let (dexpr, _) = parser.parse_file_expr();

6695+ let default_block = builder.create_block();

6696+ let override_block = builder.create_block();

6697+ let merge_block = builder.create_block();

6698+ builder.append_block_param(merge_block, self.ptr_type);

6699+ let is_null = builder

6700+ .ins()

6701+ .icmp_imm(IntCC::Equal, override_val, 0);

6702+ builder

6703+ .ins()

6704+ .brif(is_null, default_block, &[], override_block, &[]);

6705+

6706+ builder.switch_to_block(override_block);

6707+ builder.seal_block(override_block);

6708+ builder.ins().jump(merge_block, &[override_val.into()]);

6709+

6710+ builder.switch_to_block(default_block);

6711+ builder.seal_block(default_block);

6712+ let default_val =

6713+ self.compile_expr(builder, &dexpr, env, db);

6714+ builder.ins().jump(merge_block, &[default_val.into()]);

6715+

6716+ builder.switch_to_block(merge_block);

6717+ builder.seal_block(merge_block);

6718+ builder.block_params(merge_block)[0]

6719+ }

6720+ None => {

6721+ // Truly required: abort if the flag is missing.

6722+ let missing_block = builder.create_block();

6723+ let ok_block = builder.create_block();

6724+ let is_null = builder

6725+ .ins()

6726+ .icmp_imm(IntCC::Equal, override_val, 0);

6727+ builder

6728+ .ins()

6729+ .brif(is_null, missing_block, &[], ok_block, &[]);

6730+ builder.switch_to_block(missing_block);

6731+ builder.seal_block(missing_block);

6732+ let (msg_ptr, msg_len) = self.string_ptr(

6733+ builder,

6734+ &format!("missing required argument --{}", name),

6735+ );

6736+ let _ =

6737+ self.call_rt(builder, "knot_todo", &[msg_ptr, msg_len]);

6738+ builder.ins().jump(ok_block, &[]);

6739+ builder.switch_to_block(ok_block);

6740+ builder.seal_block(ok_block);

6741+ override_val

6742+ }

6743+ }

66886744 }

66896745 }

66906746 } else {

6691− // Non-overridable type — no CLI parser. Fall through to the

6692− // default (or error for required args).

6747+ // Non-overridable type — no CLI parser. Abort loudly if the

6748+ // flag was passed (rather than silently using the default),

6749+ // then fall through to the default (or error for required

6750+ // args).

6751+ let (name_ptr, name_len) = self.string_ptr(builder, name.as_str());

6752+ self.call_rt(builder, "knot_override_forbid", &[name_ptr, name_len]);

66936753 match default {

66946754 Some(d) => self.compile_expr(builder, d.as_ref(), env, db),

66956755 None => {

crates/knot-compiler/src/main.rs+2−1

@@ -626,7 +626,7 @@ fn cmd_build(

626626

627627 // Reject `with`-chain shadowing: a field name bound by two layers of the

628628 // top-level `with {…} (with {…} …)` chain silently shadows the outer one.

629− let with_shadow_diags = types::check_with_chain_shadowing(&program);

629+ let (with_shadow_diags, cli_arg_defaults) = types::check_with_chain_shadowing(&program);

630630 if !with_shadow_diags.is_empty() {

631631 for diag in &with_shadow_diags {

632632 eprintln!("{}", diag.render(&source, &filename));

@@ -636,6 +636,7 @@ fn cmd_build(

636636

637637 // Resolve types

638638 let mut type_env = types::TypeEnv::from_program(&program);

639+ type_env.cli_arg_defaults = cli_arg_defaults;

639640

640641 // Type inference

641642 let infer::CheckOutput {

crates/knot-compiler/src/types.rs+21−2

@@ -131,6 +131,11 @@ pub struct TypeEnv {

131131 pub source_refinements: HashMap<String, Vec<(Option<String>, String, Expr)>>,

132132 /// Refined type aliases: type_name -> predicate expr

133133 pub refined_types: HashMap<String, Expr>,

134+ /// CLI `#`-arg merged defaults: arg name → the concrete literal default any

135+ /// scope supplies (`None` if every declaration is `_`/required). Computed by

136+ /// `check_with_chain_shadowing`; codegen reads it so a `_` declaration with

137+ /// a concrete default elsewhere doesn't emit the missing-argument abort.

138+ pub cli_arg_defaults: HashMap<String, Option<String>>,

134139 /// Multi-variant parameterized data types: name -> (param names,

135140 /// per-constructor raw field types). Used to substitute type arguments

136141 /// at application sites.

@@ -380,6 +385,7 @@ impl TypeEnv {

380385 subset_constraints,

381386 source_refinements: HashMap::new(),

382387 refined_types,

388+ cli_arg_defaults: HashMap::new(),

383389 multi_variant_params,

384390 single_variant_params,

385391 }

@@ -710,7 +716,12 @@ pub fn check_alias_cycles(program: &Expr) -> Vec<knot::diagnostic::Diagnostic> {

710716/// one layer. This catches what the per-scope `bind_at` check cannot: the

711717/// declaration record is the INNERMOST `with`'s record, so an outer layer's

712718/// same-named field is a separate binding that no enclosing-scope check sees.

713−pub fn check_with_chain_shadowing(program: &Expr) -> Vec<knot::diagnostic::Diagnostic> {

719+pub fn check_with_chain_shadowing(

720+ program: &Expr,

721+) -> (

722+ Vec<knot::diagnostic::Diagnostic>,

723+ HashMap<String, Option<String>>,

724+) {

714725 use knot::ast::ExprKind;

715726 let mut diags = Vec::new();

716727 // (name, span) of every field bound by an OUTER layer, in chain order.

@@ -775,7 +786,15 @@ pub fn check_with_chain_shadowing(program: &Expr) -> Vec<knot::diagnostic::Diagn

775786 _ => break,

776787 }

777788 }

778− diags

789+ // Surface the merged CLI-arg defaults: name → the concrete literal default

790+ // if any scope supplies one (`None` if every declaration is `_`/required).

791+ // Codegen uses this so a `_` declaration with a concrete default elsewhere

792+ // doesn't emit the missing-argument abort.

793+ let arg_defaults: HashMap<String, Option<String>> = cli_args

794+ .into_iter()

795+ .map(|(name, (_ty, default, _span))| (name, default))

796+ .collect();

797+ (diags, arg_defaults)

779798}

780799

781800/// Record a CLI `#arg` declaration and check it against prior declarations of

crates/knot-jit/tests/cli_overrides.rs+122

@@ -675,4 +675,126 @@ Port #port (4 * 2020)

675675 yield {}))"#,

676676 "must be a literal",

677677 );

678+}

679+

680+// ── Gap closures ────────────────────────────────────────────────────────────

681+

682+#[test]

683+fn non_settable_arg_with_flag_aborts() {

684+ // A record `#`-arg isn't CLI-settable. Passing the flag must abort loudly,

685+ // not silently fall through to the default.

686+ let dir = TempDir::fresh("record_forbid");

687+ build(

688+ &dir,

689+ "record_forbid",

690+ r#"with {

691+Cfg {name Text}

692+Cfg #cfg {name "default"}

693+}

694+(|

695+ base.println cfg.name

696+ yield {})"#,

697+ );

698+ // No flag: the default applies.

699+ let (out, err, code) = run_with_args(&dir, "record_forbid", &[]);

700+ assert_eq!(code, 0, "stderr: {err}");

701+ assert_eq!(out.trim_end(), "\"default\"\n{}");

702+ // Flag present: loud error, not a silent no-op.

703+ let (_, err, code) =

704+ run_with_args(&dir, "record_forbid", &["--cfg={name \"x\"}"]);

705+ assert_ne!(code, 0, "expected a loud error, stderr: {err}");

706+ assert!(

707+ err.contains("cannot be set from the command line"),

708+ "stderr: {err}"

709+ );

710+}

711+

712+#[test]

713+fn collection_of_non_scalar_with_flag_aborts() {

714+ // `Vec Mode` (a collection of a non-scalar element) isn't CLI-settable. A

715+ // passed flag must abort loudly.

716+ let dir = TempDir::fresh("vec_enum_forbid");

717+ build(

718+ &dir,

719+ "vec_enum_forbid",

720+ r#"with {

721+Mode Dev {} Prod {}

722+Vec Mode #modes (base.the (Vec Mode) [])

723+}

724+(|

725+ base.println (base.show modes)

726+ yield {})"#,

727+ );

728+ let (_, err, code) = run_with_args(&dir, "vec_enum_forbid", &["--modes=Dev"]);

729+ assert_ne!(code, 0, "expected a loud error, stderr: {err}");

730+ assert!(

731+ err.contains("cannot be set from the command line"),

732+ "stderr: {err}"

733+ );

734+}

735+

736+#[test]

737+fn required_underscore_uses_sibling_default() {

738+ // `#port _` (required) in one scope + `#port 8080` (concrete default) in

739+ // another: the concrete default wins when the flag is absent, in both

740+ // declaration orders. The flag still wins when present.

741+ for (name, src) in [

742+ (

743+ "uscore_outer",

744+ r#"with {

745+Port Int 1

746+Port #port _

747+}

748+(with {

749+Port #port 8080

750+}

751+(|

752+ base.println port

753+ yield {}))"#,

754+ ),

755+ (

756+ "uscore_inner",

757+ r#"with {

758+Port Int 1

759+Port #port 8080

760+}

761+(with {

762+Port #port _

763+}

764+(|

765+ base.println port

766+ yield {}))"#,

767+ ),

768+ ] {

769+ let dir = TempDir::fresh(name);

770+ build(&dir, name, src);

771+ // No flag: the concrete default applies (not a missing-argument abort).

772+ let (out, err, code) = run_with_args(&dir, name, &[]);

773+ assert_eq!(code, 0, "{name}: stderr: {err}");

774+ assert_eq!(out.trim_end(), "8080\n{}", "{name}");

775+ // Flag present: the flag wins.

776+ let (out, err, code) = run_with_args(&dir, name, &["--port=9090"]);

777+ assert_eq!(code, 0, "{name}: stderr: {err}");

778+ assert_eq!(out.trim_end(), "9090\n{}", "{name}");

779+ }

780+}

781+

782+#[test]

783+fn truly_required_underscore_still_aborts() {

784+ // A `_` arg with NO concrete default anywhere stays required.

785+ let dir = TempDir::fresh("uscore_required");

786+ build(

787+ &dir,

788+ "uscore_required",

789+ r#"with {

790+Port Int 1

791+Port #port _

792+}

793+(|

794+ base.println port

795+ yield {})"#,

796+ );

797+ let (_, err, code) = run_with_args(&dir, "uscore_required", &[]);

798+ assert_ne!(code, 0, "expected a missing-argument abort, stderr: {err}");

799+ assert!(err.contains("missing required argument"), "stderr: {err}");

678800}

crates/knot-runtime/src/lib.rs+18−2

@@ -3551,8 +3551,7 @@ fn debug_sql_params(sql: &str, params: &[rusqlite::types::Value]) {

35513551/// scalar overrides only the last occurrence is meaningful (last-wins,

35523552/// matching the previous single-slot behavior). `is_bool` enables the

35533553/// bare-flag-means-true two-arg disambiguation.

3554−fn override_flag_values(name: &str, is_bool: bool) -> Vec<String> {

3555− let flag = format!("--{}", name);

3554+fn override_flag_values(name: &str, is_bool: bool) -> Vec<String> { let flag = format!("--{}", name);

35563555 // Use args_os and skip non-UTF-8 entries: std::env::args() panics on

35573556 // non-UTF-8 argv, which would crash before any override lookup runs.

35583557 let args: Vec<String> = std::env::args_os()

@@ -3611,6 +3610,23 @@ fn override_parse_error(name: &str, val: &str, expected: &str) -> ! {

36113610/// Scans `std::env::args()` for `--{name}=value` or `--{name} value`.

36123611/// `type_tag`: 0=Int, 1=Float, 2=Text, 3=Bool, 4=Maybe Int, 5=Maybe Float, 6=Maybe Text, 7=Maybe Bool.

36133612/// Maybe tags automatically wrap the parsed value in `Just`.

3613+/// Forbid a CLI flag for a `#`-argument whose type isn't CLI-settable (a

3614+/// record, a function, a collection of non-scalars). When such an arg has a

3615+/// default, codegen routes here so a passed flag aborts loudly instead of

3616+/// silently falling through to the default. No-op when the flag is absent.

3617+#[unsafe(no_mangle)]

3618+pub extern "C-unwind" fn knot_override_forbid(name_ptr: *const u8, name_len: usize) -> *mut Value {

3619+ let name = unsafe { str_from_raw(name_ptr, name_len) };

3620+ if !override_flag_values(name, false).is_empty() {

3621+ eprintln!(

3622+ "Error: argument --{} has a type that cannot be set from the command line",

3623+ name

3624+ );

3625+ std::process::exit(1);

3626+ }

3627+ alloc(Value::Unit)

3628+}

3629+

36143630/// Returns a boxed `Value` if found, or null if absent.

36153631/// On parse error, prints a message and exits.

36163632#[unsafe(no_mangle)]