Repository: sharkdp/numbat (rust)

Issue:
Add logical operators
- Add `&&`, `||` binary operators
- Add `!` unary operator
- Simplify the `examples/binomial_coefficient.nbt` example program by collapsing some layers

Pull request:
Logical operators
Fix #196

Patch:
diff --git a/book/src/example-numbat_syntax.md b/book/src/example-numbat_syntax.md
index 51d7a10..7965511 100644
--- a/book/src/example-numbat_syntax.md
+++ b/book/src/example-numbat_syntax.md
@@ -92,10 +92,10 @@ unit thing                       # New base unit with automatically generated
 
 # 8. Conditionals
 
-fn step(x: Scalar) -> Scalar =   # The construct 'if <cond> then <expr> else <expr>'
-  if x < 0                       # is an expression, not a statement. It can span
-    then 0                       # multiple lines.
-    else 1
+fn bump(x: Scalar) -> Scalar =   # The construct 'if <cond> then <expr> else <expr>'
+  if x >= 0 && x <= 1            # is an expression, not a statement. It can span
+    then 1                       # multiple lines.
+    else 0
 
 # 9. Procedures
 
diff --git a/book/src/operations.md b/book/src/operations.md
index a070b36..255dc6f 100644
--- a/book/src/operations.md
+++ b/book/src/operations.md
@@ -15,6 +15,9 @@ Numbat operators and other language constructs, ordered by precedence form *high
 | subtraction               | `x - y`                              |
 | addition                  | `x + y`                              |
 | comparisons               | `x < y`, `x <= y`, `x ≤ y`, … `x == y`, `x != y` |
+| logical negation          | `!x`                                 |
+| logical 'and'             | `x && y`                             |
+| logical 'or'              | `x || y`                             |
 | unit conversion           | `x -> y`, `x → y`, `x ➞ y`, `x to y` |
 | conditionals              | `if x then y else z`                 |
 | reverse function call     | `x // f`                             |
diff --git a/examples/binomial_coefficient.nbt b/examples/binomial_coefficient.nbt
index c0c89f4..8d027c0 100644
--- a/examples/binomial_coefficient.nbt
+++ b/examples/binomial_coefficient.nbt
@@ -2,21 +2,15 @@
 #
 # Adapted from the Python version here:
 # https://en.wikipedia.org/wiki/Binomial_coefficient
-#
-# TODO: This could really benefit from logical and/or operators
 
 fn binomial_coefficient(n: Scalar, k: Scalar) -> Scalar =
-    if k < 0
+    if k < 0 || k > n
         then 0
-        else if k > n
-            then 0
-            else if k > n - k # Take advantage of symmetry
-                then binomial_coefficient(n, n - k)
-                else if k == 0
-                    then 1
-                    else if n <= 1
-                        then 1
-                        else binomial_coefficient(n - 1, k) + binomial_coefficient(n - 1, k - 1)
+        else if k > n - k # Take advantage of symmetry
+            then binomial_coefficient(n, n - k)
+            else if k == 0 || n <= 1
+                then 1
+                else binomial_coefficient(n - 1, k) + binomial_coefficient(n - 1, k - 1)
 
 assert_eq(binomial_coefficient(10, 0), 1)
 assert_eq(binomial_coefficient(10, 1), 10)
diff --git a/examples/booleans.nbt b/examples/booleans.nbt
index 8e65787..78ebb29 100644
--- a/examples/booleans.nbt
+++ b/examples/booleans.nbt
@@ -1,16 +1,12 @@
-fn not(a: Bool) = if a then false else true
-fn and(a: Bool, b: Bool) = if a then b else false
-fn or(a: Bool, b: Bool) = if a then true else b
+assert(!false)
+assert(!!true)
 
-assert(not(false))
-assert(not(not(true)))
+assert(true && true)
+assert(!(true && false))
+assert(!(false && true))
+assert(!(false && false))
 
