bugfix: in function environments which was causing incorrect environment state in some situations

This commit is contained in:
Ishan Jain 2024-05-19 14:12:50 +05:30
parent c92f4e9e82
commit 90fa04b29f
Signed by: ishan
GPG Key ID: 0506DB2A1CC75C27
3 changed files with 50 additions and 31 deletions

View File

@ -2,20 +2,22 @@ use {
crate::parser::ast::{BlockStatement, Identifier, Node},
itertools::Itertools,
std::{
cell::RefCell,
collections::HashMap,
fmt::{Display, Formatter, Result as FmtResult, Write},
rc::Rc,
},
};
pub mod tree_walker;
pub trait Evaluator {
fn eval(&self, node: Node, env: &mut Environment) -> Option<Object>;
fn eval(&self, node: Node, env: Rc<RefCell<Environment>>) -> Option<Object>;
}
#[derive(Debug, Clone, PartialEq)]
pub struct Environment {
store: HashMap<String, Object>,
outer: Option<Box<Environment>>,
outer: Option<Rc<RefCell<Environment>>>,
}
impl Environment {
@ -29,7 +31,10 @@ impl Environment {
match self.store.get(name) {
Some(v) => Some(v.clone()),
None => match &self.outer {
Some(outer) => outer.get(name),
Some(outer) => {
let outer = outer.borrow();
outer.get(name)
}
None => None,
},
}
@ -38,10 +43,10 @@ impl Environment {
self.store.insert(name, val);
}
pub fn new_enclosed(env: Environment) -> Self {
pub fn new_enclosed(env: Rc<RefCell<Environment>>) -> Self {
Self {
store: HashMap::new(),
outer: Some(Box::new(env)),
outer: Some(env),
}
}
}
@ -101,12 +106,12 @@ impl Display for Object {
pub struct Function {
parameters: Vec<Identifier>,
body: BlockStatement,
env: Environment,
env: Rc<RefCell<Environment>>,
}
#[cfg(test)]
mod tests {
use std::assert_matches::assert_matches;
use std::{assert_matches::assert_matches, cell::RefCell, rc::Rc};
use crate::{
evaluator::{tree_walker::TreeWalker, Environment, Evaluator, Object, FALSE, NULL, TRUE},
@ -122,8 +127,8 @@ mod tests {
assert!(program.is_some());
let program = program.unwrap();
let evaluator = TreeWalker::new();
let mut env = Environment::new();
let eval = evaluator.eval(Node::Program(program), &mut env);
let env = Rc::new(RefCell::new(Environment::new()));
let eval = evaluator.eval(Node::Program(program), env);
assert_eq!(eval, test.1);
}
}
@ -305,8 +310,8 @@ mod tests {
assert!(program.is_some());
let program = program.unwrap();
let evaluator = TreeWalker::new();
let mut env = Environment::new();
let eval = evaluator.eval(Node::Program(program), &mut env);
let env = Rc::new(RefCell::new(Environment::new()));
let eval = evaluator.eval(Node::Program(program), env);
let node = eval.unwrap();
assert_matches!(node, Object::Function(_));
@ -364,6 +369,8 @@ mod tests {
#[test]
fn memory_test() {
// This test case allocates memory for `foobar=9999` over and over
// even though it is never used.
let test_cases = [(
"let counter = fn(x) {
if (x > 100 ) {
@ -375,7 +382,7 @@ mod tests {
};
counter(0);
",
Some(Object::Integer(15)),
Some(Object::Boolean(true)),
)];
run_test_cases(&test_cases);

View File

@ -1,3 +1,5 @@
use std::{cell::RefCell, rc::Rc};
// TODO: This is all a mess. Almost certainly because right now, I don't know any better way to do this.
// It's just constantly unwrapping enums from one place and rewrapping it to some other enum(or even the same enum) and returning it
// The error handling story is pretty bad too
@ -21,7 +23,7 @@ impl TreeWalker {
}
impl Evaluator for TreeWalker {
fn eval(&self, node: Node, env: &mut Environment) -> Option<Object> {
fn eval(&self, node: Node, env: Rc<RefCell<Environment>>) -> Option<Object> {
match node {
Node::Program(p) => self.eval_program(p, env),
Node::Statement(stmt) => match stmt {
@ -34,8 +36,10 @@ impl Evaluator for TreeWalker {
Some(Object::ReturnValue(Box::new(ret_val)))
}
Statement::Let(LetStatement { name, value }) => {
let value = self.eval(Node::Expression(value?), env)?;
let value = self.eval(Node::Expression(value?), env.clone())?;
let mut env = env.borrow_mut();
env.set(name.to_string(), value.clone());
Some(value)
}
},
@ -48,7 +52,7 @@ impl Evaluator for TreeWalker {
self.eval_prefix_expression(p.operator, expr)
}
Expression::InfixExpression(ie) => {
let left = self.eval(Node::Expression(*ie.left), env)?;
let left = self.eval(Node::Expression(*ie.left), env.clone())?;
let right = self.eval(Node::Expression(*ie.right), env)?;
self.eval_infix_expression(left, ie.operator, right)
}
@ -56,11 +60,11 @@ impl Evaluator for TreeWalker {
return Some(Object::Function(Function {
body: fnl.body,
parameters: fnl.parameters,
env: env.clone(),
}))
env: env,
}));
}
Expression::CallExpression(v) => {
let function = self.eval(Node::Expression(*v.function), env)?;
let function = self.eval(Node::Expression(*v.function), env.clone())?;
// Resolve function arguments and update the environment
// before executing function body
let args = match self.eval_expression(v.arguments, env) {
@ -71,7 +75,7 @@ impl Evaluator for TreeWalker {
self.apply_function(function, args)
}
Expression::IfExpression(ie) => {
let condition = self.eval(Node::Expression(*ie.condition), env)?;
let condition = self.eval(Node::Expression(*ie.condition), env.clone())?;
if self.is_truthy(&condition) {
self.eval(
@ -90,10 +94,10 @@ impl Evaluator for TreeWalker {
}
impl TreeWalker {
fn eval_program(&self, prg: Program, env: &mut Environment) -> Option<Object> {
fn eval_program(&self, prg: Program, env: Rc<RefCell<Environment>>) -> Option<Object> {
let mut out: Option<Object> = None;
for stmt in prg.statements {
out = self.eval(Node::Statement(stmt), env);
out = self.eval(Node::Statement(stmt), env.clone());
// No need to evaluate any more statements from a statements vector once we
// get a return keyword. nothing after in the block matters.
if let Some(out) = out.as_ref() {
@ -107,11 +111,15 @@ impl TreeWalker {
out
}
fn eval_block_statement(&self, bs: BlockStatement, env: &mut Environment) -> Option<Object> {
fn eval_block_statement(
&self,
bs: BlockStatement,
env: Rc<RefCell<Environment>>,
) -> Option<Object> {
let mut out: Option<Object> = None;
for stmt in bs.statements {
out = self.eval(Node::Statement(stmt), env);
out = self.eval(Node::Statement(stmt), env.clone());
// TODO: Find a nicer way to do this. :(
// The objective here is,
@ -208,7 +216,8 @@ impl TreeWalker {
}
}
fn eval_identifier(&self, node: Identifier, env: &mut Environment) -> Option<Object> {
fn eval_identifier(&self, node: Identifier, env: Rc<RefCell<Environment>>) -> Option<Object> {
let env = env.borrow();
env.get(&node.to_string()).or(Some(Object::Error(format!(
"identifier not found: {}",
node.to_string()
@ -218,12 +227,12 @@ impl TreeWalker {
fn eval_expression(
&self,
exprs: Vec<Expression>,
env: &mut Environment,
env: Rc<RefCell<Environment>>,
) -> Result<Vec<Object>, Object> {
let mut out = vec![];
for expr in exprs {
match self.eval(Node::Expression(expr), env) {
match self.eval(Node::Expression(expr), env.clone()) {
Some(v @ Object::Error(_)) => return Err(v),
Some(v) => out.push(v),
None => {
@ -243,7 +252,6 @@ impl TreeWalker {
}
v => return Some(Object::Error(format!("not a function: {}", v.to_string()))),
};
println!("{:?}", function.env);
let mut enclosed_env = Environment::new_enclosed(function.env);
for (i, parameter) in function.parameters.iter().enumerate() {
@ -256,7 +264,7 @@ impl TreeWalker {
let resp = self.eval(
Node::Statement(Statement::BlockStatement(function.body)),
&mut enclosed_env,
Rc::new(RefCell::new(enclosed_env)),
);
// Unwrap return here to prevent it from bubbling up the stack

View File

@ -4,7 +4,11 @@ use {
lexer::Lexer,
parser::{ast::Node, Error as ParserError, Parser},
},
std::io::{self, BufRead, Result as IoResult, Write},
std::{
cell::RefCell,
io::{self, BufRead, Result as IoResult, Write},
rc::Rc,
},
};
const PROMPT: &[u8] = b">> ";
@ -19,7 +23,7 @@ pub fn init() {
}
fn start<R: BufRead, W: Write>(mut ip: R, mut out: W) {
let mut environment = Environment::new();
let environment = Rc::new(RefCell::new(Environment::new()));
loop {
out.write_all(PROMPT).unwrap();
out.flush().unwrap();
@ -35,7 +39,7 @@ fn start<R: BufRead, W: Write>(mut ip: R, mut out: W) {
}
let program = program.unwrap();
let evaluator = TreeWalker::new();
let obj = evaluator.eval(Node::Program(program), &mut environment);
let obj = evaluator.eval(Node::Program(program), environment.clone());
if let Some(node) = obj {
out.write_fmt(format_args!("{}\n", node.inspect())).unwrap();
out.flush().unwrap();