From 7bfefd3ba11d8fb01c3699084a2b9b312e967a65 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 c994bbd..064079d 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 e23799f..d1e2230 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 d13c22a473b0b5929826d671d62278d24f9e61fd 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 d1e2230..4f3dc4a 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 b3866eea9e0c54468a9232bf5e008e9e7ae5a0e0 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 4f3dc4a..caf4b3a 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 2c35691ec4086aa367bb2e7c240d905865f47e3c 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 064079d..2d2a354 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 caf4b3a..38887ea 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 496138a..0ef0e14 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 d80f47716d17a4248fd6594325d0dde0859dde8e 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 38887ea..a10ec6f 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 0ef0e14..a6abee9 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 98b2fae6f30101bea94840e27e1764357083ab7f 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 a10ec6f..0753d94 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(),