From 2df4de2435edc3a768f78de501a8185b41a01ca2 Mon Sep 17 00:00:00 2001 From: Matt Paras Date: Sat, 9 Nov 2024 11:22:16 -0800 Subject: [PATCH 1/2] fix macro expansionn bug --- .../steel-core/src/compiler/passes/shadow.rs | 50 +------------------ .../steel-core/src/parser/expand_visitor.rs | 41 +++++++++++---- crates/steel-core/src/parser/rename_idents.rs | 27 ++++++++-- 3 files changed, 55 insertions(+), 63 deletions(-) diff --git a/crates/steel-core/src/compiler/passes/shadow.rs b/crates/steel-core/src/compiler/passes/shadow.rs index fd3a82160..662733d9b 100644 --- a/crates/steel-core/src/compiler/passes/shadow.rs +++ b/crates/steel-core/src/compiler/passes/shadow.rs @@ -78,55 +78,6 @@ impl RenameShadowedVariables { } impl VisitorMutRefUnit for RenameShadowedVariables { - // fn visit_begin(&mut self, begin: &mut steel_parser::ast::Begin) { - // for expr in &mut begin.exprs { - // if self.scope.depth() > 1 { - // // TODO: Flatten begins _first_ - // if let ExprKind::Define(define) = expr { - // if let ExprKind::Atom(a) = &mut define.name { - // let variable = if let Some(variable) = a.syn.ty.identifier_mut() { - // variable - // } else { - // return; - // }; - - // if self.rename_all || self.scope.contains(variable) { - // let modifier = self.scope.depth(); - // self.shadows.define(*variable, modifier); - // // println!("Renaming: {} -> {}", variable.resolve()); - - // // Create a mutable string to mangle - // let mut mut_var = "##".to_string() + variable.resolve(); - - // if let Some(char_modifier) = char::from_digit(modifier as u32, 10) { - // mut_var.push(char_modifier); - // } else if let Some(str_modifier) = self.str_modifiers.get(&modifier) { - // mut_var.push_str(str_modifier); - // } else { - // self.str_modifiers.insert(modifier, modifier.to_string()); - // mut_var.push_str(self.str_modifiers.get(&modifier).unwrap()); - // } - // println!( - // "define - Renaming: {} -> {} @ depth: {}", - // variable.resolve(), - // mut_var, - // modifier - // ); - - // *variable = mut_var.into(); - // } - - // self.scope.define(*variable); - // } - // } - // } - // } - - // for expr in &mut begin.exprs { - // self.visit(expr); - // } - // } - fn visit_define(&mut self, define: &mut steel_parser::ast::Define) { if self.scope.depth() > 1 { if let ExprKind::Atom(a) = &mut define.name { @@ -305,6 +256,7 @@ impl VisitorMutRefUnit for RenameShadowedVariables { fn visit_let(&mut self, l: &mut crate::parser::ast::Let) { l.bindings.iter_mut().for_each(|x| self.visit(&mut x.1)); + self.scope.push_layer(); self.shadows.push_layer(); diff --git a/crates/steel-core/src/parser/expand_visitor.rs b/crates/steel-core/src/parser/expand_visitor.rs index 2d79ec34d..b45f2e2c2 100644 --- a/crates/steel-core/src/parser/expand_visitor.rs +++ b/crates/steel-core/src/parser/expand_visitor.rs @@ -1,6 +1,6 @@ use fxhash::{FxBuildHasher, FxHashMap, FxHashSet}; use quickscope::ScopeSet; -use steel_parser::ast::{parse_lambda, Begin}; +use steel_parser::ast::{parse_lambda, Begin, Let}; use steel_parser::parser::SourceId; use crate::parser::ast::ExprKind; @@ -578,6 +578,9 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> return Ok(()); } + // println!("Expanding keyword args"); + // println!("Body: {}", lambda_function.body); + if (keyword_args.len() % 2 != 0 && !lambda_function.rest) || (lambda_function.rest && keyword_args.len() - 1 % 2 != 0) { @@ -634,7 +637,7 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> let original_var_name = x.1; // This is a bit wasteful... come back to this - let (var_name, expr) = if let ExprKind::List(l) = original_var_name { + let (mut var_name, expr) = if let ExprKind::List(l) = original_var_name { if l.len() != 2 { stop!(BadSyntax => "Missing default argument for keyword"; lambda_function.location.span) @@ -645,7 +648,9 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> (original_var_name.clone(), original_var_name.clone()) }; - // println!("{:?}", original_var_name); + if let Some(var) = var_name.atom_syntax_object_mut() { + var.introduced_via_macro = true; + } // TODO: Go here to implement default arguments let expression = ExprKind::default_if( @@ -694,20 +699,27 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> non_keyword_args.push(ExprKind::ident("!!dummy-rest-arg!!")); - let mut inner_application = vec![ExprKind::LambdaFunction(Box::new(LambdaFunction::new( - bindings.iter().map(|x| x.0.clone()).collect(), + let inner_application = ExprKind::Let(Box::new(Let::new( + bindings, lambda_function.body.clone(), - SyntaxObject::default(TokenType::Lambda), - )))]; + SyntaxObject::default(TokenType::Let), + ))); - inner_application.extend(bindings.iter().map(|x| x.1.clone())); + // let mut inner_application = vec![ExprKind::LambdaFunction(Box::new(LambdaFunction::new( + // bindings.iter().map(|x| x.0.clone()).collect(), + // lambda_function.body.clone(), + // SyntaxObject::default(TokenType::Lambda), + // )))]; + + // inner_application.extend(bindings.iter().map(|x| x.1.clone())); *lambda_function = LambdaFunction::new_with_rest_arg( non_keyword_args, expr_list![ ExprKind::LambdaFunction(Box::new(LambdaFunction::new( vec![ExprKind::ident("!!dummy-rest-arg!!")], - ExprKind::List(List::new(inner_application)), + // ExprKind::List(List::new(inner_application)), + inner_application, SyntaxObject::default(TokenType::Lambda), ))), expr_list![ @@ -720,6 +732,14 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> SyntaxObject::default(TokenType::Lambda), ); + // let pretty = { + // let mut w = Vec::new(); + // lambda_function.to_doc().render(60, &mut w).unwrap(); + // String::from_utf8(w).unwrap() + // }; + + // println!("After expansion: {}", pretty); + Ok(()) } @@ -749,6 +769,9 @@ impl<'a> VisitorMutRef for KernelExpander<'a> { self.visit(&mut lambda_function.body)?; // Expand keyword arguments if we can + // TODO: If this isn't a lambda function, we're gonna have problems + // if its a list, we should run the same thing, but on a list + // that starts with lambda, and coerce it to be expanded that way expand_keyword_arguments(lambda_function)?; Ok(()) diff --git a/crates/steel-core/src/parser/rename_idents.rs b/crates/steel-core/src/parser/rename_idents.rs index 226f2c16b..a051a9593 100644 --- a/crates/steel-core/src/parser/rename_idents.rs +++ b/crates/steel-core/src/parser/rename_idents.rs @@ -1,4 +1,4 @@ -use steel_parser::ast::{Atom, LAMBDA, LAMBDA_FN}; +use steel_parser::ast::{Atom, LAMBDA, LAMBDA_FN, PLAIN_LET}; use crate::compiler::program::{DATUM_SYNTAX, LAMBDA_SYMBOL}; use crate::parser::ast::ExprKind; @@ -124,7 +124,6 @@ impl<'a> VisitorMutRef for RenameIdentifiersVisitor<'a> { if self.is_gensym(&s) { a.syn.ty = TokenType::Identifier(("##".to_string() + s.resolve()).into()); } else { - // println!("Unresolved: {}", a); a.syn.unresolved = true; } } @@ -273,7 +272,7 @@ impl<'a> VisitorMutRef for RenameIdentifiersVisitor<'a> { Some(ExprKind::Atom(Atom { syn: SyntaxObject { ty, .. }, - })) if *ty == TokenType::Let => { + })) if *ty == TokenType::Let || *ty == TokenType::Identifier(*PLAIN_LET) => { match l.args.get_mut(1) { Some(ExprKind::List(bindings)) => { for pair in &mut bindings.args { @@ -390,8 +389,26 @@ impl<'a> VisitorMutRef for RenameIdentifiersVisitor<'a> { // TODO: This needs to be fixed! fn visit_let(&mut self, l: &mut super::ast::Let) -> Self::Output { - for (_, expr) in &mut l.bindings { - // println!("Visiting arg: {}", a); + for (arg, expr) in &mut l.bindings { + println!("RENAME IDENTS - Visiting arg: {}", arg); + + if let ExprKind::Atom(a) = arg { + if let SyntaxObject { + ty: TokenType::Identifier(ref s), + .. + } = a.syn + { + if !self.pattern_variables.contains(&s) { + self.add(*s); + } + + a.syn = SyntaxObject::default(TokenType::Identifier( + ("##".to_string() + s.resolve()).into(), + )); + a.syn.introduced_via_macro = true; + } + } + self.visit(expr); } From 6fc4f35e69077329a54a93c60184b0f1666cc058 Mon Sep 17 00:00:00 2001 From: Matt Paras Date: Sat, 9 Nov 2024 11:39:38 -0800 Subject: [PATCH 2/2] one more try --- .../steel-core/src/parser/expand_visitor.rs | 28 +++++++++---------- crates/steel-core/src/parser/rename_idents.rs | 2 -- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/steel-core/src/parser/expand_visitor.rs b/crates/steel-core/src/parser/expand_visitor.rs index b45f2e2c2..511f7ed75 100644 --- a/crates/steel-core/src/parser/expand_visitor.rs +++ b/crates/steel-core/src/parser/expand_visitor.rs @@ -1,6 +1,6 @@ use fxhash::{FxBuildHasher, FxHashMap, FxHashSet}; use quickscope::ScopeSet; -use steel_parser::ast::{parse_lambda, Begin, Let}; +use steel_parser::ast::{parse_lambda, Begin}; use steel_parser::parser::SourceId; use crate::parser::ast::ExprKind; @@ -699,27 +699,27 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) -> non_keyword_args.push(ExprKind::ident("!!dummy-rest-arg!!")); - let inner_application = ExprKind::Let(Box::new(Let::new( - bindings, - lambda_function.body.clone(), - SyntaxObject::default(TokenType::Let), - ))); - - // let mut inner_application = vec![ExprKind::LambdaFunction(Box::new(LambdaFunction::new( - // bindings.iter().map(|x| x.0.clone()).collect(), + // let inner_application = ExprKind::Let(Box::new(Let::new( + // bindings, // lambda_function.body.clone(), - // SyntaxObject::default(TokenType::Lambda), - // )))]; + // SyntaxObject::default(TokenType::Let), + // ))); + + let mut inner_application = vec![ExprKind::LambdaFunction(Box::new(LambdaFunction::new( + bindings.iter().map(|x| x.0.clone()).collect(), + lambda_function.body.clone(), + SyntaxObject::default(TokenType::Lambda), + )))]; - // inner_application.extend(bindings.iter().map(|x| x.1.clone())); + inner_application.extend(bindings.iter().map(|x| x.1.clone())); *lambda_function = LambdaFunction::new_with_rest_arg( non_keyword_args, expr_list![ ExprKind::LambdaFunction(Box::new(LambdaFunction::new( vec![ExprKind::ident("!!dummy-rest-arg!!")], - // ExprKind::List(List::new(inner_application)), - inner_application, + ExprKind::List(List::new(inner_application)), + // inner_application, SyntaxObject::default(TokenType::Lambda), ))), expr_list![ diff --git a/crates/steel-core/src/parser/rename_idents.rs b/crates/steel-core/src/parser/rename_idents.rs index a051a9593..09784776f 100644 --- a/crates/steel-core/src/parser/rename_idents.rs +++ b/crates/steel-core/src/parser/rename_idents.rs @@ -390,8 +390,6 @@ impl<'a> VisitorMutRef for RenameIdentifiersVisitor<'a> { // TODO: This needs to be fixed! fn visit_let(&mut self, l: &mut super::ast::Let) -> Self::Output { for (arg, expr) in &mut l.bindings { - println!("RENAME IDENTS - Visiting arg: {}", arg); - if let ExprKind::Atom(a) = arg { if let SyntaxObject { ty: TokenType::Identifier(ref s),