diff --git a/README.md b/README.md index a229b11..ddd85d1 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,27 @@ arc run --cell 'top(pitch * 4., -width / 2.)' arc run --cell 'array(cons(250., cons(350., [])), Mode::Fast)' ``` +Parameters declared with a default value are keyword parameters. They are +passed by name, may be omitted, and must follow the positional parameters. A +default is an ordinary expression evaluated at each call; it may refer to the +parameters declared before it and to module-level items, and its type must match +the declared type exactly: + +```rust +cell via(layer: String, w: Float, h: Float = w, n: Int = 1) { + // ... +} + +cell top() { + let square = inst(via("met1", 100.)); + let stack = inst(via("met1", 100., h=300., n=3)); +} +``` + +Positional parameters cannot be passed by name, and keyword parameters cannot be +passed positionally. Keyword arguments also work in cell invocations: +`arc run --cell 'via("met1", 100., n=3)'`. + GDS imports are zero-argument cells. A module-qualified entry such as `"macros::sram"` can be referenced as `lib::macros::sram()` or imported with `use lib::macros::sram;`. Paths in the manifest are relative to `Argon.toml`, diff --git a/crates/compiler/src/ast/annotated.rs b/crates/compiler/src/ast/annotated.rs index 46c772c..55b190a 100644 --- a/crates/compiler/src/ast/annotated.rs +++ b/crates/compiler/src/ast/annotated.rs @@ -303,6 +303,7 @@ impl AstTransformer for AstAnnotationPass { input: &super::ArgDecl, _name: &super::Ident, _ty: &super::TySpec, + _default: &Option>, ) -> ::ArgDecl { input.metadata.clone() } diff --git a/crates/compiler/src/ast/mod.rs b/crates/compiler/src/ast/mod.rs index 78e7ef5..e07e5e9 100644 --- a/crates/compiler/src/ast/mod.rs +++ b/crates/compiler/src/ast/mod.rs @@ -366,10 +366,12 @@ pub struct KwArgValue { pub metadata: T::KwArgValue, } +/// A parameter of a cell or function; `default` makes it a keyword parameter. #[derive_where(Debug, Clone, Serialize, Deserialize; S)] pub struct ArgDecl { pub name: Ident, pub ty: TySpec, + pub default: Option>, pub metadata: T::ArgDecl, } @@ -580,6 +582,7 @@ pub trait AstTransformer { input: &ArgDecl, name: &Ident, ty: &TySpec, + default: &Option>, ) -> ::ArgDecl; fn dispatch_scope( &mut self, @@ -972,8 +975,19 @@ pub trait AstTransformer { ) -> ArgDecl { let name = self.transform_ident(&input.name); let ty = self.transform_ty_spec(&input.ty); - let metadata = self.dispatch_arg_decl(input, &name, &ty); - ArgDecl { name, ty, metadata } + // The default is visited before the parameter is dispatched, so it can + // see earlier parameters but not this one. + let default = input + .default + .as_ref() + .map(|default| self.transform_expr(default)); + let metadata = self.dispatch_arg_decl(input, &name, &ty, &default); + ArgDecl { + name, + ty, + default, + metadata, + } } fn transform_scope( diff --git a/crates/compiler/src/cli.rs b/crates/compiler/src/cli.rs index 663ddf5..300ce62 100644 --- a/crates/compiler/src/cli.rs +++ b/crates/compiler/src/cli.rs @@ -596,6 +596,20 @@ mod tests { assert_eq!(rect.x1.0, 20.); } + #[test] + fn execution_accepts_keyword_cell_arguments() { + let source = temp_source( + "kwargs", + "cell top(w: Float = 100., h: Float = w) {\n\ + let r = rect(\"met1\", x0=0., y0=0., x1=w, y1=h);\n\ + }\n", + ); + let rect = compiled_rect("kwargs-default", source.clone(), "top()"); + assert_eq!((rect.x1.0, rect.y1.0), (100., 100.)); + let rect = compiled_rect("kwargs-explicit", source, "top(w=300.)"); + assert_eq!((rect.x1.0, rect.y1.0), (300., 300.)); + } + #[test] fn out_of_range_cell_argument_is_reported_cleanly() { let source = temp_source("out-of-range-arg", "cell top(n: Int) {}\n"); diff --git a/crates/compiler/src/compile.rs b/crates/compiler/src/compile.rs index 4c45b05..ab2276e 100644 --- a/crates/compiler/src/compile.rs +++ b/crates/compiler/src/compile.rs @@ -771,6 +771,7 @@ impl<'a> AstTransformer for ImportPass<'a> { _input: &ArgDecl, _name: &Ident, _ty: &TySpec, + _default: &Option>, ) -> ::ArgDecl { } @@ -1248,13 +1249,8 @@ impl std::fmt::Display for Ty { Ty::SeqNil => write!(f, "[]"), Ty::Cell(cell) => write!(f, "Cell({})", cell.name), Ty::Inst(cell) => write!(f, "Inst({})", cell.name), - Ty::CellFn(cell_fn) => write!( - f, - "cell {}({})", - cell_fn.cell.name, - cell_fn.args.iter().format(", ") - ), - Ty::Fn(func) => write!(f, "fn({}) -> {}", func.args.iter().format(", "), func.ret), + Ty::CellFn(cell_fn) => write!(f, "cell {}({})", cell_fn.cell.name, cell_fn.sig), + Ty::Fn(func) => write!(f, "fn({}) -> {}", func.sig, func.ret), // The name is what distinguishes two same-shaped enums: `EnumTy` // equality keys on `id`, so without it a mismatch renders as // `expected enum {A, B}, found enum {A, B}`. @@ -1354,22 +1350,122 @@ impl Ty { } } +/// The parameters a call must supply: positional types in order, then keyword +/// parameters by name. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Signature { + pub(crate) args: Vec, + pub(crate) kwargs: IndexMap, +} + +impl Signature { + /// A signature with only positional parameters. + fn positional(args: impl IntoIterator) -> Self { + Self { + args: args.into_iter().collect(), + kwargs: IndexMap::new(), + } + } + + /// Adds keyword parameters. + fn keywords<'s>(mut self, kwargs: impl IntoIterator) -> Self { + self.kwargs + .extend(kwargs.into_iter().map(|(name, ty)| (name.to_owned(), ty))); + self + } +} + +/// Builds a signature from typed parameter declarations: parameters without a +/// default are positional, the rest are keyword parameters. +impl<'a, M: AstMetadata> FromIterator<(&'a ArgDecl, Ty)> for Signature { + fn from_iter, Ty)>>(params: I) -> Self { + let mut sig = Self::default(); + for (arg, ty) in params { + match arg.default { + Some(_) => { + sig.kwargs.insert(arg.name.name.to_string(), ty); + } + None => sig.args.push(ty), + } + } + sig + } +} + +impl std::fmt::Display for Signature { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let args = self.args.iter().map(ToString::to_string); + let kwargs = self.kwargs.iter().map(|(name, ty)| format!("{name}: {ty}")); + write!(f, "{}", args.chain(kwargs).format(", ")) + } +} + +/// Signatures of the builtins whose parameters do not depend on the call. +/// Built once rather than per call site, since every keyword name is owned. +mod builtin_sig { + use std::sync::LazyLock; + + use super::{Signature, Ty}; + + /// The coordinate keywords every rectangle constructor accepts. + fn coordinates() -> impl Iterator { + ["x0", "x1", "y0", "y1", "x0i", "x1i", "y0i", "y1i", "w", "h"] + .into_iter() + .map(|name| (name, Ty::Float)) + } + + pub(super) static CRECT: LazyLock = LazyLock::new(|| { + Signature::default().keywords(coordinates().chain([("layer", Ty::String)])) + }); + pub(super) static RECT: LazyLock = + LazyLock::new(|| Signature::positional([Ty::String]).keywords(coordinates())); + pub(super) static TEXT: LazyLock = + LazyLock::new(|| Signature::positional([Ty::String, Ty::String, Ty::Float, Ty::Float])); + pub(super) static RANGE_FULL: LazyLock = + LazyLock::new(|| Signature::positional([Ty::Int, Ty::Int, Ty::Int])); + pub(super) static EQ: LazyLock = + LazyLock::new(|| Signature::positional([Ty::Float, Ty::Float])); + pub(super) static DIMENSION: LazyLock = LazyLock::new(|| { + Signature::positional([ + Ty::Float, + Ty::Float, + Ty::Float, + Ty::Float, + Ty::Float, + Ty::Float, + Ty::Bool, + ]) + }); + /// The single positional parameter is a cell, which has no nameable type; + /// `Ty::Any` checks the arity and leaves the category to + /// `assert_ty_is_cell`. + pub(super) static INST: LazyLock = LazyLock::new(|| { + Signature::positional([Ty::Any]).keywords([ + ("reflect", Ty::Bool), + ("angle", Ty::Int), + ("x", Ty::Float), + ("y", Ty::Float), + ("xi", Ty::Float), + ("yi", Ty::Float), + ("construction", Ty::Bool), + ]) + }); + /// A builtin that takes no arguments, and the empty keyword set that + /// builtins with variadic positional arguments check against. + pub(super) static NONE: LazyLock = LazyLock::new(Signature::default); +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct FnTy { - pub(crate) args: Vec, + pub(crate) sig: Signature, pub(crate) ret: Ty, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CellFnTy { - args: Vec, - /// The structural type produced when this cell function is called. - /// - /// Stored behind an `Arc` so that every caller (and every `inst` of the - /// resulting cell) shares one allocation instead of deep-copying it. This - /// keeps the type representation a DAG rather than a tree: a cell that - /// references a child twice embeds two `Arc`s to the *same* `CellTy`, so - /// type size stays linear in hierarchy depth instead of doubling per level. + sig: Signature, + /// The structural type produced when this cell function is called, shared + /// with every caller and every `inst` of the result. pub(crate) cell: Arc, } @@ -1647,16 +1743,17 @@ impl<'a> VarIdTyPass<'a> { kind: StaticErrorKind::RedeclarationOfBuiltin, }); } - let args: Vec<_> = input + self.check_params(&input.args); + let sig = input .args .iter() .map(|arg| { let ty_spec = self.transform_ty_spec(&arg.ty); - self.ty_from_spec(&ty_spec) + (arg, self.ty_from_spec(&ty_spec)) }) .collect(); let ty = Ty::Fn(Box::new(FnTy { - args, + sig, ret: if let Some(return_ty) = &input.return_ty { self.ty_from_spec(return_ty) } else { @@ -1957,33 +2054,26 @@ impl<'a> VarIdTyPass<'a> { fn typecheck_kwargs( &mut self, kwargs: &[KwArgValue], - kwarg_defs: IndexMap<&str, Ty>, + defs: &IndexMap, ) { - let mut defined = IndexSet::new(); + let mut seen = IndexSet::new(); for kwarg in kwargs { - let mut cont = false; - if !kwarg_defs.contains_key(&kwarg.name.name.as_str()) { + let name = kwarg.name.name.as_str(); + let Some(expected) = defs.get(name) else { self.errors.push(StaticError { span: self.span(kwarg.name.span), kind: StaticErrorKind::InvalidKwArg, }); - cont = true; - } - if defined.contains(&&kwarg.name.name) { + continue; + }; + if !seen.insert(name) { self.errors.push(StaticError { span: self.span(kwarg.name.span), kind: StaticErrorKind::DuplicateKwArg, }); - cont = true; - } - defined.insert(&kwarg.name.name); - if !cont { - self.assert_eq_ty( - kwarg.value.span(), - &kwarg.value.ty(), - kwarg_defs.get(&kwarg.name.name.as_str()).unwrap(), - ); + continue; } + self.assert_eq_ty(kwarg.value.span(), &kwarg.value.ty(), expected); } } @@ -1999,15 +2089,40 @@ impl<'a> VarIdTyPass<'a> { } } + /// Checks a call's arguments against the callee's signature. fn typecheck_args( &mut self, call_span: cfgrammar::Span, args: &crate::ast::Args, - arg_defs: &[Ty], - kwarg_defs: IndexMap<&str, Ty>, + sig: &Signature, ) { - self.typecheck_posargs(call_span, &args.posargs, arg_defs); - self.typecheck_kwargs(&args.kwargs, kwarg_defs); + self.typecheck_posargs(call_span, &args.posargs, &sig.args); + self.typecheck_kwargs(&args.kwargs, &sig.kwargs); + } + + /// Rejects repeated parameter names and positional parameters declared + /// after keyword parameters. + fn check_params(&mut self, args: &[ArgDecl]) { + let mut seen = IndexSet::new(); + let mut keyword_seen = false; + for arg in args { + if !seen.insert(arg.name.name.as_str()) { + self.errors.push(StaticError { + span: self.span(arg.name.span), + kind: StaticErrorKind::DuplicateNameDeclaration, + }); + } + match arg.default { + Some(_) => keyword_seen = true, + None if keyword_seen => self.errors.push(StaticError { + span: self.span(arg.name.span), + kind: StaticErrorKind::PositionalParamAfterDefault { + name: arg.name.name.to_string(), + }, + }), + None => {} + } + } } fn typecheck_call( @@ -2021,11 +2136,11 @@ impl<'a> VarIdTyPass<'a> { if let Some((varid, ty)) = lookup { match ty { Ty::Fn(ty) => { - self.typecheck_args(call_span, args, &ty.args, IndexMap::new()); + self.typecheck_args(call_span, args, &ty.sig); (Some(varid), ty.ret.clone()) } Ty::CellFn(ty) => { - self.typecheck_args(call_span, args, &ty.args, IndexMap::new()); + self.typecheck_args(call_span, args, &ty.sig); (Some(varid), Ty::Cell(ty.cell.clone())) } ty => { @@ -2242,6 +2357,7 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { kind: StaticErrorKind::RedeclarationOfBuiltin, }); } + self.check_params(&input.args); self.enter_scope(&input.scope); let args: Vec<_> = input .args @@ -2290,7 +2406,10 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { // field accesses on it fall through to the builtin geometry fields. let cell_id = self.alloc_id(); let ty = Ty::CellFn(Box::new(CellFnTy { - args: args.iter().map(|arg| arg.metadata.1.clone()).collect(), + sig: args + .iter() + .map(|arg| (arg, arg.metadata.1.clone())) + .collect(), cell: Arc::new(CellTy { name: self.qualified_name(&input.name.name), data, @@ -2652,94 +2771,44 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { if func.path.len() == 1 { match func.path[0].name.as_str() { name @ "crect" | name @ "rect" => { - let kwarg_defs = if name == "crect" { - self.typecheck_posargs(input.span, &args.posargs, &[]); - IndexMap::from_iter([ - ("x0", Ty::Float), - ("x1", Ty::Float), - ("y0", Ty::Float), - ("y1", Ty::Float), - ("x0i", Ty::Float), - ("x1i", Ty::Float), - ("y0i", Ty::Float), - ("y1i", Ty::Float), - ("w", Ty::Float), - ("h", Ty::Float), - ("layer", Ty::String), - ]) + let sig = if name == "crect" { + &builtin_sig::CRECT } else { - self.typecheck_posargs(input.span, &args.posargs, &[Ty::String]); - IndexMap::from_iter([ - ("x0", Ty::Float), - ("x1", Ty::Float), - ("y0", Ty::Float), - ("y1", Ty::Float), - ("x0i", Ty::Float), - ("x1i", Ty::Float), - ("y0i", Ty::Float), - ("y1i", Ty::Float), - ("w", Ty::Float), - ("h", Ty::Float), - ]) + &builtin_sig::RECT }; - self.typecheck_kwargs(&args.kwargs, kwarg_defs); + self.typecheck_args(input.span, args, sig); (None, Ty::Rect) } "polygon" => { - self.assert_eq_arity(input.span, args.posargs.len(), 2); - if let Some(layer) = args.posargs.first() { - self.assert_eq_ty(layer.span(), &layer.ty(), &Ty::String); - } - if let Some(points) = args.posargs.get(1) { - self.assert_eq_ty(points.span(), &points.ty(), &Ty::Int); - } - let kwarg_defs = args - .kwargs - .iter() - .filter_map(|kwarg| { - polygon_coordinate(kwarg.name.name.as_str()) - .map(|_| (kwarg.name.name.as_str(), Ty::Float)) - }) - .collect(); - self.typecheck_kwargs(&args.kwargs, kwarg_defs); + let coordinates = args.kwargs.iter().filter_map(|kwarg| { + let name = kwarg.name.name.as_str(); + polygon_coordinate(name).map(|_| (name, Ty::Float)) + }); + let sig = Signature::positional([Ty::String, Ty::Int]).keywords(coordinates); + self.typecheck_args(input.span, args, &sig); (None, Ty::Polygon) } "path" => { - self.assert_eq_arity(input.span, args.posargs.len(), 2); - if let Some(layer) = args.posargs.first() { - self.assert_eq_ty(layer.span(), &layer.ty(), &Ty::String); - } - if let Some(points) = args.posargs.get(1) { - self.assert_eq_ty(points.span(), &points.ty(), &Ty::Int); - } - let kwarg_defs = args - .kwargs - .iter() - .filter_map(|kwarg| { - let name = kwarg.name.name.as_str(); - (matches!( - name, - "width" - | "widthi" - | "begin_extension" - | "begin_extensioni" - | "end_extension" - | "end_extensioni" - ) || polygon_coordinate(name).is_some()) - .then_some((name, Ty::Float)) - }) - .collect(); - self.typecheck_kwargs(&args.kwargs, kwarg_defs); + let keywords = args.kwargs.iter().filter_map(|kwarg| { + let name = kwarg.name.name.as_str(); + (matches!( + name, + "width" + | "widthi" + | "begin_extension" + | "begin_extensioni" + | "end_extension" + | "end_extensioni" + ) || polygon_coordinate(name).is_some()) + .then_some((name, Ty::Float)) + }); + let sig = Signature::positional([Ty::String, Ty::Int]).keywords(keywords); + self.typecheck_args(input.span, args, &sig); (None, Ty::Path) } "text" => { // text, layer, x, y - self.typecheck_posargs( - input.span, - &args.posargs, - &[Ty::String, Ty::String, Ty::Float, Ty::Float], - ); - self.typecheck_kwargs(&args.kwargs, IndexMap::default()); + self.typecheck_args(input.span, args, &builtin_sig::TEXT); (None, Ty::Nil) } "cons" => { @@ -2762,7 +2831,7 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { } } "list" => { - self.typecheck_kwargs(&args.kwargs, IndexMap::default()); + self.typecheck_kwargs(&args.kwargs, &builtin_sig::NONE.kwargs); if args.posargs.is_empty() { self.errors.push(StaticError { span: self.span(input.span), @@ -2796,8 +2865,7 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { "range_full" => { // Native builtin backing `std::range`/`std::range_full`: builds the // whole `[Int]` in one pass instead of recursive `cons`. - self.typecheck_posargs(input.span, &args.posargs, &[Ty::Int, Ty::Int, Ty::Int]); - self.typecheck_kwargs(&args.kwargs, IndexMap::default()); + self.typecheck_args(input.span, args, &builtin_sig::RANGE_FULL); (None, Ty::Seq(Box::new(Ty::Int))) } "head" => { @@ -2867,44 +2935,19 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { (None, Ty::Rect) } "float" => { - self.typecheck_args(input.span, args, &[], IndexMap::new()); + self.typecheck_args(input.span, args, &builtin_sig::NONE); (None, Ty::Float) } "eq" => { - self.typecheck_args(input.span, args, &[Ty::Float, Ty::Float], IndexMap::new()); + self.typecheck_args(input.span, args, &builtin_sig::EQ); (None, Ty::Nil) } "dimension" => { - self.typecheck_args( - input.span, - args, - &[ - Ty::Float, - Ty::Float, - Ty::Float, - Ty::Float, - Ty::Float, - Ty::Float, - Ty::Bool, - ], - IndexMap::new(), - ); + self.typecheck_args(input.span, args, &builtin_sig::DIMENSION); (None, Ty::Nil) } "inst" => { - self.assert_eq_arity(input.span, args.posargs.len(), 1); - self.typecheck_kwargs( - &args.kwargs, - IndexMap::from_iter([ - ("reflect", Ty::Bool), - ("angle", Ty::Int), - ("x", Ty::Float), - ("y", Ty::Float), - ("xi", Ty::Float), - ("yi", Ty::Float), - ("construction", Ty::Bool), - ]), - ); + self.typecheck_args(input.span, args, &builtin_sig::INST); if let Some(ty) = args.posargs.first() { self.assert_ty_is_cell(ty.span(), &ty.ty()); match ty.ty() { @@ -3013,8 +3056,12 @@ impl<'a> AstTransformer for VarIdTyPass<'a> { input: &ArgDecl, _name: &Ident, _ty: &TySpec, + default: &Option>, ) -> ::ArgDecl { let ty = self.ty_from_spec(&input.ty); + if let Some(default) = default { + self.assert_eq_ty(default.span(), &default.ty(), &ty); + } (self.alloc(&input.name.name, ty.clone()), ty) } @@ -5347,6 +5394,88 @@ impl<'a> ExecPass<'a> { .unwrap_or(self.nil_value) } + /// Creates an empty frame whose parent is the global frame. + fn new_call_frame(&mut self) -> FrameId { + let fid = self.frame_id(); + self.frames.insert( + fid, + Frame { + bindings: Default::default(), + parent: Some(self.global_frame), + }, + ); + fid + } + + /// Evaluates a call's explicit arguments in the caller's context: one slot + /// per parameter in declaration order, `None` where the default applies. + fn explicit_args( + &mut self, + loc: DynLoc, + call: &CallExpr, + params: &[ArgDecl], + ) -> Vec> { + params + .iter() + .enumerate() + .map(|(index, param)| { + let arg = match call.args.posargs.get(index) { + Some(arg) => arg, + None => { + let kwarg = call + .args + .kwargs + .iter() + .find(|kwarg| kwarg.name.name == param.name.name)?; + &kwarg.value + } + }; + Some(self.visit_expr(loc, arg)) + }) + .collect() + } + + /// Binds every parameter in `loc.frame` and returns the bound values in + /// declaration order. A parameter without an explicit argument gets its + /// default, evaluated in a scope of its own under `loc.scope` once the + /// parameters before it are bound. + fn bind_args( + &mut self, + loc: DynLoc, + call_order: u64, + path: &FsPath, + params: &[ArgDecl], + explicit: Vec>, + ) -> Vec { + params + .iter() + .zip(explicit) + .map(|(param, explicit)| { + let value = match (explicit, ¶m.default) { + (Some(value), _) => value, + (None, Some(default)) => { + let scope = self.create_exec_scope_at_loc( + loc, + format!("{call_order} default {}", param.name.name), + Span { + path: path.to_path_buf(), + span: default.span(), + }, + ); + self.visit_expr(DynLoc { scope, ..loc }, default) + } + (None, None) => unreachable!("a parameter without an argument has a default"), + }; + self.frames + .get_mut(&loc.frame) + .unwrap() + .bindings + .insert(param.metadata.0, value); + value + }) + .collect() + } + fn new_ready_value(&mut self, val: Value) -> ValueId { let vid = self.value_id(); self.values.insert(vid, Defer::Ready(val)); @@ -5424,33 +5553,19 @@ impl<'a> ExecPass<'a> { })) }) } else { - let arg_vals = c - .args - .posargs - .iter() - .map(|arg| self.visit_expr(loc, arg)) - .collect_vec(); - let val = &self.values[&self + let callee = self .lookup( loc.frame, c.metadata .0 .expect("no var ID assigned to function being called"), ) - .unwrap()] - .as_ref() - .unwrap_ready() - .as_ref(); - match val { + .unwrap(); + match self.values[&callee].as_ref().unwrap_ready().as_ref() { ValueRef::Fn(val) => { - let mut call_frame = Frame { - bindings: Default::default(), - parent: Some(self.global_frame), - }; - for (arg_val, arg_decl) in arg_vals.iter().zip(&val.args) { - call_frame.bindings.insert(arg_decl.metadata.0, *arg_val); - } - let new_scope = val.scope.clone(); + let (params, body, path) = + (val.args.clone(), val.scope.clone(), val.metadata.0.clone()); + let explicit = self.explicit_args(loc, c, ¶ms); let scope = self.create_exec_scope( loc.cell, loc.scope, @@ -5461,12 +5576,11 @@ impl<'a> ExecPass<'a> { c.func.path.iter().map(|ident| &ident.name).join("::") ), Span { - path: val.metadata.0.clone(), - span: val.scope.span, + path: path.clone(), + span: body.span, }, ); - let fid = self.frame_id(); - self.frames.insert(fid, call_frame); + let fid = self.new_call_frame(); // A `fn` body is inlined here and now, unlike an // `if`/`match` branch, so a recursive call that is // not inside one descends natively with no @@ -5483,25 +5597,34 @@ impl<'a> ExecPass<'a> { }); return self.nil_value; } - let value = - self.visit_scope_expr_inner(loc.cell, fid, scope, &new_scope); + let callee_loc = DynLoc { + cell: loc.cell, + frame: fid, + scope, + seq_num: SeqNum::new(), + }; + self.bind_args(callee_loc, c.scope_order, &path, ¶ms, explicit); + let value = self.visit_scope_expr_inner(loc.cell, fid, scope, &body); self.eval_depth -= 1; value } - ValueRef::CellFn(_) => self.new_deferred_value(loc, |this| { - PartialEvalState::Call(Box::new(PartialCallExpr { - expr: c.clone(), - state: CallExprState { - posargs: arg_vals, - kwargs: c - .args - .kwargs - .iter() - .map(|arg| this.visit_expr(loc, &arg.value)) - .collect(), - }, - })) - }), + ValueRef::CellFn(val) => { + let (params, path) = (val.args.clone(), val.metadata.0.clone()); + let explicit = self.explicit_args(loc, c, ¶ms); + let fid = self.new_call_frame(); + let callee_loc = DynLoc { frame: fid, ..loc }; + let posargs = + self.bind_args(callee_loc, c.scope_order, &path, ¶ms, explicit); + self.new_deferred_value(loc, |_| { + PartialEvalState::Call(Box::new(PartialCallExpr { + expr: c.clone(), + state: CallExprState { + posargs, + kwargs: Vec::new(), + }, + })) + }) + } _ => { self.errors.push(ExecError { span: Some(self.span(&loc, c.span)), diff --git a/crates/compiler/src/compile/result.rs b/crates/compiler/src/compile/result.rs index 6baf59a..882014f 100644 --- a/crates/compiler/src/compile/result.rs +++ b/crates/compiler/src/compile/result.rs @@ -135,6 +135,9 @@ pub enum StaticErrorKind { /// A call supplies the same keyword argument more than once. #[error("duplicate keyword argument")] DuplicateKwArg, + /// A parameter without a default is declared after a parameter with one. + #[error("positional parameter `{name}` cannot follow a parameter with a default value")] + PositionalParamAfterDefault { name: String }, /// An identifier was used without being declared in the current scope. #[error("`{name}` is not declared in this scope")] UndeclaredVar { name: String }, diff --git a/crates/compiler/src/fingerprint.rs b/crates/compiler/src/fingerprint.rs index afd8162..f5062af 100644 --- a/crates/compiler/src/fingerprint.rs +++ b/crates/compiler/src/fingerprint.rs @@ -291,11 +291,13 @@ impl Builder { } } - /// A parameter's dependencies come from its *checked* type, not its - /// written one: a `TySpec`'s identifier carries no metadata, so the - /// declaration it names is only recoverable from `ArgDecl`'s `Ty`. + /// A parameter depends on the declarations its checked type and its + /// default value refer to. fn arg_decl(&self, arg: &ArgDecl, out: &mut IndexSet) { self.ty(&arg.metadata.1, out); + if let Some(default) = &arg.default { + self.expr(default, out); + } } /// Collects the declarations a checked type refers to. @@ -329,7 +331,7 @@ impl Builder { } } Ty::Fn(fn_ty) => { - for arg in &fn_ty.args { + for arg in fn_ty.sig.args.iter().chain(fn_ty.sig.kwargs.values()) { self.ty(arg, out); } self.ty(&fn_ty.ret, out); @@ -759,6 +761,30 @@ cell uses_unrelated() { let r = rect(\"met1\", x0 = unrelated(), y0 = 0., x1 = 1 ); } + #[test] + fn a_default_value_is_part_of_the_fingerprint() { + const SOURCE: &str = "\ +fn helper() -> Float { 1. } +fn scaled(x: Float, scale: Float = helper()) -> Float { x * scale } +cell uses_scaled() { let r = rect(\"met1\", x0 = scaled(1.), y0 = 0., x1 = 1., y1 = 1.); } +"; + assert_eq!( + changed( + SOURCE, + &SOURCE.replace("scale: Float = helper()", "scale: Float = 2. * helper()") + ), + ["scaled", "uses_scaled"] + ); + // A declaration a default calls is a dependency of the declaring fn. + assert_eq!( + changed( + SOURCE, + &SOURCE.replace("fn helper() -> Float { 1. }", "fn helper() -> Float { 9. }") + ), + ["helper", "scaled", "uses_scaled"] + ); + } + /// Text that moves without changing keeps its fingerprint. This is what /// lets an edit reuse declarations that merely shifted down the file. #[test] diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index dd197b6..f364d88 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -212,6 +212,8 @@ mod tests { const ARGON_PRECEDENCE: &str = concatcp!(EXAMPLES_DIR, "/precedence/lib.ar"); const ARGON_POLYGON: &str = concatcp!(EXAMPLES_DIR, "/polygon/lib.ar"); const ARGON_PATH: &str = concatcp!(EXAMPLES_DIR, "/path/lib.ar"); + const ARGON_KWARGS_FN: &str = concatcp!(EXAMPLES_DIR, "/kwargs_fn/lib.ar"); + const ARGON_KWARGS_CELL: &str = concatcp!(EXAMPLES_DIR, "/kwargs_cell/lib.ar"); // --------------------------------------------------------------------- // Scaling / stress benchmarks. @@ -1620,6 +1622,89 @@ mod tests { cells.unwrap_valid(); } + #[test] + fn argon_kwargs_fn() { + let o = parse_workspace_with_std(ARGON_KWARGS_FN); + assert!(o.static_errors().is_empty()); + let cells = compile( + &o.ast(), + CompileInput { + cell: &["top"], + args: Vec::new(), + }, + ) + .unwrap_valid(); + let top = &cells.cells[&cells.top]; + // Every rect's `x1` is a call with omitted, explicit, or + // parameter-referencing defaults, keyed by its `y0`. + let mut widths: Vec<(f64, f64)> = top + .objects + .values() + .filter_map(|object| object.get_rect()) + .map(|rect| (rect.y0.0, rect.x1.0)) + .collect(); + widths.sort_by(|a, b| a.0.total_cmp(&b.0)); + let expected = [ + (0., 30.), + (20., 40.), + (40., 20.), + (60., 10.), + (80., 10.), + (100., 50.), + ]; + assert_eq!(widths.len(), expected.len()); + for ((y0, x1), (expected_y0, expected_x1)) in widths.iter().zip(expected) { + assert_relative_eq!(*y0, expected_y0, epsilon = EPSILON); + assert_relative_eq!(*x1, expected_x1, epsilon = EPSILON); + } + } + + #[test] + fn argon_kwargs_cell() { + let o = parse_workspace_with_std(ARGON_KWARGS_CELL); + assert!(o.static_errors().is_empty()); + // The positional API supplies every parameter, keyword ones included. + let cells = compile( + &o.ast(), + CompileInput { + cell: &["top"], + args: vec![CellArg::Float(100.)], + }, + ) + .unwrap_valid(); + let top = &cells.cells[&cells.top]; + let mut insts: Vec<_> = top + .objects + .values() + .filter_map(|object| object.get_instance()) + .collect(); + insts.sort_by(|a, b| a.x.total_cmp(&b.x)); + let [square, tall, same, met2] = insts.as_slice() else { + panic!("expected four instances, got {}", insts.len()); + }; + assert_eq!(cells.cells.len(), 5); + let child_rect = |cell| { + cells.cells[&cell] + .objects + .values() + .find_map(|object| object.get_rect()) + .cloned() + .expect("child emits a rect") + }; + // `child(w)` and `child(w, h=w)` resolve to the same arguments. + for (inst, x1, y1, layer) in [ + (square, 100., 100., "met1"), + (tall, 100., 200., "met1"), + (same, 100., 100., "met1"), + (met2, 50., 50., "met2"), + ] { + let rect = child_rect(inst.cell); + assert_relative_eq!(rect.x1.0, x1, epsilon = EPSILON); + assert_relative_eq!(rect.y1.0, y1, epsilon = EPSILON); + assert_eq!(rect.layer.as_deref(), Some(layer)); + } + } + #[test] fn argon_library() { let o = parse_workspace_with_std(ARGON_LIBRARY); @@ -2471,6 +2556,102 @@ mod tests { output.errors } + /// Asserts that `source` reports an error accepted by `matches`. + #[track_caller] + fn assert_static_error(source: &str, matches: impl Fn(&StaticErrorKind) -> bool) { + let errors = static_errors(source); + assert!( + errors.iter().any(|error| matches(&error.kind)), + "{errors:?}" + ); + } + + #[test] + fn keyword_parameters_type_check() { + let errors = static_errors( + r#" + fn scaled(x: Float, scale: Float = 2., offset: Float = x) -> Float { + x * scale + offset + } + cell child(w: Float, h: Float = w, layer: String = "met1") { + let body = rect(layer, x0=0., y0=0., x1=w, y1=h); + } + cell top(n: Int = 1) { + let a = scaled(1.); + let b = scaled(1., offset=0., scale=3.); + let c = inst(child(10.)); + let d = inst(child(10., layer="met2", h=20.)); + } + "#, + ); + assert!(errors.is_empty(), "{errors:?}"); + } + + #[test] + fn keyword_parameter_declarations_are_checked() { + assert_static_error( + "fn f(a: Float, b: Float = 1., c: Float) -> Float { a }", + |kind| matches!(kind, StaticErrorKind::PositionalParamAfterDefault { name } if name == "c"), + ); + assert_static_error("fn f(a: Float, a: Int) -> Float { a }", |kind| { + matches!(kind, StaticErrorKind::DuplicateNameDeclaration) + }); + assert_static_error("cell c(w: Float, w: Float = 1.) {}", |kind| { + matches!(kind, StaticErrorKind::DuplicateNameDeclaration) + }); + assert_static_error("fn f(b: Float = 1) -> Float { b }", |kind| { + matches!( + kind, + StaticErrorKind::IncorrectTy { expected, found } if expected == "Float" && found == "Int" + ) + }); + // A default sees only the parameters before it. + assert_static_error( + "fn f(a: Float = b, b: Float = 1.) -> Float { a }", + |kind| matches!(kind, StaticErrorKind::UndeclaredVar { name } if name == "b"), + ); + assert_static_error( + "fn f(a: Float = a) -> Float { a }", + |kind| matches!(kind, StaticErrorKind::UndeclaredVar { name } if name == "a"), + ); + } + + #[test] + fn keyword_arguments_are_checked_at_calls() { + let call = |args: &str| { + format!( + "fn f(a: Float, b: Float = 1.) -> Float {{ a + b }}\ncell top() {{ let v = f({args}); }}" + ) + }; + // A keyword parameter cannot be passed positionally. + assert_static_error(&call("1., 2."), |kind| { + matches!( + kind, + StaticErrorKind::CallIncorrectPositionalArity { + expected: 1, + found: 2 + } + ) + }); + // A positional parameter cannot be passed by keyword. + assert_static_error(&call("a=1."), |kind| { + matches!(kind, StaticErrorKind::InvalidKwArg) + }); + assert_static_error(&call("1., zz=2."), |kind| { + matches!(kind, StaticErrorKind::InvalidKwArg) + }); + assert_static_error(&call("1., b=2., b=3."), |kind| { + matches!(kind, StaticErrorKind::DuplicateKwArg) + }); + assert_static_error(&call("1., b=true"), |kind| { + matches!( + kind, + StaticErrorKind::IncorrectTy { expected, found } if expected == "Float" && found == "Bool" + ) + }); + assert!(static_errors(&call("1., b=2.")).is_empty()); + } + /// Cell typing is structural. `CellTy` carries the declaring cell's `VarId` /// so that a field access can be navigated back to its `let`, and that id /// is deliberately excluded from `PartialEq`: if it were not, two cells diff --git a/crates/compiler/src/nav.rs b/crates/compiler/src/nav.rs index 203cabe..14f63e3 100644 --- a/crates/compiler/src/nav.rs +++ b/crates/compiler/src/nav.rs @@ -86,8 +86,7 @@ pub enum Builtin { Type(&'static str), /// A field of a primitive type, such as `Rect::x0`. Field(String), - /// A keyword argument. Argon only permits these on builtin calls, so a - /// keyword argument never names a parameter declared in source. + /// A keyword argument of a builtin call. KwArg(String), } @@ -244,6 +243,8 @@ struct Builder<'a> { enums: HashMap, /// Cell `VarId` to field name to the `VarId` of the `let` declaring it. cell_fields: HashMap>, + /// Fn or cell `VarId` to parameter name to the parameter's `VarId`. + params: HashMap>, /// Module currently being walked. current: &'a ModPath, path: &'a Path, @@ -253,6 +254,13 @@ struct Builder<'a> { index: NavIndex, } +/// Parameter names to the `VarId` each is bound to. +fn params(args: &[ArgDecl]) -> HashMap { + args.iter() + .map(|arg| (arg.name.name.to_string(), arg.metadata.0)) + .collect() +} + /// Whether a module is real source rather than something the compiler /// synthesized. /// @@ -281,6 +289,7 @@ impl<'a> Builder<'a> { ast, enums: HashMap::new(), cell_fields: HashMap::new(), + params: HashMap::new(), current: const { &Vec::new() }, path: Path::new(""), visible: 0, @@ -290,10 +299,9 @@ impl<'a> Builder<'a> { builder } - /// Records what the reference walk needs to have seen already: which - /// `VarId` each enum's name holds, and which `let` declares each of a - /// cell's fields. Both can be referred to from a module that is walked - /// earlier. + /// Records what the reference walk may need before it reaches the + /// declaring module: each enum's `VarId`, each cell's field bindings, and + /// each fn's or cell's parameters. fn collect_declarations(&mut self) { for (module, ast) in self.ast.iter() { if !is_navigable(&ast.path) { @@ -327,6 +335,10 @@ impl<'a> Builder<'a> { }) .collect(); self.cell_fields.insert(decl.metadata.1, fields); + self.params.insert(decl.metadata.1, params(&decl.args)); + } + Decl::Fn(decl) => { + self.params.insert(decl.metadata.1, params(&decl.args)); } _ => {} } @@ -334,6 +346,19 @@ impl<'a> Builder<'a> { } } + /// What a keyword argument names: a parameter of the callee bound to + /// `callee`, or a builtin's keyword when the callee is a builtin. + fn kwarg_target(&self, callee: Option, name: &str) -> Target { + match callee { + None => Target::Builtin(Builtin::KwArg(name.to_owned())), + Some(callee) => self + .params + .get(&callee) + .and_then(|params| params.get(name)) + .map_or(Target::Unresolved, |id| Target::Def(DefKey::Var(*id))), + } + } + fn run(mut self) -> NavIndex { for (module, ast) in self.ast.iter() { if !is_navigable(&ast.path) { @@ -543,6 +568,10 @@ impl<'a> Builder<'a> { } fn arg_decl(&mut self, arg: &'a ArgDecl) { + // The default is evaluated before the parameter is bound. + if let Some(default) = &arg.default { + self.expr(default); + } self.define( DefKey::Var(arg.metadata.0), SymbolKind::Parameter, @@ -649,10 +678,8 @@ impl<'a> Builder<'a> { self.expr(arg); } for kwarg in &call.args.kwargs { - self.record( - kwarg.name.span, - Target::Builtin(Builtin::KwArg(kwarg.name.name.to_string())), - ); + let target = self.kwarg_target(call.metadata.0, &kwarg.name.name); + self.record(kwarg.name.span, target); self.expr(&kwarg.value); } } @@ -1177,6 +1204,28 @@ cell top(w: Flo$0at) { ); } + #[test] + fn keyword_arguments_resolve_to_parameters() { + check( + r#" +fn grow(base: Float, fac$0tor: Float = 2., shift: Float = ba$0se) -> Float { + base * factor + shift +} +cell top() { + let a = grow(1., fac$0tor=3., shi$0ft=0.); + let r = rect("met1", x$00=a, y0=0., x1=1., y1=1.); +} +"#, + &[ + "factor#0", + "base#0", + "factor#0", + "shift#0", + r#"Builtin(KwArg("x0"))"#, + ], + ); + } + #[test] fn the_standard_library_is_navigable() { let (_, index, offsets) = index( diff --git a/crates/compiler/src/parse.rs b/crates/compiler/src/parse.rs index b11ab4a..2387c3a 100644 --- a/crates/compiler/src/parse.rs +++ b/crates/compiler/src/parse.rs @@ -528,9 +528,6 @@ impl EntryCell { invocation: &str, ) -> Result { let call = parse_cell(invocation)?; - if !call.args.kwargs.is_empty() { - bail!("cells take positional arguments only; keyword arguments are not supported"); - } // Splice the call expression itself rather than the raw argument: // trailing trivia such as a line comment would otherwise swallow the // generated `;`. diff --git a/crates/compiler/src/parser/grammar.rs b/crates/compiler/src/parser/grammar.rs index 14ac699..04b15e9 100644 --- a/crates/compiler/src/parser/grammar.rs +++ b/crates/compiler/src/parser/grammar.rs @@ -525,18 +525,25 @@ impl<'a> Parser<'a> { } /// `argDecls : (argDecl (COMMA argDecl)* COMMA?)?` + /// + /// Default values number their scopes from zero, like a brace scope. fn parse_arg_decls(&mut self) -> Vec> { - self.separated_list(TokenKind::RParen, |p| p.parse_arg_decl()) + self.scope_orders.push(0); + let args = self.separated_list(TokenKind::RParen, |p| p.parse_arg_decl()); + self.scope_orders.pop(); + args } - /// `argDecl : ident COLON tySpec` + /// `argDecl : ident COLON tySpec (EQ expr)?` fn parse_arg_decl(&mut self) -> ArgDecl<&'a str, Md> { let name = self.ident(); self.expect(TokenKind::Colon); let ty = self.parse_ty_spec(); + let default = self.eat(TokenKind::Eq).then(|| self.parse_expr(0)); ArgDecl { name, ty, + default, metadata: (), } } diff --git a/crates/compiler/src/parser/mod.rs b/crates/compiler/src/parser/mod.rs index b386fa6..fbae345 100644 --- a/crates/compiler/src/parser/mod.rs +++ b/crates/compiler/src/parser/mod.rs @@ -528,6 +528,34 @@ mod tests { } } + #[test] + fn default_values_parse() { + use crate::ast::{Decl, Expr}; + + let src = "fn f(a: Float, b: Float = 1., c: [Int] = []) {}\ncell c(n: Int = 2 * 3) {}"; + let mut parser = super::grammar::Parser::new(src, 0); + let ast = parser.parse_root(); + assert!(parser.errors.is_empty(), "{:?}", parser.errors); + let [Decl::Fn(f), Decl::Cell(c)] = ast.decls.as_slice() else { + panic!("expected a fn and a cell, got {:?}", ast.decls); + }; + assert!(f.args[0].default.is_none()); + let b = f.args[1].default.as_ref().expect("`b` has a default"); + assert!(matches!(b, Expr::FloatLiteral(_))); + assert_eq!(&src[b.span().start()..b.span().end()], "1."); + assert!(matches!(f.args[2].default, Some(Expr::SeqNil(_)))); + let n = c.args[0].default.as_ref().expect("`n` has a default"); + assert_eq!(&src[n.span().start()..n.span().end()], "2 * 3"); + + for src in [ + "fn f(a: Float = ) {}", + "cell c(n: Int = 1 {}", + "fn f(a = 1.) {}", + ] { + assert!(parse(src).is_err(), "should be rejected: `{src}`"); + } + } + #[test] fn distinct_diagnostics_at_same_offset_are_kept() { // `foo(` then `}`: the `}` is simultaneously where an expression, a `)`, diff --git a/docs/parser.md b/docs/parser.md index cd74036..6c29bd8 100644 --- a/docs/parser.md +++ b/docs/parser.md @@ -345,14 +345,16 @@ while !self.at(Eof) { |----------|---------------------|---------------|-------| | `enum` | `parse_enum_decl` | `EnumDecl` | `enum Name { ident, … }` | | `struct` | `parse_struct_decl` | `StructDecl` | `struct Name { field: Ty, … }` | -| `cell` | `parse_cell_decl` | `CellDecl` | `cell Name(args) scope` | -| `fn` | `parse_fn_decl` | `FnDecl` | `fn Name(args) (-> Ty)? scope` | +| `cell` | `parse_cell_decl` | `CellDecl` | `cell Name(argDecls) scope` | +| `fn` | `parse_fn_decl` | `FnDecl` | `fn Name(argDecls) (-> Ty)? scope` | | `const` | `parse_const_decl` | `ConstantDecl`| `const Name: Ty = expr;` | | `mod` | `parse_mod_decl` | `ModDecl` | `mod Name;` | -Argument declarations (`argDecl : ident COLON tySpec`) and enum variants / -struct fields are comma-separated lists parsed by the shared `separated_list` -helper (§8). +Argument declarations (`argDecl : ident COLON tySpec (EQ expr)?`) and enum +variants / struct fields are comma-separated lists parsed by the shared +`separated_list` helper (§8). A parameter with a default value is a keyword +parameter; `parse_arg_decls` gives the parameter list its own scope-ordinal +counter (§10) so scopes opened inside default values are numbered from zero. --- diff --git a/examples/kwargs_cell/Argon.toml b/examples/kwargs_cell/Argon.toml new file mode 100644 index 0000000..758dae5 --- /dev/null +++ b/examples/kwargs_cell/Argon.toml @@ -0,0 +1,2 @@ +name = "kwargs-cell" +tech = "../tech/basic.tech.toml" diff --git a/examples/kwargs_cell/lib.ar b/examples/kwargs_cell/lib.ar new file mode 100644 index 0000000..6f32215 --- /dev/null +++ b/examples/kwargs_cell/lib.ar @@ -0,0 +1,21 @@ +// Cells take keyword parameters too. Omitted keywords are filled from their +// defaults at the call site, so `child(w)` and `child(w, h=w)` receive the same +// arguments. +cell child(w: Float, h: Float = w, layer: String = "met1") { + let body = rect(layer, x0=0., y0=0., x1=w, y1=h); +} + +cell top(w: Float = 100.) { + let square = inst(child(w)); + let tall = inst(child(w, h=200.)); + let same = inst(child(w, h=w)); + let met2 = inst(child(50., layer="met2")); + eq(square.x, 0.); + eq(square.y, 0.); + eq(tall.x, 200.); + eq(tall.y, 0.); + eq(same.x, 400.); + eq(same.y, 0.); + eq(met2.x, 600.); + eq(met2.y, 0.); +} diff --git a/examples/kwargs_fn/Argon.toml b/examples/kwargs_fn/Argon.toml new file mode 100644 index 0000000..3325857 --- /dev/null +++ b/examples/kwargs_fn/Argon.toml @@ -0,0 +1,2 @@ +name = "kwargs-fn" +tech = "../tech/basic.tech.toml" diff --git a/examples/kwargs_fn/lib.ar b/examples/kwargs_fn/lib.ar new file mode 100644 index 0000000..aa7f211 --- /dev/null +++ b/examples/kwargs_fn/lib.ar @@ -0,0 +1,19 @@ +// Parameters with defaults are keyword parameters: they are passed by name and +// may be omitted. A default may refer to the parameters declared before it. +fn scaled(x: Float, scale: Float = 2., offset: Float = x) -> Float { + x * scale + offset +} + +// A default may be any expression, including one that opens a scope. +fn pick(flag: Bool, value: Float = if flag { 1. } else { 2. }) -> Float { + value +} + +cell top() { + let a = rect("met1", x0=0., y0=0., x1=scaled(10.), y1=10.); + let b = rect("met1", x0=0., y0=20., x1=scaled(10., scale=3.), y1=30.); + let c = rect("met1", x0=0., y0=40., x1=scaled(10., offset=0.), y1=50.); + let d = rect("met1", x0=0., y0=60., x1=scaled(10., scale=0.5, offset=5.), y1=70.); + let e = rect("met1", x0=0., y0=80., x1=pick(true) * 10., y1=90.); + let f = rect("met1", x0=0., y0=100., x1=pick(false, value=5.) * 10., y1=110.); +}