-assert(and(true, true))
-assert(not(and(true, false)))
-assert(not(and(false, true)))
-assert(not(and(false, false)))
-
-assert(or(true, true))
-assert(or(true, false))
-assert(or(false, true))
-assert(not(or(false, false)))
+assert(true || true)
+assert(true || false)
+assert(false || true)
+assert(!(false || false))
diff --git a/examples/numbat_syntax.nbt b/examples/numbat_syntax.nbt
index 71a5ae6..6fc976e 100644
--- a/examples/numbat_syntax.nbt
+++ b/examples/numbat_syntax.nbt
@@ -87,10 +87,10 @@ unit thing                       # New base unit with automatically generated
 
 # 8. Conditionals
 
-fn step(x: Scalar) -> Scalar =   # The construct 'if <cond> then <expr> else <expr>'
-  if x < 0                       # is an expression, not a statement. It can span
-    then 0                       # multiple lines.
-    else 1
+fn bump(x: Scalar) -> Scalar =   # The construct 'if <cond> then <expr> else <expr>'
+  if x >= 0 && x <= 1            # is an expression, not a statement. It can span
+    then 1                       # multiple lines.
+    else 0
 
 # 9. Procedures
 
diff --git a/numbat/src/ast.rs b/numbat/src/ast.rs
index 1d73563..5787b7a 100644
--- a/numbat/src/ast.rs
+++ b/numbat/src/ast.rs
@@ -10,6 +10,7 @@ use num_traits::Signed;
 pub enum UnaryOperator {
     Factorial,
     Negate,
+    LogicalNeg,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -26,6 +27,8 @@ pub enum BinaryOperator {
     GreaterOrEqual,
     Equal,
     NotEqual,
+    LogicalAnd,
+    LogicalOr,
 }
 
 impl PrettyPrint for BinaryOperator {
@@ -45,6 +48,8 @@ impl PrettyPrint for BinaryOperator {
             GreaterOrEqual => m::space() + m::operator("≥") + m::space(),
             Equal => m::space() + m::operator("==") + m::space(),
             NotEqual => m::space() + m::operator("≠") + m::space(),
+            LogicalAnd => m::space() + m::operator("&&") + m::space(),
+            LogicalOr => m::space() + m::operator("||") + m::space(),
         }
     }
 }
@@ -124,6 +129,24 @@ macro_rules! identifier {
     }};
 }
 
