From 344a11084e17678fe8a4dc5c4d28abd3039d3d21 Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Thu, 27 May 2021 10:05:57 -0500 Subject: [PATCH 1/6] Add some unit tests to `interpret.rs` Currently `overflow_should_not_panic` and `variable_persistence` both fail due to bugs in the interpreter. --- src/error.rs | 1 + src/interpret.rs | 104 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/error.rs b/src/error.rs index c994bbd0..064079d2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,4 +15,5 @@ pub enum ErrorKind { MeloVariable(String), TypeError(String), TopLevelBreak, + ArithmeticError, } diff --git a/src/interpret.rs b/src/interpret.rs index e23799fd..d1e2230f 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -269,3 +269,107 @@ impl ExecEnv { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn basic_expression_test() { + // Check that 2 + 2 = 4. + let mut env = ExecEnv::new(); + assert_eq!( + env.eval_items(&[Item::Expr(Expr::Add { + left: Box::new(Expr::Literal(Value::Int(2))), + right: Box::new(Expr::Literal(Value::Int(2))), + })]) + .unwrap(), + Value::Int(4) + ) + } + + #[test] + fn type_errors() { + // The sum of an integer and a boolean results in a type + // error. + let mut env = ExecEnv::new(); + assert!(matches!( + env.eval_items(&[Item::Expr(Expr::Add { + left: Box::new(Expr::Literal(Value::Int(i32::MAX))), + right: Box::new(Expr::Literal(Value::Bool(false))), + })]), + Err(Error { + kind: ErrorKind::TypeError(_), + position: _, + }) + )); + } + + #[test] + fn overflow_should_not_panic() { + // Integer overflow should throw a recoverable error instead + // of panicking. + let mut env = ExecEnv::new(); + assert!(matches!( + env.eval_items(&[Item::Expr(Expr::Add { + left: Box::new(Expr::Literal(Value::Int(i32::MAX))), + right: Box::new(Expr::Literal(Value::Int(1))), + })]), + Err(Error { + kind: ErrorKind::ArithmeticError, + position: _, + }) + )); + } + + // From here on out, I'll use this function to parse and run + // expressions, because writing out abstract syntax trees by hand + // takes forever and is error-prone. + fn eval(env: &mut ExecEnv, src: &str) -> Result { + let mut parser = crate::parser::Parser::new(src); + + // We can assume there won't be any syntax errors in the + // interpreter tests. + let ast = parser.init().unwrap(); + env.eval_items(&ast) + } + + #[test] + fn variable_decl_and_assignment() { + // Declaring and reading from a variable. + assert_eq!( + eval(&mut ExecEnv::new(), "var foo = 32; foo + 1").unwrap(), + Value::Int(33) + ); + + // It should be possible to overwrite variables as well. + assert_eq!( + eval(&mut ExecEnv::new(), "var bar = 10; bar = 20; bar").unwrap(), + Value::Int(20) + ); + } + + #[test] + fn variable_persistence() { + // Global variables should persist between invocations of + // ExecEnv::eval_items(). + let mut env = ExecEnv::new(); + eval(&mut env, "var foo = 32;").unwrap(); + assert_eq!(eval(&mut env, "foo").unwrap(), Value::Int(32)); + } + + #[test] + fn scope_visibility_rules() { + // Declaration and assignment of variables declared in an `if` + // statement should have no effect on those declared outside + // of it. + assert_eq!( + eval( + &mut ExecEnv::new(), + "var foo = 1; if (true) { var foo = 2; foo = 3; } foo" + ) + .unwrap(), + Value::Int(1) + ); + } +} From acfd81ead2962899bfcdd2903f40befd80d6a41e Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Sun, 30 May 2021 13:24:16 -0500 Subject: [PATCH 2/6] More thorough unit tests --- src/interpret.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/interpret.rs b/src/interpret.rs index d1e2230f..4f3dc4a6 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -320,6 +320,18 @@ mod tests { position: _, }) )); + + // And the same for divide by zero. + assert!(matches!( + env.eval_items(&[Item::Expr(Expr::Divide { + left: Box::new(Expr::Literal(Value::Int(1))), + right: Box::new(Expr::Literal(Value::Int(0))), + })]), + Err(Error { + kind: ErrorKind::ArithmeticError, + position: _, + }) + )); } // From here on out, I'll use this function to parse and run @@ -347,6 +359,10 @@ mod tests { eval(&mut ExecEnv::new(), "var bar = 10; bar = 20; bar").unwrap(), Value::Int(20) ); + + // But variable assignment should be illegal when the variable + // hasn't been declared in advance. + eval(&mut ExecEnv::new(), "baz = 10;").unwrap_err(); } #[test] From 07195d4cf61f2cdc34355e5b52897072f60a576e Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Sun, 30 May 2021 13:26:10 -0500 Subject: [PATCH 3/6] Fix panic on arithmetic error Divide by zero and add, subtract, or multiply with overflow are all caught now and reported as an ArithmeticError, rather than causing the interpreter to panic. --- src/interpret.rs | 49 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/src/interpret.rs b/src/interpret.rs index 4f3dc4a6..caf4b3af 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -110,21 +110,42 @@ impl ExecEnv { use Expr::*; use Value::*; - // NOTE(Alex): This is quite nasty, and should probably be - // re-done using macros or something. + // NOTE(Alex): This is really quite horrible. I think the only + // real way to clean it up would be to re-engineer the AST's + // representation to be more hierarchical: rather than having + // e.g. "Expr::Add" and "Expr::Subtract" each with a "left" + // and "right" struct member, have something like + // "Expr::Binary { oper: BinOp, left: Box, right: + // Box }". That way we could factor out a whole bunch of + // common code here. + // + // That work should probably wait for Ondra's new parser to + // come in, however. Ok(match expr { - Add { left, right } => { - Int(i32::try_from(self.eval_expr(left)?)? + i32::try_from(self.eval_expr(right)?)?) - } - Subtract { left, right } => { - Int(i32::try_from(self.eval_expr(left)?)? - i32::try_from(self.eval_expr(right)?)?) - } - Multiply { left, right } => { - Int(i32::try_from(self.eval_expr(left)?)? * i32::try_from(self.eval_expr(right)?)?) - } - Divide { left, right } => { - Int(i32::try_from(self.eval_expr(left)?)? / i32::try_from(self.eval_expr(right)?)?) - } + Add { left, right } => Int(i32::try_from(self.eval_expr(left)?)? + .checked_add(i32::try_from(self.eval_expr(right)?)?) + .ok_or(Error { + kind: ErrorKind::ArithmeticError, + position: 0..0, + })?), + Subtract { left, right } => Int(i32::try_from(self.eval_expr(left)?)? + .checked_sub(i32::try_from(self.eval_expr(right)?)?) + .ok_or(Error { + kind: ErrorKind::ArithmeticError, + position: 0..0, + })?), + Multiply { left, right } => Int(i32::try_from(self.eval_expr(left)?)? + .checked_mul(i32::try_from(self.eval_expr(right)?)?) + .ok_or(Error { + kind: ErrorKind::ArithmeticError, + position: 0..0, + })?), + Divide { left, right } => Int(i32::try_from(self.eval_expr(left)?)? + .checked_div(i32::try_from(self.eval_expr(right)?)?) + .ok_or(Error { + kind: ErrorKind::ArithmeticError, + position: 0..0, + })?), Lt { left, right } => { Bool(i32::try_from(self.eval_expr(left)?)? < i32::try_from(self.eval_expr(right)?)?) } From 326d0511e7e8f5c45a6bb41d541e61106772059f Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Wed, 2 Jun 2021 15:29:31 -0500 Subject: [PATCH 4/6] Add Brainfuck functio interpretation BF functios can now be declared and called from AbleScript code. Function parameters supplied from AbleScript are serialized manually into byte sequences using the `BfWriter` trait, and are available from BF code using the input operator, `,`. At the moment, BF functios simply write output to stdout, and are therefore incapable of communicating results to the rest of the program; this might change in the future if we can get something close to pass-by-reference working. --- src/error.rs | 3 ++ src/interpret.rs | 76 ++++++++++++++++++++++++++++++------- src/variables.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 160 insertions(+), 18 deletions(-) diff --git a/src/error.rs b/src/error.rs index 064079d2..2d2a3541 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,7 @@ use std::ops::Range; +use crate::brian::InterpretError; + #[derive(Debug, Clone)] pub struct Error { pub kind: ErrorKind, @@ -16,4 +18,5 @@ pub enum ErrorKind { TypeError(String), TopLevelBreak, ArithmeticError, + BfInterpretError(InterpretError), } diff --git a/src/interpret.rs b/src/interpret.rs index caf4b3af..38887eaa 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -8,13 +8,16 @@ #[deny(missing_docs)] use std::collections::HashMap; -use std::convert::TryFrom; +use std::{ + convert::TryFrom, + io::{stdout, Write}, +}; use crate::{ base_55, error::{Error, ErrorKind}, parser::item::{Expr, Iden, Item, Stmt}, - variables::{Value, Variable}, + variables::{Functio, Value, Variable}, }; /// An environment for executing AbleScript code. @@ -178,29 +181,64 @@ impl ExecEnv { None => Value::Nul, }; - // There's always at least one stack frame on the - // stack if we're evaluating something, so we can - // `unwrap` here. - self.stack.iter_mut().last().unwrap().variables.insert( - iden.0.clone(), - Variable { - melo: false, - value: init, - }, - ); + self.decl_var(&iden.0, init); } Stmt::FunctionDeclaration { iden: _, args: _, body: _, } => todo!(), - Stmt::BfFDeclaration { iden: _, body: _ } => todo!(), + Stmt::BfFDeclaration { iden, body } => { + self.decl_var( + &iden.0, + Value::Functio(Functio::BfFunctio(body.as_bytes().into())), + ); + } Stmt::If { cond, body } => { if self.eval_expr(cond)?.into() { return self.eval_items_hs(body); } } - Stmt::FunctionCall { iden: _, args: _ } => todo!(), + Stmt::FunctionCall { iden, args } => { + let func = self.get_var(&iden.0)?; + match func { + Value::Functio(func) => { + match func { + Functio::BfFunctio(body) => { + use crate::variables::BfWriter; + let mut input: Vec = vec![]; + for arg in args { + input.write_value(&self.eval_expr(arg)?); + } + println!("input = {:?}", input); + let mut output = vec![]; + + crate::brian::interpret_with_io(&body, &input as &[_], &mut output) + .map_err(|e| Error { + kind: ErrorKind::BfInterpretError(e), + position: 0..0, + })?; + + // I guess Brainfuck functions write + // output to stdout? It's not quite + // clear to me what else to do. ~~Alex + stdout() + .write_all(&output) + .expect("Failed to write to stdout"); + } + Functio::AbleFunctio(_) => { + todo!() + } + } + } + _ => { + return Err(Error { + kind: ErrorKind::TypeError(iden.0.to_owned()), + position: 0..0, + }) + } + } + } Stmt::Loop { body } => loop { let res = self.eval_items_hs(body)?; match res { @@ -289,6 +327,16 @@ impl ExecEnv { }), } } + + /// Declares a new variable, with the given initial value. + fn decl_var(&mut self, name: &str, value: Value) { + self.stack + .iter_mut() + .last() + .expect("Declaring variable on empty stack") + .variables + .insert(name.to_owned(), Variable { melo: false, value }); + } } #[cfg(test)] diff --git a/src/variables.rs b/src/variables.rs index 496138a5..0ef0e14e 100644 --- a/src/variables.rs +++ b/src/variables.rs @@ -1,8 +1,11 @@ -use std::{convert::TryFrom, fmt::Display}; +use std::{convert::TryFrom, fmt::Display, io::Write}; use rand::Rng; -use crate::error::{Error, ErrorKind}; +use crate::{ + error::{Error, ErrorKind}, + parser::item::Item, +}; #[derive(Debug, Clone, PartialEq)] pub enum Abool { @@ -31,23 +34,45 @@ impl From for bool { } } +#[derive(Debug, Clone, PartialEq)] +pub enum Functio { + BfFunctio(Vec), + AbleFunctio(Vec), +} + #[derive(Debug, Clone, PartialEq)] pub enum Value { + Nul, Str(String), Int(i32), Bool(bool), Abool(Abool), - Nul, + Functio(Functio), } impl Display for Value { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + Value::Nul => write!(f, "nul"), Value::Str(v) => write!(f, "{}", v), Value::Int(v) => write!(f, "{}", v), Value::Bool(v) => write!(f, "{}", v), Value::Abool(v) => write!(f, "{}", v), - Value::Nul => write!(f, "nul"), + Value::Functio(v) => match v { + Functio::BfFunctio(source) => { + write!( + f, + "{}", + String::from_utf8(source.to_owned()) + .expect("Brainfuck functio source should be UTF-8") + ) + } + Functio::AbleFunctio(source) => { + // TODO: what's the proper way to display an + // AbleScript functio? + write!(f, "{:?}", source) + } + }, } } } @@ -84,6 +109,8 @@ impl From for bool { Value::Str(s) => s.len() != 0, // 0 is falsey, nonzero is truthy. Value::Int(x) => x != 0, + // Functios are always truthy. + Value::Functio(_) => true, // And nul is truthy as a symbol of the fact that the // deep, fundamental truth of this world is nothing but // the eternal void. @@ -92,6 +119,70 @@ impl From for bool { } } +/// Allows writing AbleScript values to Brainfuck. +/// +/// This trait is blanket implemented for all `Write`rs, but should +/// typically only be used for `Write`rs that cannot fail, e.g., +/// `Vec`, because any IO errors will cause a panic. +/// +/// The mapping from values to encodings is as follows, where all +/// multi-byte integers are little-endian: +/// +/// | AbleScript representation | Brainfuck representation | +/// |---------------------------|-----------------------------------------------------------| +/// | Nul | 00 | +/// | Str | 01 [length, 4 bytes] [string, \[LENGTH\] bytes, as UTF-8] | +/// | Int | 02 [value, 4 bytes] | +/// | Bool | 03 00 false, 03 01 true. | +/// | Abool | 04 00 never, 04 01 always, 04 02 sometimes. | +/// | Brainfuck Functio | 05 00 [length, 4 bytes] [source code, \[LENGTH\] bytes] | +/// | AbleScript Functio | 05 01 (todo, not yet finalized or implemented) | +/// +/// The existing mappings should never change, as they are directly +/// visible from Brainfuck code and modifying them would break a +/// significant amount of AbleScript code. If more types are added in +/// the future, they should be assigned the remaining discriminant +/// bytes from 06..FF. +pub trait BfWriter { + /// Write a value. Panic if writing fails for any reason. + fn write_value(&mut self, value: &Value); +} + +impl BfWriter for T { + fn write_value(&mut self, value: &Value) { + match value { + Value::Nul => self.write_all(&[0]), + Value::Str(s) => self + .write_all(&[1]) + .and_then(|_| self.write_all(&(s.len() as u32).to_le_bytes())) + .and_then(|_| self.write_all(&s.as_bytes())), + Value::Int(v) => self + .write_all(&[2]) + .and_then(|_| self.write_all(&v.to_le_bytes())), + Value::Bool(b) => self + .write_all(&[3]) + .and_then(|_| self.write_all(&[*b as _])), + Value::Abool(a) => self.write_all(&[4]).and_then(|_| { + self.write_all(&[match *a { + Abool::Never => 0, + Abool::Sometimes => 2, + Abool::Always => 1, + }]) + }), + Value::Functio(f) => self.write_all(&[5]).and_then(|_| match f { + Functio::BfFunctio(f) => self + .write_all(&[0]) + .and_then(|_| self.write_all(&(f.len() as u32).to_le_bytes())) + .and_then(|_| self.write_all(&f)), + Functio::AbleFunctio(_) => { + todo!() + } + }), + } + .expect("Failed to write to Brainfuck input"); + } +} + #[derive(Debug)] pub struct Variable { pub melo: bool, From ce02aebd91ea94752e08d7d4a3906347322bfd86 Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Wed, 2 Jun 2021 18:20:30 -0500 Subject: [PATCH 5/6] Use `impl Value` rather than `trait BfWriter` It probably makes more sense to be writing values with `Value::bf_write` than to go for the complexity of using a trait to allow `Writer::write_value` (which also looks ambiguous because it's not immediately clear that a generic name like "write_value" relates to BF or AbleScript). --- src/interpret.rs | 3 +- src/variables.rs | 121 ++++++++++++++++++++++------------------------- 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/src/interpret.rs b/src/interpret.rs index 38887eaa..a10ec6f6 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -205,10 +205,9 @@ impl ExecEnv { Value::Functio(func) => { match func { Functio::BfFunctio(body) => { - use crate::variables::BfWriter; let mut input: Vec = vec![]; for arg in args { - input.write_value(&self.eval_expr(arg)?); + self.eval_expr(arg)?.bf_write(&mut input); } println!("input = {:?}", input); let mut output = vec![]; diff --git a/src/variables.rs b/src/variables.rs index 0ef0e14e..a6abee9e 100644 --- a/src/variables.rs +++ b/src/variables.rs @@ -50,6 +50,63 @@ pub enum Value { Functio(Functio), } +impl Value { + /// Writes an AbleScript value to a Brainfuck input stream. This + /// should generally only be called on `Write`rs that cannot fail, + /// e.g., `Vec`, because any IO errors will cause a panic. + /// + /// The mapping from values to encodings is as follows, where all + /// multi-byte integers are little-endian: + /// + /// | AbleScript representation | Brainfuck representation | + /// |---------------------------|-------------------------------------------------------------| + /// | Nul | `00` | + /// | Str | `01` [length, 4 bytes] [string, \[LENGTH\] bytes, as UTF-8] | + /// | Int | `02` [value, 4 bytes] | + /// | Bool | `03` `00` false, `03` `01` true. | + /// | Abool | `04` `00` never, `04` `01` always, `04` `02` sometimes. | + /// | Brainfuck Functio | `05` `00` [length, 4 bytes] [source code, \[LENGTH\] bytes] | + /// | AbleScript Functio | `05` `01` (todo, not yet finalized or implemented) | + /// + /// The existing mappings should never change, as they are + /// directly visible from Brainfuck code and modifying them would + /// break a significant amount of AbleScript code. If more types + /// are added in the future, they should be assigned the remaining + /// discriminant bytes from 06..FF. + pub fn bf_write(&mut self, stream: &mut impl Write) { + match self { + Value::Nul => stream.write_all(&[0]), + Value::Str(s) => stream + .write_all(&[1]) + .and_then(|_| stream.write_all(&(s.len() as u32).to_le_bytes())) + .and_then(|_| stream.write_all(&s.as_bytes())), + Value::Int(v) => stream + .write_all(&[2]) + .and_then(|_| stream.write_all(&v.to_le_bytes())), + Value::Bool(b) => stream + .write_all(&[3]) + .and_then(|_| stream.write_all(&[*b as _])), + Value::Abool(a) => stream.write_all(&[4]).and_then(|_| { + stream.write_all(&[match *a { + Abool::Never => 0, + Abool::Sometimes => 2, + Abool::Always => 1, + }]) + }), + Value::Functio(f) => stream.write_all(&[5]).and_then(|_| match f { + Functio::BfFunctio(f) => stream + .write_all(&[0]) + .and_then(|_| stream.write_all(&(f.len() as u32).to_le_bytes())) + .and_then(|_| stream.write_all(&f)), + Functio::AbleFunctio(_) => { + todo!() + } + }), + } + .expect("Failed to write to Brainfuck input"); + } +} + impl Display for Value { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -119,70 +176,6 @@ impl From for bool { } } -/// Allows writing AbleScript values to Brainfuck. -/// -/// This trait is blanket implemented for all `Write`rs, but should -/// typically only be used for `Write`rs that cannot fail, e.g., -/// `Vec`, because any IO errors will cause a panic. -/// -/// The mapping from values to encodings is as follows, where all -/// multi-byte integers are little-endian: -/// -/// | AbleScript representation | Brainfuck representation | -/// |---------------------------|-----------------------------------------------------------| -/// | Nul | 00 | -/// | Str | 01 [length, 4 bytes] [string, \[LENGTH\] bytes, as UTF-8] | -/// | Int | 02 [value, 4 bytes] | -/// | Bool | 03 00 false, 03 01 true. | -/// | Abool | 04 00 never, 04 01 always, 04 02 sometimes. | -/// | Brainfuck Functio | 05 00 [length, 4 bytes] [source code, \[LENGTH\] bytes] | -/// | AbleScript Functio | 05 01 (todo, not yet finalized or implemented) | -/// -/// The existing mappings should never change, as they are directly -/// visible from Brainfuck code and modifying them would break a -/// significant amount of AbleScript code. If more types are added in -/// the future, they should be assigned the remaining discriminant -/// bytes from 06..FF. -pub trait BfWriter { - /// Write a value. Panic if writing fails for any reason. - fn write_value(&mut self, value: &Value); -} - -impl BfWriter for T { - fn write_value(&mut self, value: &Value) { - match value { - Value::Nul => self.write_all(&[0]), - Value::Str(s) => self - .write_all(&[1]) - .and_then(|_| self.write_all(&(s.len() as u32).to_le_bytes())) - .and_then(|_| self.write_all(&s.as_bytes())), - Value::Int(v) => self - .write_all(&[2]) - .and_then(|_| self.write_all(&v.to_le_bytes())), - Value::Bool(b) => self - .write_all(&[3]) - .and_then(|_| self.write_all(&[*b as _])), - Value::Abool(a) => self.write_all(&[4]).and_then(|_| { - self.write_all(&[match *a { - Abool::Never => 0, - Abool::Sometimes => 2, - Abool::Always => 1, - }]) - }), - Value::Functio(f) => self.write_all(&[5]).and_then(|_| match f { - Functio::BfFunctio(f) => self - .write_all(&[0]) - .and_then(|_| self.write_all(&(f.len() as u32).to_le_bytes())) - .and_then(|_| self.write_all(&f)), - Functio::AbleFunctio(_) => { - todo!() - } - }), - } - .expect("Failed to write to Brainfuck input"); - } -} - #[derive(Debug)] pub struct Variable { pub melo: bool, From 528de718dcac91a46526cc902d98e2c443e2eca4 Mon Sep 17 00:00:00 2001 From: Alex Bethel Date: Wed, 2 Jun 2021 18:41:20 -0500 Subject: [PATCH 6/6] Clean up `eval_expr` some more I keep going back and forth on how I want this block to look :P Anyway, this is *probably* its final-ish form, until the parser gets re-written. --- src/interpret.rs | 127 +++++++++++++++++++++++++++++------------------ 1 file changed, 79 insertions(+), 48 deletions(-) diff --git a/src/interpret.rs b/src/interpret.rs index a10ec6f6..0753d94f 100644 --- a/src/interpret.rs +++ b/src/interpret.rs @@ -113,55 +113,86 @@ impl ExecEnv { use Expr::*; use Value::*; - // NOTE(Alex): This is really quite horrible. I think the only - // real way to clean it up would be to re-engineer the AST's - // representation to be more hierarchical: rather than having - // e.g. "Expr::Add" and "Expr::Subtract" each with a "left" - // and "right" struct member, have something like - // "Expr::Binary { oper: BinOp, left: Box, right: - // Box }". That way we could factor out a whole bunch of - // common code here. - // - // That work should probably wait for Ondra's new parser to - // come in, however. + // NOTE(Alex): This block will get a whole lot cleaner once + // Ondra's parser stuff gets merged (specifically 97fb19e). + // For now, though, we've got a bunch of manually-checked + // unreachable!()s in here which makes me sad... Ok(match expr { - Add { left, right } => Int(i32::try_from(self.eval_expr(left)?)? - .checked_add(i32::try_from(self.eval_expr(right)?)?) - .ok_or(Error { - kind: ErrorKind::ArithmeticError, - position: 0..0, - })?), - Subtract { left, right } => Int(i32::try_from(self.eval_expr(left)?)? - .checked_sub(i32::try_from(self.eval_expr(right)?)?) - .ok_or(Error { - kind: ErrorKind::ArithmeticError, - position: 0..0, - })?), - Multiply { left, right } => Int(i32::try_from(self.eval_expr(left)?)? - .checked_mul(i32::try_from(self.eval_expr(right)?)?) - .ok_or(Error { - kind: ErrorKind::ArithmeticError, - position: 0..0, - })?), - Divide { left, right } => Int(i32::try_from(self.eval_expr(left)?)? - .checked_div(i32::try_from(self.eval_expr(right)?)?) - .ok_or(Error { - kind: ErrorKind::ArithmeticError, - position: 0..0, - })?), - Lt { left, right } => { - Bool(i32::try_from(self.eval_expr(left)?)? < i32::try_from(self.eval_expr(right)?)?) - } - Gt { left, right } => { - Bool(i32::try_from(self.eval_expr(left)?)? > i32::try_from(self.eval_expr(right)?)?) - } - Eq { left, right } => Bool(self.eval_expr(left)? == self.eval_expr(right)?), - Neq { left, right } => Bool(self.eval_expr(left)? != self.eval_expr(right)?), - And { left, right } => { - Bool(bool::from(self.eval_expr(left)?) && bool::from(self.eval_expr(right)?)) - } - Or { left, right } => { - Bool(bool::from(self.eval_expr(left)?) || bool::from(self.eval_expr(right)?)) + // Binary expressions. + Add { left, right } + | Subtract { left, right } + | Multiply { left, right } + | Divide { left, right } + | Lt { left, right } + | Gt { left, right } + | Eq { left, right } + | Neq { left, right } + | And { left, right } + | Or { left, right } => { + let left = self.eval_expr(left)?; + let right = self.eval_expr(right)?; + + match expr { + // Arithmetic operators. + Add { .. } + | Subtract { .. } + | Multiply { .. } + | Divide { .. } => { + let left = i32::try_from(left)?; + let right = i32::try_from(right)?; + + let res = match expr { + Add { .. } => left.checked_add(right), + Subtract { .. } => left.checked_sub(right), + Multiply { .. } => left.checked_mul(right), + Divide { .. } => left.checked_div(right), + _ => unreachable!(), + } + .ok_or(Error { + kind: ErrorKind::ArithmeticError, + position: 0..0, + })?; + Int(res) + } + + // Numeric comparisons. + Lt { .. } | Gt { .. } => { + let left = i32::try_from(left)?; + let right = i32::try_from(right)?; + + let res = match expr { + Lt { .. } => left < right, + Gt { .. } => left > right, + _ => unreachable!(), + }; + Bool(res) + } + + // General comparisons. + Eq { .. } | Neq { .. } => { + let res = match expr { + Eq { .. } => left == right, + Neq { .. } => left != right, + _ => unreachable!(), + }; + Bool(res) + } + + // Logical connectives. + And { .. } | Or { .. } => { + let left = bool::from(left); + let right = bool::from(right); + let res = match expr { + And { .. } => left && right, + Or { .. } => left || right, + _ => unreachable!(), + }; + Bool(res) + } + + // That's all the binary operations. + _ => unreachable!(), + } } Not(expr) => Bool(!bool::from(self.eval_expr(expr)?)), Literal(value) => value.clone(),