From a818390fbbf9beb681b75494a2b1025a5070684e Mon Sep 17 00:00:00 2001 From: Matthew Paras <34500476+mattwparas@users.noreply.github.com> Date: Sun, 23 Jun 2024 09:51:42 -0700 Subject: [PATCH] Add minimal support for improper lists in macro expansion (#226) --- .../steel-core/src/parser/expand_visitor.rs | 4 ++ crates/steel-core/src/parser/expander.rs | 49 +++++++++++++++---- .../steel-core/src/parser/replace_idents.rs | 37 ++++++++++---- crates/steel-core/src/tests/success/docs.scm | 7 +++ crates/steel-parser/src/ast.rs | 13 +++++ 5 files changed, 91 insertions(+), 19 deletions(-) diff --git a/crates/steel-core/src/parser/expand_visitor.rs b/crates/steel-core/src/parser/expand_visitor.rs index db0efc44c..ea8ffb0f6 100644 --- a/crates/steel-core/src/parser/expand_visitor.rs +++ b/crates/steel-core/src/parser/expand_visitor.rs @@ -190,6 +190,10 @@ impl<'a> VisitorMutRef for Expander<'a> { if let ExprKind::LambdaFunction(mut lambda) = parse_lambda(ident.clone(), std::mem::take(&mut l.args))? { + if l.improper { + lambda.rest = true; + } + self.visit_lambda_function(&mut lambda)?; *expr = ExprKind::LambdaFunction(lambda); diff --git a/crates/steel-core/src/parser/expander.rs b/crates/steel-core/src/parser/expander.rs index 4eee0adaf..c7751155a 100644 --- a/crates/steel-core/src/parser/expander.rs +++ b/crates/steel-core/src/parser/expander.rs @@ -352,6 +352,7 @@ impl MacroCase { &expr[1..], &mut bindings, &mut binding_kind, + expr.improper, )?; let mut body = self.body.clone(); @@ -884,6 +885,7 @@ pub fn collect_bindings( list: &[ExprKind], bindings: &mut FxHashMap, binding_kind: &mut FxHashMap, + improper: bool, ) -> Result<()> { let mut token_iter = list.iter(); @@ -919,7 +921,7 @@ pub fn collect_bindings( // bind the ellipses to the rest of the statement MacroPattern::Many(ident) => { let rest: Vec<_> = token_iter.cloned().collect(); - bindings.insert(*ident, List::new(rest).into()); + bindings.insert(*ident, List::new_maybe_improper(rest, improper).into()); binding_kind.insert(*ident, BindingKind::Many); break; } @@ -929,7 +931,7 @@ pub fn collect_bindings( .ok_or_else(throw!(ArityMismatch => "Macro expected a pattern"))?; if let ExprKind::List(l) = child { - collect_bindings(children, l, bindings, binding_kind)?; + collect_bindings(children, l, bindings, binding_kind, l.improper)?; } else if let ExprKind::Quote(q) = child { if let &[MacroPattern::Quote(x)] = children.as_slice() { bindings.insert(x, q.expr.clone()); @@ -941,8 +943,6 @@ pub fn collect_bindings( } } MacroPattern::ManyNested(children) => { - // dbg!(children); - let exprs_to_destruct = token_iter.collect::>(); for i in 0..children.len() { @@ -956,7 +956,9 @@ pub fn collect_bindings( if is_ellipses_pattern { // Bind the "rest" of the values into this - values_to_bind.push(List::new(l[i..].to_vec()).into()); + values_to_bind.push( + List::new_maybe_improper(l[i..].to_vec(), l.improper).into(), + ); } else { values_to_bind.push(l[i].clone()); } @@ -995,6 +997,7 @@ pub fn collect_bindings( list_expr, &mut new_bindings, binding_kind, + list_expr.improper, )?; for (key, value) in new_bindings { @@ -1238,7 +1241,14 @@ mod collect_bindings_tests { ])), ]); - collect_bindings(&pattern_args, &list_expr, &mut bindings, &mut binding_kind).unwrap(); + collect_bindings( + &pattern_args, + &list_expr, + &mut bindings, + &mut binding_kind, + false, + ) + .unwrap(); let mut post_bindings = FxHashMap::default(); post_bindings.insert( @@ -1285,7 +1295,14 @@ mod collect_bindings_tests { ])), ]); - collect_bindings(&pattern_args, &list_expr, &mut bindings, &mut binding_kind).unwrap(); + collect_bindings( + &pattern_args, + &list_expr, + &mut bindings, + &mut binding_kind, + false, + ) + .unwrap(); let mut post_bindings = FxHashMap::default(); post_bindings.insert( @@ -1329,7 +1346,14 @@ mod collect_bindings_tests { atom_identifier("y"), ]); - collect_bindings(&pattern_args, &list_expr, &mut bindings, &mut binding_kind).unwrap(); + collect_bindings( + &pattern_args, + &list_expr, + &mut bindings, + &mut binding_kind, + false, + ) + .unwrap(); let mut post_bindings = FxHashMap::default(); post_bindings.insert( @@ -1379,7 +1403,14 @@ mod collect_bindings_tests { let mut post_bindings = FxHashMap::default(); - collect_bindings(&pattern_args, &list_expr, &mut bindings, &mut binding_kind).unwrap(); + collect_bindings( + &pattern_args, + &list_expr, + &mut bindings, + &mut binding_kind, + false, + ) + .unwrap(); post_bindings.insert("a".into(), atom_int(1)); post_bindings.insert("b".into(), atom_identifier("apple")); post_bindings.insert( diff --git a/crates/steel-core/src/parser/replace_idents.rs b/crates/steel-core/src/parser/replace_idents.rs index 289122fe8..d92b7f993 100644 --- a/crates/steel-core/src/parser/replace_idents.rs +++ b/crates/steel-core/src/parser/replace_idents.rs @@ -144,10 +144,11 @@ impl<'a> ReplaceExpressions<'a> { // ExprKind::Atom(expr) // } - fn expand_ellipses(&mut self, vec_exprs: &mut Vec) -> Result<()> { + // Note: Returns a bool indicating if this should be made improper/rest or not + fn expand_ellipses(&mut self, vec_exprs: &mut Vec) -> Result { if let Some(ellipses_pos) = vec_exprs.iter().position(check_ellipses) { if ellipses_pos == 0 { - return Ok(()); + return Ok(false); } let variable_to_lookup = vec_exprs.get(ellipses_pos - 1).ok_or_else( @@ -162,24 +163,30 @@ impl<'a> ReplaceExpressions<'a> { .. }, }) => { + let improper; + // let rest = self.bindings.get(var).ok_or_else(throw!(BadSyntax => format!("macro expansion failed at finding the variable when expanding ellipses: {var}")))?; let rest = if let Some(rest) = self.bindings.get(var) { rest } else { - return Ok(()); + return Ok(false); }; let list_of_exprs = if let ExprKind::List(list_of_exprs) = rest { + improper = list_of_exprs.improper; + list_of_exprs } else { let res = if let Some(res) = self.fallback_bindings.get(var) { res.list_or_else( throw!(BadSyntax => "macro expansion failed, expected list of expressions, found: {}, within {}", rest, super::ast::List::new(vec_exprs.clone())))? } else { - return Ok(()); + return Ok(false); }; + improper = res.improper; + // let res = self.fallback_bindings.get(var).ok_or_else(throw!(BadSyntax => format!("macro expansion failed at finding the variable when expanding ellipses: {var}")))?.list_or_else( // throw!(BadSyntax => "macro expansion failed, expected list of expressions, found: {}, within {}", rest, super::ast::List::new(vec_exprs.clone())) // )?; @@ -206,10 +213,12 @@ impl<'a> ReplaceExpressions<'a> { // *vec_exprs = first_chunk; - Ok(()) + Ok(improper) } - ExprKind::List(_) => { + ExprKind::List(bound_list) => { + let improper = bound_list.improper; + let visitor = EllipsesExpanderVisitor::find_expansion_width_and_collect_ellipses_expanders(self.bindings, self.binding_kind, variable_to_lookup); if let Some(error) = visitor.error { @@ -273,7 +282,7 @@ impl<'a> ReplaceExpressions<'a> { // *vec_exprs = first_chunk; - Ok(()) + Ok(improper) // Ok(()) @@ -285,7 +294,7 @@ impl<'a> ReplaceExpressions<'a> { } } } else { - Ok(()) + Ok(false) } } @@ -481,12 +490,16 @@ impl<'a> VisitorMutRef for ReplaceExpressions<'a> { // return self.visit(expanded); } - self.expand_ellipses(&mut l.args)?; + let improper = self.expand_ellipses(&mut l.args)?; for expr in l.args.iter_mut() { self.visit(expr)?; } + if improper { + l.set_improper(); + } + if let Some(expanded) = self.vec_syntax_span_object(&l.args)? { *expr = expanded; return Ok(()); @@ -523,7 +536,7 @@ impl<'a> VisitorMutRef for ReplaceExpressions<'a> { &mut self, lambda_function: &mut super::ast::LambdaFunction, ) -> Self::Output { - self.expand_ellipses(&mut lambda_function.args)?; + let improper = self.expand_ellipses(&mut lambda_function.args)?; for arg in lambda_function.args.iter_mut() { self.visit(arg)?; @@ -531,6 +544,10 @@ impl<'a> VisitorMutRef for ReplaceExpressions<'a> { self.visit(&mut lambda_function.body)?; + if improper { + lambda_function.rest = true; + } + // TODO: @Matt - 2/28/12 -> clean up this // This mangles the values // lambda_function.args.iter_mut().for_each(|x| { diff --git a/crates/steel-core/src/tests/success/docs.scm b/crates/steel-core/src/tests/success/docs.scm index 886659e72..6187812be 100644 --- a/crates/steel-core/src/tests/success/docs.scm +++ b/crates/steel-core/src/tests/success/docs.scm @@ -4,3 +4,10 @@ (list 10 20 30 40 x)) (assert! (equal? "This is a function that does something\n" foo__doc__)) + +;;@doc +;; This is a function that takes multiple arguments +(define (bar . args) + (list 10 20 30 40 args)) + +(assert! (equal? (list 10 20 30 40 (list 50 60)) (bar 50 60))) diff --git a/crates/steel-parser/src/ast.rs b/crates/steel-parser/src/ast.rs index 063e21043..9a01648e0 100644 --- a/crates/steel-parser/src/ast.rs +++ b/crates/steel-parser/src/ast.rs @@ -992,6 +992,15 @@ impl List { } } + pub fn new_maybe_improper(args: Vec, improper: bool) -> Self { + List { + args, + syntax_object_id: SyntaxObjectId::fresh().0, + improper, + location: None, + } + } + pub fn with_spans(args: Vec, open: Span, close: Span) -> Self { List { args, @@ -1006,6 +1015,10 @@ impl List { self } + pub fn set_improper(&mut self) { + self.improper = true; + } + pub fn is_empty(&self) -> bool { self.args.is_empty() }