+#[cfg(test)]
+macro_rules! boolean {
+    ( $name:expr ) => {{
+        crate::ast::Expression::Boolean(Span::dummy(), $name.into())
+    }};
+}
+
+#[cfg(test)]
+macro_rules! logical_neg {
+    ( $rhs:expr ) => {{
+        crate::ast::Expression::UnaryOperator {
+            op: UnaryOperator::LogicalNeg,
+            expr: Box::new($rhs),
+            span_op: Span::dummy(),
+        }
+    }};
+}
+
 #[cfg(test)]
 macro_rules! negate {
     ( $rhs:expr ) => {{
@@ -173,12 +196,16 @@ macro_rules! conditional {
 #[cfg(test)]
 pub(crate) use binop;
 #[cfg(test)]
+pub(crate) use boolean;
+#[cfg(test)]
 pub(crate) use conditional;
 #[cfg(test)]
 pub(crate) use factorial;
 #[cfg(test)]
 pub(crate) use identifier;
 #[cfg(test)]
+pub(crate) use logical_neg;
+#[cfg(test)]
 pub(crate) use negate;
 #[cfg(test)]
 pub(crate) use scalar;
diff --git a/numbat/src/bytecode_interpreter.rs b/numbat/src/bytecode_interpreter.rs
index 112ffa9..c93d668 100644
--- a/numbat/src/bytecode_interpreter.rs
+++ b/numbat/src/bytecode_interpreter.rs
@@ -86,6 +86,10 @@ impl BytecodeInterpreter {
                 self.compile_expression(lhs)?;
                 self.vm.add_op(Op::Factorial);
             }
+            Expression::UnaryOperator(_span, UnaryOperator::LogicalNeg, lhs, _type) => {
+                self.compile_expression(lhs)?;
+                self.vm.add_op(Op::LogicalNeg);
+            }
             Expression::BinaryOperator(_span, operator, lhs, rhs, _type) => {
                 self.compile_expression(lhs)?;
                 self.compile_expression(rhs)?;
@@ -103,6 +107,8 @@ impl BytecodeInterpreter {
                     BinaryOperator::GreaterOrEqual => Op::GreatorOrEqual,
                     BinaryOperator::Equal => Op::Equal,
                     BinaryOperator::NotEqual => Op::NotEqual,
+                    BinaryOperator::LogicalAnd => Op::LogicalAnd,
+                    BinaryOperator::LogicalOr => Op::LogicalOr,
                 };
                 self.vm.add_op(op);
             }
diff --git a/numbat/src/parser.rs b/numbat/src/parser.rs
index 8a699fe..ba46e4c 100644
--- a/numbat/src/parser.rs
+++ b/numbat/src/parser.rs
@@ -25,7 +25,10 @@
 //! expression      ::=   postfix_apply
 //! postfix_apply   ::=   condition ( "//" identifier ) *
 //! condition       ::=   ( "if" conversion "then" condition "else" condition ) | conversion
-//! conversion      ::=   comparison ( ( "→" | "->" | "to" ) comparison ) *
+//! conversion      ::=   logical_or ( ( "→" | "->" | "to" ) logical_or ) *
+//! logical_or      ::=   logical_and ( "||" logical_and ) *
+//! logical_and     ::=   logical_neg ( "&&" logical_neg ) *
+//! logical_neg     ::=   ( "!" logical_neg) | comparison
 //! comparison      ::=   term ( (">" | ">="| "≥" | "<" | "<=" | "≤" | "==" | "!=" | "≠" ) term ) *
 //! term            ::=   factor ( ( "+" | "-") factor ) *
 //! factor          ::=   unary ( ( "*" | "/") per_factor ) *
@@ -802,10 +805,10 @@ impl<'a> Parser<'a> {
     }
 
     fn conversion(&mut self) -> Result<Expression> {
-        let mut expr = self.comparison()?;
+        let mut expr = self.logical_or()?;
         while self.match_any(&[TokenKind::Arrow, TokenKind::To]).is_some() {
             let span_op = Some(self.last().unwrap().span);
-            let rhs = self.comparison()?;
+            let rhs = self.logical_or()?;
 
             expr = Expression::BinaryOperator {
                 op: BinaryOperator::ConvertTo,
@@ -817,6 +820,53 @@ impl<'a> Parser<'a> {
         Ok(expr)
     }
 
+    fn logical_or(&mut self) -> Result<Expression> {
+        let mut expr = self.logical_and()?;
+        while self.match_exact(TokenKind::LogicalOr).is_some() {
+            let span_op = Some(self.last().unwrap().span);
+            let rhs = self.logical_and()?;
+
+            expr = Expression::BinaryOperator {
+                op: BinaryOperator::LogicalOr,
+                lhs: Box::new(expr),
+                rhs: Box::new(rhs),
+                span_op,
+            };
+        }
+        Ok(expr)
+    }
+
+    fn logical_and(&mut self) -> Result<Expression> {
+        let mut expr = self.logical_neg()?;
+        while self.match_exact(TokenKind::LogicalAnd).is_some() {
+            let span_op = Some(self.last().unwrap().span);
+            let rhs = self.logical_neg()?;
+
+            expr = Expression::BinaryOperator {
+                op: BinaryOperator::LogicalAnd,
+                lhs: Box::new(expr),
+                rhs: Box::new(rhs),
+                span_op,
+            };
+        }
+        Ok(expr)
+    }
+
+    fn logical_neg(&mut self) -> Result<Expression> {
+        if self.match_exact(TokenKind::ExclamationMark).is_some() {
+            let span = self.last().unwrap().span;
+            let rhs = self.logical_neg()?;
+
+            Ok(Expression::UnaryOperator {
+                op: UnaryOperator::LogicalNeg,
+                expr: Box::new(rhs),
+                span_op: span,
+            })
+        } else {
+            self.comparison()
+        }
+    }
+
     fn comparison(&mut self) -> Result<Expression> {
         let mut expr = self.term()?;
         while let Some(token) = self.match_any(&[
@@ -1454,7 +1504,10 @@ mod tests {
     use std::fmt::Write;
 
     use super::*;
-    use crate::ast::{binop, conditional, factorial, identifier, negate, scalar, ReplaceSpans};
+    use crate::ast::{
+        binop, boolean, conditional, factorial, identifier, logical_neg, negate, scalar,
+        ReplaceSpans,
+    };
 
     #[track_caller]
     fn parse_as(inputs: &[&str], statement_expected: Statement) {
@@ -2283,6 +2336,44 @@ mod tests {
         );
     }
 
+    #[test]
+    fn logical_operation() {
+        // basic
+        assert_snapshot!(snap_parse(
+            "true || false"), @r###"
+        Expression(BinaryOperator { op: LogicalOr, lhs: Boolean(Span { start: SourceCodePositition { byte: 0, line: 1, position: 1 }, end: SourceCodePositition { byte: 4, line: 1, position: 5 }, code_source_id: 0 }, true), rhs: Boolean(Span { start: SourceCodePositition { byte: 8, line: 1, position: 9 }, end: SourceCodePositition { byte: 13, line: 1, position: 14 }, code_source_id: 0 }, false), span_op: Some(Span { start: SourceCodePositition { byte: 5, line: 1, position: 6 }, end: SourceCodePositition { byte: 7, line: 1, position: 8 }, code_source_id: 0 }) })
+        "###);
+        assert_snapshot!(snap_parse(
+            "true && false"), @r###"
+        Expression(BinaryOperator { op: LogicalAnd, lhs: Boolean(Span { start: SourceCodePositition { byte: 0, line: 1, position: 1 }, end: SourceCodePositition { byte: 4, line: 1, position: 5 }, code_source_id: 0 }, true), rhs: Boolean(Span { start: SourceCodePositition { byte: 8, line: 1, position: 9 }, end: SourceCodePositition { byte: 13, line: 1, position: 14 }, code_source_id: 0 }, false), span_op: Some(Span { start: SourceCodePositition { byte: 5, line: 1, position: 6 }, end: SourceCodePositition { byte: 7, line: 1, position: 8 }, code_source_id: 0 }) })
+        "###);
+        assert_snapshot!(snap_parse(
+            "!true"), @r###"
+        Expression(UnaryOperator { op: LogicalNeg, expr: Boolean(Span { start: SourceCodePositition { byte: 1, line: 1, position: 2 }, end: SourceCodePositition { byte: 5, line: 1, position: 6 }, code_source_id: 0 }, true), span_op: Span { start: SourceCodePositition { byte: 0, line: 1, position: 1 }, end: SourceCodePositition { byte: 1, line: 1, position: 2 }, code_source_id: 0 } })
+        "###);
+
+        // priority
+        #[rustfmt::skip]
+        parse_as_expression(
+            &["true || false && true || false"],
+            binop!( // the 'and' operator has the highest priority
+                binop!(boolean!(true), LogicalOr, binop!(boolean!(false), LogicalAnd, boolean!(true))),
+                LogicalOr,
+                boolean!(false)
+                )
+            );
+
+        #[rustfmt::skip]
+        parse_as_expression(
+            &["!true && false"],
+            binop!( // The negation has precedence over the 'and'
+                logical_neg!(boolean!(true)),
+                LogicalAnd,
+                boolean!(false)
+                )
+            );
+    }
+
     #[test]
     fn comparisons() {
         parse_as_expression(&["1 < 2"], binop!(scalar!(1.0), LessThan, scalar!(2.0)));
diff --git a/numbat/src/tokenizer.rs b/numbat/src/tokenizer.rs
index 9df25d3..399123d 100644
--- a/numbat/src/tokenizer.rs
+++ b/numbat/src/tokenizer.rs
@@ -75,6 +75,8 @@ pub enum TokenKind {
     GreaterThan,
     LessOrEqual,
     GreaterOrEqual,
+    LogicalAnd,
+    LogicalOr,
 
     // Keywords
     Let,
@@ -456,6 +458,8 @@ impl Tokenizer {
                 return Ok(None);
             }
             '\n' => TokenKind::Newline,
+            '&' if self.match_char('&') => TokenKind::LogicalAnd,
+            '|' if self.match_char('|') => TokenKind::LogicalOr,
             '*' if self.match_char('*') => TokenKind::Power,
             '+' => TokenKind::Plus,
             '*' | '·' | '⋅' | '×' => TokenKind::Multiply,
@@ -1001,6 +1005,39 @@ fn test_tokenize_string() {
     );
 }
 
+#[test]
+fn test_logical_operators() {
+    insta::assert_snapshot!(
+        tokenize_reduced_pretty("true || false").unwrap(),
+        @r###"
+    "true", True, (1, 1)
+    "||", LogicalOr, (1, 6)
+    "false", False, (1, 9)
+    "", Eof, (1, 14)
+    "###
+    );
+
+    insta::assert_snapshot!(
+        tokenize_reduced_pretty("true && false").unwrap(),
+        @r###"
+    "true", True, (1, 1)
+    "&&", LogicalAnd, (1, 6)
+    "false", False, (1, 9)
+    "", Eof, (1, 14)
+    "###
+    );
+
+    insta::assert_snapshot!(
+        tokenize_reduced_pretty("true | false").unwrap_err(),
+        @"Error at (1, 6): `Unexpected character: '|'`"
+    );
+
+    insta::assert_snapshot!(
+        tokenize_reduced_pretty("true & false").unwrap_err(),
+        @"Error at (1, 6): `Unexpected character: '&'`"
+    );
+}
+
 #[test]
 fn test_is_currency_char() {
     assert!(is_currency_char('€'));
diff --git a/numbat/src/typechecker.rs b/numbat/src/typechecker.rs
index 822e5b4..0ce939e 100644
--- a/numbat/src/typechecker.rs
+++ b/numbat/src/typechecker.rs
@@ -315,6 +315,9 @@ fn evaluate_const_expr(expr: &typed_ast::Expression) -> Result<Exponent> {
         e @ typed_ast::Expression::UnaryOperator(_, ast::UnaryOperator::Factorial, _, _) => Err(
             TypeCheckError::UnsupportedConstEvalExpression(e.full_span(), "factorial"),
         ),
+        e @ typed_ast::Expression::UnaryOperator(_, ast::UnaryOperator::LogicalNeg, _, _) => Err(
+            TypeCheckError::UnsupportedConstEvalExpression(e.full_span(), "logical"),
+        ),
         e @ typed_ast::Expression::BinaryOperator(_span_op, op, lhs_expr, rhs_expr, _) => {
             let lhs = evaluate_const_expr(lhs_expr)?;
             let rhs = evaluate_const_expr(rhs_expr)?;
@@ -366,6 +369,12 @@ fn evaluate_const_expr(expr: &typed_ast::Expression) -> Result<Exponent> {
                 | typed_ast::BinaryOperator::NotEqual => Err(
                     TypeCheckError::UnsupportedConstEvalExpression(e.full_span(), "comparison"),
                 ),
+                typed_ast::BinaryOperator::LogicalAnd | typed_ast::BinaryOperator::LogicalOr => {
+                    Err(TypeCheckError::UnsupportedConstEvalExpression(
+                        e.full_span(),
+                        "logical",
+                    ))
+                }
             }
         }
         e @ typed_ast::Expression::Identifier(..) => Err(
@@ -445,27 +454,27 @@ impl TypeChecker {
             ast::Expression::UnaryOperator { op, expr, span_op } => {
                 let checked_expr = self.check_expression(expr)?;
                 let type_ = checked_expr.get_type();
-                let dtype = match &type_ {
-                    Type::Dimension(d) => d.clone(),
-                    _ => {
-                        return Err(TypeCheckError::ExpectedDimensionType(
-                            checked_expr.full_span(),
-                            type_.clone(),
-                        ))
-                    }
-                };
-
-                match *op {
-                    ast::UnaryOperator::Factorial => {
+                match (&type_, op) {
+                    (Type::Dimension(dtype), ast::UnaryOperator::Factorial) => {
                         if !dtype.is_scalar() {
                             return Err(TypeCheckError::NonScalarFactorialArgument(
                                 expr.full_span(),
-                                dtype,
+                                dtype.clone(),
                             ));
                         }
                     }
-                    ast::UnaryOperator::Negate => {}
-                }
+                    (Type::Dimension(_), ast::UnaryOperator::Negate) => (),
+                    (Type::Boolean, ast::UnaryOperator::LogicalNeg) => (),
+                    (_, ast::UnaryOperator::LogicalNeg) => {
+                        return Err(TypeCheckError::ExpectedBool(expr.full_span()))
+                    }
+                    _ => {
+                        return Err(TypeCheckError::ExpectedDimensionType(
+                            checked_expr.full_span(),
+                            type_.clone(),
+                        ));
+                    }
+                };
 
                 typed_ast::Expression::UnaryOperator(*span_op, *op, Box::new(checked_expr), type_)
             }
@@ -507,6 +516,8 @@ impl TypeChecker {
                                     | typed_ast::BinaryOperator::GreaterOrEqual
                                     | typed_ast::BinaryOperator::Equal
                                     | typed_ast::BinaryOperator::NotEqual => "comparison".into(),
+                                    typed_ast::BinaryOperator::LogicalAnd => "and".into(),
+                                    typed_ast::BinaryOperator::LogicalOr => "or".into(),
                                 },
                                 span_expected: lhs.full_span(),
                                 expected_name: " left hand side",
@@ -580,6 +591,19 @@ impl TypeChecker {
                             ));
                         }
 
+                        Type::Boolean
+                    }
+                    typed_ast::BinaryOperator::LogicalAnd
+                    | typed_ast::BinaryOperator::LogicalOr => {
+                        let lhs_type = lhs_checked.get_type();
+                        let rhs_type = rhs_checked.get_type();
+
+                        if lhs_type != Type::Boolean {
+                            return Err(TypeCheckError::ExpectedBool(lhs.full_span()));
+                        } else if rhs_type != Type::Boolean {
+                            return Err(TypeCheckError::ExpectedBool(rhs.full_span()));
+                        }
+
                         Type::Boolean
                     }
                 };
diff --git a/numbat/src/typed_ast.rs b/numbat/src/typed_ast.rs
index 0f85685..6340132 100644
--- a/numbat/src/typed_ast.rs
+++ b/numbat/src/typed_ast.rs
@@ -538,6 +538,9 @@ impl PrettyPrint for Expression {
             UnaryOperator(_, self::UnaryOperator::Factorial, expr, _type) => {
                 with_parens(expr) + m::operator("!")
             }
+            UnaryOperator(_, self::UnaryOperator::LogicalNeg, expr, _type) => {
+                m::operator("!") + with_parens(expr)
+            }
             BinaryOperator(_, op, lhs, rhs, _type) => pretty_print_binop(op, lhs, rhs),
             FunctionCall(_, _, name, args, _type) => {
                 m::identifier(name)
diff --git a/numbat/src/vm.rs b/numbat/src/vm.rs
index 204d683..4ad6dff 100644
--- a/numbat/src/vm.rs
+++ b/numbat/src/vm.rs
@@ -65,6 +65,9 @@ pub enum Op {
     GreatorOrEqual,
     Equal,
     NotEqual,
+    LogicalAnd,
+    LogicalOr,
+    LogicalNeg,
 
     /// Move IP forward by the given offset argument if the popped-of value on
     /// top of the stack is false.
@@ -118,6 +121,9 @@ impl Op {
             | Op::GreatorOrEqual
             | Op::Equal
             | Op::NotEqual
+            | Op::LogicalAnd
+            | Op::LogicalOr
+            | Op::LogicalNeg
             | Op::FullSimplify
             | Op::Return
             | Op::GetLastResult => 0,
@@ -146,6 +152,9 @@ impl Op {
             Op::GreatorOrEqual => "GreatorOrEqual",
             Op::Equal => "Equal",
             Op::NotEqual => "NotEqual",
+            Op::LogicalAnd => "LogicalAnd",
+            Op::LogicalOr => "LogicalOr",
+            Op::LogicalNeg => "LogicalNeg",
             Op::JumpIfFalse => "JumpIfFalse",
             Op::Jump => "Jump",
             Op::Call => "Call",
@@ -478,6 +487,10 @@ impl Vm {
         self.stack.push(Value::Quantity(quantity));
     }
 
+    fn push_bool(&mut self, boolean: bool) {
+        self.stack.push(Value::Boolean(boolean));
+    }
+
     fn push(&mut self, value: Value) {
         self.stack.push(value);
     }
@@ -634,6 +647,21 @@ impl Vm {
                     };
                     self.push(Value::Boolean(result));
                 }
+                op @ (Op::LogicalAnd | Op::LogicalOr) => {
+                    let rhs = self.pop_bool();
+                    let lhs = self.pop_bool();
+
+                    let result = match op {
+                        Op::LogicalAnd => lhs && rhs,
+                        Op::LogicalOr => lhs || rhs,
+                        _ => unreachable!(),
+                    };
+                    self.push_bool(result);
+                }
+                Op::LogicalNeg => {
+                    let rhs = self.pop_bool();
+                    self.push_bool(!rhs);
+                }
                 Op::Negate => {
                     let rhs = self.pop_quantity();
                     self.push_quantity(-rhs);


Test patch:
diff --git a/numbat/tests/interpreter.rs b/numbat/tests/interpreter.rs
index bc6ed98..8fb5582 100644
--- a/numbat/tests/interpreter.rs
+++ b/numbat/tests/interpreter.rs
@@ -5,6 +5,7 @@ use common::get_test_context;
 use insta::assert_snapshot;
 use numbat::markup::{Formatter, PlainTextFormatter};
 use numbat::resolver::CodeSource;
+use numbat::NumbatError;
 use numbat::{pretty_print::PrettyPrint, Context, InterpreterResult};
 
 #[track_caller]
@@ -21,6 +22,24 @@ fn expect_output_with_context(ctx: &mut Context, code: &str, expected_output: im
     }
 }
 
+#[track_caller]
+fn fail(code: &str) -> NumbatError {
+    let mut ctx = get_test_context();
+    let ret = ctx.interpret(code, CodeSource::Internal);
+    match ret {
+        Err(e) => e,
+        Ok((_stmts, ret)) => {
+            if let InterpreterResult::Value(val) = ret {
+                let fmt = PlainTextFormatter {};
+                let output = fmt.format(&val.pretty_print(), false);
+                panic!("was supposed to fail but instead got:\n{}", output.trim())
+            } else {
+                panic!("was supposed to fail but instead got:\n{:?}", ret)
+            }
+        }
+    }
+}
+
 #[track_caller]
 fn expect_output(code: &str, expected_output: impl AsRef<str>) {
     let mut ctx = get_test_context();
@@ -482,6 +501,35 @@ fn test_comparisons() {
     expect_output("201 cm != 2 m", "true");
 }
 
+#[test]
+fn test_logical() {
+    // negation
+    expect_output("!true", "false");
+    expect_output("!false", "true");
+
+    // or
+    expect_output("true || false", "true");
+    expect_output("false || false", "false");
+
+    // and
+    expect_output("true && false", "false");
+    expect_output("true && true", "true");
+
+    // priority
+    expect_output("false || true && false", "false");
+    expect_output("false || true && !false", "true");
+
+    // Errors
+    insta::assert_display_snapshot!(fail("1 || 2"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("true || 2"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("1 || true"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("1 && 2"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("true && 2"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("1 && true"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("!1"), @"Expected boolean value");
+    insta::assert_display_snapshot!(fail("!1 || true"), @"Expected boolean value");
+}
+
 #[test]
 fn test_conditionals() {
     expect_output("if 1 < 2 then 3 else 4", "3");