Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix macro expansion bug with keywords #288

Merged
merged 2 commits into from
Nov 9, 2024
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
50 changes: 1 addition & 49 deletions crates/steel-core/src/compiler/passes/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();

Expand Down
27 changes: 25 additions & 2 deletions crates/steel-core/src/parser/expand_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down Expand Up @@ -694,6 +699,12 @@ 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(),
lambda_function.body.clone(),
Expand All @@ -708,6 +719,7 @@ fn expand_keyword_arguments(lambda_function: &mut super::ast::LambdaFunction) ->
ExprKind::LambdaFunction(Box::new(LambdaFunction::new(
vec![ExprKind::ident("!!dummy-rest-arg!!")],
ExprKind::List(List::new(inner_application)),
// inner_application,
SyntaxObject::default(TokenType::Lambda),
))),
expr_list![
Expand All @@ -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(())
}

Expand Down Expand Up @@ -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(())
Expand Down
25 changes: 20 additions & 5 deletions crates/steel-core/src/parser/rename_idents.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -390,8 +389,24 @@ 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 {
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);
}

Expand Down
Loading