From 43066cfce6d7b67b9b48f78387d95931ef8fafd1 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 3 Jun 2025 16:34:16 +0200 Subject: [PATCH 01/18] Implement most operators as functions --- ndc_lib/src/ast/expression.rs | 30 +- ndc_lib/src/ast/parser.rs | 86 +++--- ndc_lib/src/interpreter/environment.rs | 27 ++ ndc_lib/src/interpreter/evaluate.rs | 141 +++------ ndc_lib/src/interpreter/function.rs | 82 +++++- ndc_lib/src/interpreter/int.rs | 14 +- ndc_lib/src/interpreter/num.rs | 107 +++---- ndc_lib/src/interpreter/value.rs | 6 + ndc_lib/src/stdlib/math.rs | 271 ++++++++++++++++-- ndc_macros/src/function.rs | 2 +- .../programs/001_math/019_bit_operations.ndct | 4 - 11 files changed, 519 insertions(+), 251 deletions(-) diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index bd9f9511..3ee3aca0 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -1,4 +1,4 @@ -use crate::ast::operator::{BinaryOperator, LogicalOperator, UnaryOperator}; +use crate::ast::operator::{BinaryOperator, LogicalOperator}; use crate::ast::parser::Error as ParseError; use crate::interpreter::evaluate::EvaluationError; use crate::lexer::Span; @@ -25,17 +25,7 @@ pub enum Expression { BigIntLiteral(BigInt), ComplexLiteral(Complex64), Identifier(String), - // Statement(Box), - Unary { - operator: UnaryOperator, - expression: Box, - }, - Binary { - left: Box, - operator: BinaryOperator, - right: Box, - }, Logical { left: Box, operator: LogicalOperator, @@ -288,24 +278,6 @@ impl fmt::Debug for ExpressionLocation { .debug_struct("Statement") .field("expression", &expression_location) .finish(), - Expression::Unary { - operator, - expression, - } => f - .debug_struct("Unary") - .field("operator", operator) - .field("expression", expression) - .finish(), - Expression::Binary { - left, - operator, - right, - } => f - .debug_struct("Binary") - .field("left", left) - .field("operator", operator) - .field("right", right) - .finish(), Expression::Logical { left, operator, diff --git a/ndc_lib/src/ast/parser.rs b/ndc_lib/src/ast/parser.rs index 506d4455..41755607 100644 --- a/ndc_lib/src/ast/parser.rs +++ b/ndc_lib/src/ast/parser.rs @@ -166,7 +166,7 @@ impl Parser { } while let Some(token_location) = self.consume_token_if(&extended_valid_tokens) { - let (invert, token_location) = if token_location.token == Token::LogicNot { + let (invert, operator_token_loc) = if token_location.token == Token::LogicNot { let augmented = self.consume_token_if(valid_tokens).ok_or_else(|| { Error::text( format!( @@ -185,28 +185,38 @@ impl Parser { (None, token_location) }; - let operator: BinaryOperator = token_location.clone().try_into().unwrap_or_else(|_| { - panic!( - "cannot convert '{}' into a binary operator", - token_location.token - ) - }); + let operator: BinaryOperator = + operator_token_loc.clone().try_into().unwrap_or_else(|_| { + panic!( + "cannot convert '{}' into a binary operator", + operator_token_loc.token + ) + }); let right = next(self)?; // IS this new span logic sound? let new_span = left.span.merge(right.span); - left = Expression::Binary { - left: Box::new(left), - operator, - right: Box::new(right), + + // Is this always the same + debug_assert_eq!(operator.to_string(), operator_token_loc.token.to_string()); + + left = Expression::Call { + function: Box::new( + Expression::Identifier(operator_token_loc.token.to_string()) + .to_location(operator_token_loc.span), + ), + arguments: vec![left, right], } .to_location(new_span); - if let Some(invert) = invert { - left = Expression::Unary { - expression: Box::new(left), - operator: UnaryOperator::Not, + if let Some(not_token) = invert { + left = Expression::Call { + function: Box::new( + Expression::Identifier(not_token.token.to_string()) + .to_location(not_token.span), + ), + arguments: vec![left], } - .to_location(new_span.merge(invert.span)); + .to_location(new_span.merge(not_token.span)); } } Ok(left) @@ -221,15 +231,18 @@ impl Parser { let left = next(self)?; if let Some(token_location) = self.consume_token_if(valid_tokens) { + let operator_span = token_location.span; let operator = BinaryOperator::try_from(token_location) .expect("COMPILER ERROR: consume_token_if must guarantee the correct token"); let right = current(self)?; let new_span = left.span.merge(right.span); - return Ok(Expression::Binary { - left: Box::new(left), - operator, - right: Box::new(right), + + return Ok(Expression::Call { + function: Box::new( + Expression::Identifier(operator.to_string()).to_location(operator_span), + ), + arguments: vec![left, right], } .to_location(new_span)); } @@ -450,17 +463,22 @@ impl Parser { // x := not foo in bar // ``` fn logic_not(&mut self) -> Result { - if let Some(token) = self.consume_token_if(&[Token::LogicNot]) { - let operator: UnaryOperator = token + if let Some(operator_token_loc) = self.consume_token_if(&[Token::LogicNot]) { + let operator_span = operator_token_loc.span; + let _: UnaryOperator = operator_token_loc + .clone() .try_into() .expect("consume_operator_if guaranteed us that this token can be UnaryOperator"); let right = self.logic_not()?; let span = right.span; - Ok(Expression::Unary { - operator, - expression: Box::new(right), + Ok(Expression::Call { + function: Box::new( + Expression::Identifier(operator_token_loc.token.to_string()) + .to_location(operator_span), + ), + arguments: vec![right], } .to_location(span)) } else { @@ -571,18 +589,20 @@ impl Parser { } fn tight_unary(&mut self) -> Result { - if let Some(token) = self.consume_token_if(&[Token::Bang, Token::Minus, Token::Tilde]) { - let token_span = token.span; - let operator: UnaryOperator = token - .try_into() - .expect("consume_operator_if guaranteed us that the next token is an Operator"); + if let Some(operator_token_loc) = + self.consume_token_if(&[Token::Bang, Token::Minus, Token::Tilde]) + { + let token_span = operator_token_loc.span; let right = self.tight_unary()?; let span = right.span; - Ok(Expression::Unary { - operator, - expression: Box::new(right), + Ok(Expression::Call { + function: Box::new( + Expression::Identifier(operator_token_loc.token.to_string()) + .to_location(token_span), + ), + arguments: vec![right], } .to_location(span.merge(token_span))) } else { diff --git a/ndc_lib/src/interpreter/environment.rs b/ndc_lib/src/interpreter/environment.rs index 3ab8f93f..53b501ff 100644 --- a/ndc_lib/src/interpreter/environment.rs +++ b/ndc_lib/src/interpreter/environment.rs @@ -117,6 +117,33 @@ impl Environment { self.values.insert(name.into(), RefCell::new(value)); } + /// Declare a function globally using its self-exposed name, if there already exists a function + /// with the same name it's simply overloaded. + pub fn declare_global_fn(&mut self, function: impl Into) { + let fn_to_add = function.into(); + let name = fn_to_add.name(); + + match self.values.get(name) { + Some(v) => { + let existing = &mut *v.borrow_mut(); + match existing { + Value::Function(of) => { + let mut of = of.borrow_mut(); + of.add(fn_to_add); + } + // TODO: what should we do in this case + _ => panic!( + "attempting to declare global function under a name that's already taken by a global non-function" + ), + } + } + None => { + self.values + .insert(name.into(), RefCell::new(Value::function(fn_to_add))); + } + } + } + pub fn assign(&mut self, name: &str, value: Value) -> bool { // Clippy wants us to use self.values.entry(name) but that moves name and breaks the recursive case return if self.values.contains_key(name) { diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index b5000d3d..d9980d3a 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; use std::cmp::Ordering; use std::fmt; -use std::ops::{Add, BitAnd, BitOr, BitXor, Div, IndexMut, Mul, Neg, Not, Rem, Sub}; +use std::ops::{Add, BitAnd, BitOr, BitXor, Div, IndexMut, Mul, Rem, Sub}; use std::rc::Rc; use either::Either; @@ -10,7 +10,6 @@ use itertools::Itertools; use crate::ast::{ BinaryOperator, Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue, - UnaryOperator, }; use crate::hash_map; use crate::hash_map::HashMap; @@ -18,7 +17,7 @@ use crate::interpreter::environment::{Environment, EnvironmentRef}; use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction}; use crate::interpreter::int::Int; use crate::interpreter::iterator::mut_value_to_iterator; -use crate::interpreter::num::{EuclideanDivisionError, Number}; +use crate::interpreter::num::{BinaryOperatorError, Number}; use crate::interpreter::sequence::Sequence; use crate::interpreter::value::{Value, ValueType}; use crate::lexer::Span; @@ -40,50 +39,7 @@ pub(crate) fn evaluate_expression( Expression::BigIntLiteral(n) => Value::Number(Number::Int(Int::BigInt(n.clone()))), Expression::Float64Literal(n) => Value::Number(Number::Float(*n)), Expression::ComplexLiteral(n) => Value::Number(Number::Complex(*n)), - Expression::Unary { - expression: expression_location, - operator, - } => { - let value = evaluate_expression(expression_location, environment)?; - match (value, operator) { - (Value::Number(n), UnaryOperator::BitNot) => Value::Number(n.not()), - (Value::Number(n), UnaryOperator::Neg) => Value::Number(n.neg()), - // Just like in C the bitwise negation of `false` is `-1` - (Value::Bool(b), UnaryOperator::BitNot) => i64::from(b).not().into(), - (Value::Bool(b), UnaryOperator::Not) => Value::Bool(b.not()), - (v, UnaryOperator::Not) => { - return Err(EvaluationError::new( - format!("the '!' operator cannot be applied to {}", v.value_type()), - span, - ) - .into()); - } - (v, UnaryOperator::Neg) => { - return Err(EvaluationError::new( - format!("{} does not support negation", v.value_type()), - span, - ) - .into()); - } - (v, UnaryOperator::BitNot) => { - return Err(EvaluationError::new( - format!("{} does not support bitwise negation", v.value_type()), - span, - ) - .into()); - } - } - } - Expression::Binary { - left, - operator: operator_token, - right, - } => { - let span = left.span.merge(right.span); - let left = evaluate_expression(left, environment)?; - let right = evaluate_expression(right, environment)?; - apply_operator(left, *operator_token, right).into_evaluation_result(span)? - } + // Expression::Unary { Expression::Grouping(expr) => evaluate_expression(expr, environment)?, Expression::VariableDeclaration { l_value, value } => { let value = evaluate_expression(value, environment)?; @@ -861,32 +817,15 @@ fn apply_operation_to_value( } } -#[derive(thiserror::Error, Debug)] -enum BinaryOpError { - #[error("operator {operator} is not defined for {left} and {right}")] - UndefinedOperation { - operator: BinaryOperator, - left: ValueType, - right: ValueType, - }, - #[error(transparent)] - EuclideanDivisionFailed(#[from] EuclideanDivisionError), - #[error("operator {operator} failed because one of its operands is invalid")] - InvalidOperand { operator: BinaryOperator }, -} - #[allow(clippy::too_many_lines)] fn apply_operator( left: Value, operator: BinaryOperator, right: Value, -) -> Result { +) -> Result { let (left_type, right_type) = (left.value_type(), right.value_type()); - let create_type_error = || BinaryOpError::UndefinedOperation { - operator, - left: left_type, - right: right_type, - }; + let create_type_error = + || BinaryOperatorError::undefined_operation(operator, left_type, right_type); if left.supports_vectorization_with(&right) && operator.supports_vectorization() { return apply_operation_vectorized(left, &right, operator); @@ -922,29 +861,29 @@ fn apply_operator( } } BinaryOperator::Plus => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a + b), + (Value::Number(a), Value::Number(b)) => Value::Number(a.add(b)?), _ => return Err(create_type_error()), }, BinaryOperator::Minus => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a - b), + (Value::Number(a), Value::Number(b)) => Value::Number(a.sub(b)?), _ => return Err(create_type_error()), }, BinaryOperator::Multiply => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a * b), + (Value::Number(a), Value::Number(b)) => Value::Number(a.mul(b)?), _ => return Err(create_type_error()), }, BinaryOperator::Divide => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a / b), + (Value::Number(a), Value::Number(b)) => Value::Number(a.div(b)?), _ => return Err(create_type_error()), }, BinaryOperator::FloorDivide => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.floor_div(b)), + (Value::Number(a), Value::Number(b)) => Value::Number(a.floor_div(b)?), _ => return Err(create_type_error()), }, BinaryOperator::CModulo => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.rem(b)), + (Value::Number(a), Value::Number(b)) => Value::Number(a.rem(b)?), _ => return Err(create_type_error()), }, @@ -954,7 +893,7 @@ fn apply_operator( _ => return Err(create_type_error()), }, BinaryOperator::Exponent => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.pow(b)), + (Value::Number(a), Value::Number(b)) => Value::Number(a.pow(b)?), ( left @ Value::Sequence(Sequence::Tuple(_)), @@ -1015,20 +954,22 @@ fn apply_operator( _ => return Err(create_type_error()), }, BinaryOperator::ShiftRight => match (left, right) { - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::Number(Number::Int( - a.checked_shr(b) - .ok_or(BinaryOpError::InvalidOperand { operator })?, - )) - } + (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => Value::Number( + Number::Int(a.checked_shr(b).ok_or(BinaryOperatorError::new(format!( + // TODO: check this before merge + "operator {operator} failed because one of its operands is invalid" + )))?), + ), _ => return Err(create_type_error()), }, BinaryOperator::ShiftLeft => match (left, right) { (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::Number(Number::Int( - a.checked_shl(b) - .ok_or(BinaryOpError::InvalidOperand { operator })?, - )) + Value::Number(Number::Int(a.checked_shl(b).ok_or( + BinaryOperatorError::new(format!( + // TODO: check this before merge + "operator {operator} failed because one of its operands is invalid" + )), + )?)) } _ => return Err(create_type_error()), }, @@ -1086,7 +1027,7 @@ fn apply_operation_vectorized( left: Value, right: &Value, operator: BinaryOperator, -) -> Result { +) -> Result { let (left_type, right_type) = (left.value_type(), right.value_type()); let (mut left, right) = match (left, right) { @@ -1100,11 +1041,9 @@ fn apply_operation_vectorized( (left, std::slice::from_ref(right)) } _ => { - return Err(BinaryOpError::UndefinedOperation { - operator, - left: left_type, - right: right_type, - }); + return Err(BinaryOperatorError::undefined_operation( + operator, left_type, right_type, + )); } }; @@ -1115,22 +1054,20 @@ fn apply_operation_vectorized( if let (Value::Number(l), Value::Number(r)) = (l, r) { // TODO: can we use AddAssign etc? *l = match operator { - BinaryOperator::Plus => Number::add(l.clone(), r), - BinaryOperator::Minus => Number::sub(l.clone(), r), - BinaryOperator::Multiply => Number::mul(l.clone(), r), - BinaryOperator::Divide => Number::div(l.clone(), r), - BinaryOperator::FloorDivide => Number::floor_div(l.clone(), r.clone()), // Get rid of r.clone? - BinaryOperator::CModulo => Number::rem(l.clone(), r), + BinaryOperator::Plus => Number::add(l.clone(), r)?, + BinaryOperator::Minus => Number::sub(l.clone(), r)?, + BinaryOperator::Multiply => Number::mul(l.clone(), r)?, + BinaryOperator::Divide => Number::div(l.clone(), r)?, + BinaryOperator::FloorDivide => Number::floor_div(l.clone(), r.clone())?, // Get rid of r.clone? + BinaryOperator::CModulo => Number::rem(l.clone(), r)?, BinaryOperator::EuclideanModulo => { Number::checked_rem_euclid(l.clone(), r.clone())? // Get rid of r.clone? } - BinaryOperator::Exponent => Number::pow(l.clone(), r.clone()), // Get rid of r.clone? + BinaryOperator::Exponent => Number::pow(l.clone(), r.clone())?, // Get rid of r.clone? _ => { - return Err(BinaryOpError::UndefinedOperation { - operator, - left: left_type, - right: right_type, - }); + return Err(BinaryOperatorError::undefined_operation( + operator, left_type, right_type, + )); } }; } else { diff --git a/ndc_lib/src/interpreter/function.rs b/ndc_lib/src/interpreter/function.rs index c9a033d6..29c7a2d5 100644 --- a/ndc_lib/src/interpreter/function.rs +++ b/ndc_lib/src/interpreter/function.rs @@ -4,7 +4,7 @@ use crate::interpreter::environment::{Environment, EnvironmentRef}; use crate::interpreter::evaluate::{ ErrorConverter, EvaluationError, EvaluationResult, evaluate_expression, }; -use crate::interpreter::num::{Number, NumberType}; +use crate::interpreter::num::{BinaryOperatorError, Number, NumberType}; use crate::interpreter::sequence::Sequence; use crate::interpreter::value::{Value, ValueType}; use crate::lexer::Span; @@ -27,6 +27,7 @@ impl Callable<'_> { self.function.borrow().call(args, self.environment) } } + #[derive(Clone, Builder)] pub struct Function { #[builder(default, setter(strip_option))] @@ -76,9 +77,12 @@ pub enum FunctionBody { body: Rc, environment: EnvironmentRef, }, - SingleNumberFunction { + NumericUnaryOp { body: fn(number: Number) -> Number, }, + NumericBinaryOp { + body: fn(left: Number, right: Number) -> Result, + }, GenericFunction { type_signature: TypeSignature, function: fn(&mut [Value], &EnvironmentRef) -> EvaluationResult, @@ -113,9 +117,13 @@ impl FunctionBody { .collect(), ), Self::Memoized { cache: _, function } => function.type_signature(), - Self::SingleNumberFunction { .. } => { + Self::NumericUnaryOp { .. } => { TypeSignature::Exact(vec![Parameter::new("num", ParamType::Number)]) } + Self::NumericBinaryOp { .. } => TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Number), + Parameter::new("right", ParamType::Number), + ]), Self::GenericFunction { type_signature, .. } => type_signature.clone(), } } @@ -141,16 +149,39 @@ impl FunctionBody { r => r, } } - Self::SingleNumberFunction { body } => match args { + Self::NumericUnaryOp { body } => match args { [Value::Number(num)] => Ok(Value::Number(body(num.clone()))), [v] => Err(FunctionCallError::ArgumentTypeError { - expected: ValueType::Number(NumberType::Float), + expected: ParamType::Number, actual: v.value_type(), } .into()), - _ => Err(FunctionCallError::ArgumentCountError { + args => Err(FunctionCallError::ArgumentCountError { expected: 1, - actual: 0, + actual: args.len(), + } + .into()), + }, + Self::NumericBinaryOp { body } => match args { + [Value::Number(left), Value::Number(right)] => { + // TODO: we could use references here? + Ok(Value::Number(body(left.clone(), right.clone()).map_err( + |err| FunctionCarrier::IntoEvaluationError(Box::new(err)), + )?)) + } + [Value::Number(_), right] => Err(FunctionCallError::ArgumentTypeError { + expected: ParamType::Number, + actual: right.value_type(), + } + .into()), + [left, _] => Err(FunctionCallError::ArgumentTypeError { + expected: ParamType::Number, + actual: left.value_type(), + } + .into()), + args => Err(FunctionCallError::ArgumentCountError { + expected: 2, + actual: args.len(), } .into()), }, @@ -190,6 +221,15 @@ pub struct OverloadedFunction { } impl OverloadedFunction { + pub fn from_multiple(functions: Vec) -> Self { + Self { + implementations: functions + .into_iter() + .map(|f| (f.type_signature(), f)) + .collect(), + } + } + pub fn add(&mut self, function: Function) { self.implementations .insert(function.type_signature(), function); @@ -385,11 +425,37 @@ impl From<&Value> for ParamType { } } +impl fmt::Display for ParamType { + // TODO replace with strum? + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Any => write!(f, "Any"), + Self::Bool => write!(f, "Bool"), + Self::Function => write!(f, "Function"), + Self::Option => write!(f, "Option"), + Self::Number => write!(f, "Number"), + Self::Float => write!(f, "Float"), + Self::Int => write!(f, "Int"), + Self::Rational => write!(f, "Rational"), + Self::Complex => write!(f, "Complex"), + Self::Sequence => write!(f, "Sequence"), + Self::List => write!(f, "List"), + Self::String => write!(f, "String"), + Self::Tuple => write!(f, "Tuple"), + Self::Map => write!(f, "Map"), + Self::Iterator => write!(f, "Iterator"), + Self::MinHeap => write!(f, "MinHeap"), + Self::MaxHeap => write!(f, "MaxHeap"), + Self::Deque => write!(f, "Deque"), + } + } +} + #[derive(thiserror::Error, Debug)] pub enum FunctionCallError { #[error("invalid argument, expected {expected} got {actual}")] ArgumentTypeError { - expected: ValueType, + expected: ParamType, actual: ValueType, }, diff --git a/ndc_lib/src/interpreter/int.rs b/ndc_lib/src/interpreter/int.rs index ba3f5bc2..16848ff1 100644 --- a/ndc_lib/src/interpreter/int.rs +++ b/ndc_lib/src/interpreter/int.rs @@ -169,7 +169,19 @@ impl Int { } } +impl PartialEq for Int { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::BigInt(left), Self::BigInt(right)) => left == right, + (Self::BigInt(left), Self::Int64(right)) => left == &BigInt::from(*right), + (Self::Int64(left), Self::BigInt(right)) => &BigInt::from(*left) == right, + (Self::Int64(left), Self::Int64(right)) => left == right, + } + } +} + impl Eq for Int {} + impl Ord for Int { fn cmp(&self, other: &Self) -> Ordering { match (self, other) { @@ -290,11 +302,11 @@ macro_rules! impl_binary_operator { } }; } - impl_binary_operator!(Add, add, checked_add); impl_binary_operator!(Sub, sub, checked_sub); impl_binary_operator!(Mul, mul, checked_mul); impl_binary_operator!(Div, div, checked_div); + impl_binary_operator!(Rem, rem, checked_rem); impl BitAnd for &Int { diff --git a/ndc_lib/src/interpreter/num.rs b/ndc_lib/src/interpreter/num.rs index 57ce6121..cedb07d6 100644 --- a/ndc_lib/src/interpreter/num.rs +++ b/ndc_lib/src/interpreter/num.rs @@ -4,17 +4,16 @@ use std::hash::{Hash, Hasher}; use std::num::TryFromIntError; use std::ops::{Add, Div, Mul, Neg, Not, Rem, Sub}; +use super::value::ValueType; +use crate::ast::BinaryOperator; +use crate::interpreter::evaluate::EvaluationError; +use crate::interpreter::int::Int; +use crate::lexer::Span; use num::bigint::TryFromBigIntError; use num::complex::{Complex64, ComplexFloat}; use num::{BigInt, BigRational, Complex, FromPrimitive, Signed, ToPrimitive, Zero}; use ordered_float::OrderedFloat; -use crate::interpreter::evaluate::EvaluationError; -use crate::interpreter::int::Int; -use crate::lexer::Span; - -use super::value::ValueType; - #[derive(Debug, Clone)] pub enum Number { Int(Int), @@ -77,17 +76,6 @@ impl Default for Number { } } -impl PartialEq for Int { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - (Self::BigInt(left), Self::BigInt(right)) => left == right, - (Self::BigInt(left), Self::Int64(right)) => left == &BigInt::from(*right), - (Self::Int64(left), Self::BigInt(right)) => &BigInt::from(*left) == right, - (Self::Int64(left), Self::Int64(right)) => left == right, - } - } -} - impl Hash for Number { fn hash(&self, state: &mut H) { match self { @@ -218,12 +206,39 @@ impl<'a> Unbox for &'a Box { } } +#[derive(Debug)] +pub struct BinaryOperatorError(String); + +impl fmt::Display for BinaryOperatorError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::error::Error for BinaryOperatorError {} + +impl BinaryOperatorError { + pub fn new(message: String) -> Self { + Self(message) + } + + pub fn undefined_operation( + operator: BinaryOperator, + left: ValueType, + right: ValueType, + ) -> Self { + Self(format!( + "operator {operator} is not defined for {left} and {right}" + )) + } +} + macro_rules! impl_binary_operator { ($self:ty, $other:ty, $trait:ident, $method:ident,$intmethod:expr,$floatmethod:expr,$rationalmethod:expr,$complexmethod:expr) => { impl $trait<$other> for $self { - type Output = Number; - fn $method(self, other: $other) -> Number { - match (self, other) { + type Output = Result; + fn $method(self, other: $other) -> Self::Output { + Ok(match (self, other) { // Integer (Number::Int(left), Number::Int(right)) => Number::Int($intmethod(left, right)), // Complex @@ -253,7 +268,7 @@ macro_rules! impl_binary_operator { left.unbox(), right.to_rational().expect("cannot convert to rational"), )), - } + }) } } }; @@ -325,35 +340,27 @@ impl Div<&Number> for &Number { } impl Div for Number { - type Output = Self; + type Output = Result; fn div(self, rhs: Self) -> Self::Output { - &self / &rhs + Ok(&self / &rhs) } } impl Div<&Self> for Number { - type Output = Self; + type Output = Result; fn div(self, rhs: &Self) -> Self::Output { - &self / rhs + Ok(&self / rhs) } } impl Div for &Number { - type Output = Number; + type Output = Result; fn div(self, rhs: Number) -> Self::Output { - self / &rhs + Ok(self / &rhs) } } -#[derive(thiserror::Error, Debug)] -pub enum EuclideanDivisionError { - #[error("euclidean division failed (because division by zero?)")] - OperationFailed, - #[error("euclidean division is not defined for {left} and {right}")] - UndefinedOperation { left: NumberType, right: NumberType }, -} - impl Number { #[must_use] pub fn complex(re: f64, im: f64) -> Self { @@ -380,35 +387,35 @@ impl Number { } } - pub fn checked_rem_euclid(self, rhs: Self) -> Result { + pub fn checked_rem_euclid(self, rhs: Self) -> Result { match (self, rhs) { (Self::Int(p1), Self::Int(p2)) => p1 .checked_rem_euclid(&p2) - .ok_or(EuclideanDivisionError::OperationFailed) + .ok_or(BinaryOperatorError::new("operation failed".to_string())) .map(Self::Int), (Self::Float(p1), Self::Float(p2)) => Ok(Self::Float(p1.rem_euclid(p2))), - (left, right) => Err(EuclideanDivisionError::UndefinedOperation { - left: NumberType::from(&left), - right: NumberType::from(&right), - }), + (left, right) => Err(BinaryOperatorError::undefined_operation( + BinaryOperator::EuclideanModulo, + ValueType::from(left), + ValueType::from(right), + )), } } - #[must_use] - pub fn floor_div(self, rhs: Self) -> Self { + pub fn floor_div(self, rhs: Self) -> Result { match (self, rhs) { // Handle this case separately because it's faster?? (Self::Int(Int::Int64(l)), Self::Int(Int::Int64(r))) => { - Self::Int(Int::Int64(l.div_euclid(r))) + Ok(Self::Int(Int::Int64(l.div_euclid(r)))) } - (l, r) => (l / r).floor(), + (l, r) => Ok(l.div(r)?.floor()), } } #[must_use] - pub fn pow(self, rhs: Self) -> Self { - match (self, rhs) { + pub fn pow(self, rhs: Self) -> Result { + Ok(match (self, rhs) { // Int vs others (Self::Int(p1), Self::Int(p2)) => { if p2.is_negative() { @@ -426,7 +433,7 @@ impl Number { } (Self::Int(p1), Self::Rational(p2)) => { if p2.is_integer() { - return Self::Int(p1.pow(&Int::BigInt(p2.to_integer()))); + return Ok(Self::Int(p1.pow(&Int::BigInt(p2.to_integer())))); } Self::Float(f64::from(p1).powf(rational_to_float(&p2))) @@ -439,7 +446,7 @@ impl Number { (Self::Rational(p1), Self::Rational(p2)) => { if p2.is_integer() { if let Some(p2) = p2.to_i32() { - return Self::Rational(Box::new(p1.pow(p2))); + return Ok(Self::Rational(Box::new(p1.pow(p2)))); } } @@ -465,7 +472,7 @@ impl Number { (Self::Complex(p1), Self::Rational(p2)) => { Self::Complex(p1.powc(rational_to_complex(&p2))) } - } + }) } /// # Errors diff --git a/ndc_lib/src/interpreter/value.rs b/ndc_lib/src/interpreter/value.rs index 05f469e4..49197a84 100644 --- a/ndc_lib/src/interpreter/value.rs +++ b/ndc_lib/src/interpreter/value.rs @@ -673,6 +673,12 @@ impl From<&Value> for ValueType { } } +impl From for ValueType { + fn from(value: Number) -> Self { + ValueType::Number((&value).into()) + } +} + impl fmt::Display for ValueType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/ndc_lib/src/stdlib/math.rs b/ndc_lib/src/stdlib/math.rs index 765b838a..4970c4c3 100644 --- a/ndc_lib/src/stdlib/math.rs +++ b/ndc_lib/src/stdlib/math.rs @@ -1,50 +1,52 @@ use crate::interpreter::environment::Environment; -use crate::interpreter::num::Number; +use crate::interpreter::num::{BinaryOperatorError, Number}; use crate::interpreter::sequence::Sequence; use crate::interpreter::value::Value; use factorial::Factorial; use ndc_macros::export_module; use num::ToPrimitive; +use std::ops::{Add, Mul}; -trait FallibleSum { - fn try_sum(&mut self) -> Result; +trait FallibleSum { + fn try_sum(&mut self) -> Result; } -impl FallibleSum for C +impl FallibleSum for C where C: Iterator, T: std::borrow::Borrow, { - fn try_sum(&mut self) -> anyhow::Result { + fn try_sum(&mut self) -> Result { self.try_fold(Number::from(0), |acc, cur| match cur.borrow() { - Value::Number(n) => Ok(acc + n), - value => Err(anyhow::anyhow!( + Value::Number(n) => acc.add(n), + value => Err(BinaryOperatorError::new(format!( "cannot sum {} and number", value.value_type() - )), + ))), }) } } -trait FallibleProduct { - fn try_product(&mut self) -> Result; +trait FallibleProduct { + fn try_product(&mut self) -> Result; } -impl FallibleProduct for C +impl FallibleProduct for C where C: Iterator, T: std::borrow::Borrow, { - fn try_product(&mut self) -> anyhow::Result { - self.try_fold(Number::from(1), |acc, cur| match cur.borrow() { - Value::Number(n) => Ok(acc * n), - value => Err(anyhow::anyhow!( + fn try_product(&mut self) -> Result { + self.try_fold(Number::from(0), |acc, cur| match cur.borrow() { + Value::Number(n) => acc.mul(n), + value => Err(BinaryOperatorError::new(format!( "cannot multiply {} and number", value.value_type() - )), + ))), }) } } + #[export_module] mod inner { use std::ops::Sub; @@ -52,7 +54,7 @@ mod inner { use super::FallibleSum; use crate::interpreter::int::Int; use crate::interpreter::num::Number; - use anyhow::{Context, anyhow}; + use anyhow::Context; use num::{BigInt, BigRational, BigUint, Integer, complex::Complex64}; /// Returns the sign of a number. @@ -85,7 +87,9 @@ mod inner { pub fn sum(seq: &Sequence) -> anyhow::Result { match seq { - Sequence::String(_s) => Err(anyhow!("string cannot be summed")), + Sequence::String(_s) => Err(BinaryOperatorError::new( + "string cannot be summed".to_string(), + )), Sequence::List(list) => list.borrow().iter().try_sum(), Sequence::Tuple(tup) => tup.iter().try_sum(), Sequence::Map(map, _) => map.borrow().keys().try_sum(), @@ -94,11 +98,14 @@ mod inner { Sequence::MinHeap(h) => h.borrow().iter().map(|v| &v.0.0).try_sum(), Sequence::Deque(d) => d.borrow().iter().try_sum(), } + .context("type error while multiplying sequence") } pub fn product(seq: &Sequence) -> anyhow::Result { match seq { - Sequence::String(_s) => Err(anyhow!("string cannot be summed")), + Sequence::String(_s) => Err(BinaryOperatorError::new( + "string cannot be multiplied".to_string(), + )), Sequence::List(list) => list.borrow().iter().try_product(), Sequence::Tuple(tup) => tup.iter().try_product(), Sequence::Map(map, _) => map.borrow().keys().try_product(), @@ -107,6 +114,7 @@ mod inner { Sequence::MinHeap(h) => h.borrow().iter().map(|v| &v.0.0).try_product(), Sequence::Deque(d) => d.borrow().iter().try_product(), } + .context("type error while multiplying sequence") } pub fn factorial(a: &BigInt) -> anyhow::Result { @@ -140,8 +148,8 @@ mod inner { number.abs() } - pub fn abs_diff(left: &Number, right: &Number) -> Number { - left.sub(right).abs() + pub fn abs_diff(left: &Number, right: &Number) -> Result { + Ok(left.sub(right)?.abs()) } pub fn float(value: &Value) -> anyhow::Result { @@ -197,15 +205,232 @@ mod inner { pub mod f64 { use super::{Environment, Number, ToPrimitive, f64}; - use crate::interpreter::function::FunctionBuilder; + use crate::interpreter::evaluate::EvaluationResult; + use crate::interpreter::function::FunctionCarrier::EvaluationError; + use crate::interpreter::function::{ + FunctionBody, FunctionBuilder, FunctionCarrier, ParamType, Parameter, TypeSignature, + }; + use crate::interpreter::num::BinaryOperatorError; + use crate::interpreter::value::Value; + use std::cmp::Ordering; + use std::ops::Not; pub fn register(env: &mut Environment) { + macro_rules! implement_binary_operator_on_num { + ($operator:literal,$method:expr) => { + env.declare_global_fn( + FunctionBuilder::default() + .name($operator.to_string()) + .body(FunctionBody::NumericBinaryOp { body: $method }) + .build() + .expect("must be valid"), + ); + }; + } + + implement_binary_operator_on_num!("-", std::ops::Sub::sub); + implement_binary_operator_on_num!("+", std::ops::Add::add); + implement_binary_operator_on_num!("*", std::ops::Mul::mul); + implement_binary_operator_on_num!("/", std::ops::Div::div); + implement_binary_operator_on_num!("\\", Number::floor_div); + implement_binary_operator_on_num!("^", Number::pow); + implement_binary_operator_on_num!("%", std::ops::Rem::rem); + implement_binary_operator_on_num!("%%", Number::checked_rem_euclid); + + env.declare_global_fn( + FunctionBuilder::default() + .body(FunctionBody::NumericUnaryOp { + body: std::ops::Neg::neg, + }) + .name("-".to_string()) + .build() + .expect("must succeed"), + ); + + macro_rules! impl_cmp { + ($operator:literal,$expected:pat) => { + env.declare_global_fn( + FunctionBuilder::default() + .name($operator.to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Any), + Parameter::new("right", ParamType::Any), + ]), + function: |values, _env| match values { + [left, right] => match left.partial_cmp(&right) { + Some($expected) => Ok(Value::Bool(true)), + Some(_) => Ok(Value::Bool(false)), + None => Err(anyhow::anyhow!("cannot compare {} and {}",left.value_type(),right.value_type()).into()), + }, + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + }; + } + + impl_cmp!(">", Ordering::Greater); + impl_cmp!(">=", Ordering::Greater | Ordering::Equal); + impl_cmp!("==", Ordering::Equal); + impl_cmp!("<", Ordering::Less); + impl_cmp!("<=", Ordering::Less | Ordering::Equal); + impl_cmp!("!=", Ordering::Less | Ordering::Greater); + + env.declare_global_fn( + FunctionBuilder::default() + .name("<=>".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Any), + Parameter::new("right", ParamType::Any), + ]), + function: |values, _env| match values { + [left, right] => match left.partial_cmp(&right) { + Some(Ordering::Equal) => Ok(Value::from(0)), + Some(Ordering::Less) => Ok(Value::from(-1)), + Some(Ordering::Greater) => Ok(Value::from(1)), + None => Err(anyhow::anyhow!("cannot compare {} and {}",left.value_type(),right.value_type()).into()), + }, + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + + env.declare_global_fn( + FunctionBuilder::default() + .name(">=<".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Any), + Parameter::new("right", ParamType::Any), + ]), + function: |values, _env| match values { + [left, right] => match left.partial_cmp(&right) { + Some(Ordering::Equal) => Ok(Value::from(0)), + Some(Ordering::Less) => Ok(Value::from(1)), + Some(Ordering::Greater) => Ok(Value::from(-1)), + None => Err(anyhow::anyhow!("cannot compare {} and {}",left.value_type(),right.value_type()).into()), + }, + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + + macro_rules! impl_bitop { + ($operator:literal,$operation:expr) => { + env.declare_global_fn( + FunctionBuilder::default() + .name($operator.to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Bool), + Parameter::new("right", ParamType::Bool), + ]), + function: |values, _env| match values { + [Value::Bool(left), Value::Bool(right)] => Ok(Value::Bool($operation(*left, *right))), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + env.declare_global_fn( + FunctionBuilder::default() + .name($operator.to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Int), + Parameter::new("right", ParamType::Int), + ]), + function: |values, _env| match values { + // TODO: remove this clone + [Value::Number(Number::Int(left)), Value::Number(Number::Int(right))] => Ok(Value::Number(Number::Int($operation(left.clone(), right.clone())))), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed"), + ); + }; + } + + impl_bitop!("&", std::ops::BitAnd::bitand); + impl_bitop!("|", std::ops::BitOr::bitor); + impl_bitop!("~", std::ops::BitXor::bitxor); + + env.declare_global_fn( + FunctionBuilder::default() + .body(FunctionBody::NumericUnaryOp { body: |x| x.not() }) + .name("~".to_string()) + .build() + .expect("must succeed"), + ); + + env.declare_global_fn( + FunctionBuilder::default() + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![Parameter::new("value", ParamType::Bool)]), + function: |values, _env| match values { + [Value::Bool(b)] => Ok(Value::Bool(b.not())), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match"), + }, + }) + .name("!".to_string()) + .build() + .expect("must succeed") + ); + + env.declare_global_fn( + FunctionBuilder::default() + .name(">>".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Int), + Parameter::new("right", ParamType::Int), + ]), + function: |values, _env| match values { + [Value::Number(Number::Int(left)), Value::Number(Number::Int(right))] => left.clone().checked_shr(right.clone()) + .ok_or_else(|| FunctionCarrier::IntoEvaluationError(Box::new(BinaryOperatorError::new("cannot apply >> operator to operands".to_string())))) // TODO: improve error message + .map(|x| Value::Number(Number::Int(x))), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + + env.declare_global_fn( + FunctionBuilder::default() + .name("<<".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Int), + Parameter::new("right", ParamType::Int), + ]), + function: |values, _env| match values { + [Value::Number(Number::Int(left)), Value::Number(Number::Int(right))] => left.clone().checked_shl(right.clone()) + .ok_or_else(|| FunctionCarrier::IntoEvaluationError(Box::new(BinaryOperatorError::new("cannot apply << operator to operands".to_string())))) // TODO: improve error message + .map(|x| Value::Number(Number::Int(x))), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + macro_rules! delegate_to_f64 { ($method:ident,$docs:literal) => { let function = $crate::interpreter::value::Value::function( FunctionBuilder::default() .body( - $crate::interpreter::function::FunctionBody::SingleNumberFunction { + $crate::interpreter::function::FunctionBody::NumericUnaryOp { body: |num: Number| match num { Number::Int(i) => Number::Float(f64::from(i).$method()), Number::Float(f) => Number::Float(f.$method()), diff --git a/ndc_macros/src/function.rs b/ndc_macros/src/function.rs index 0b54dc80..0a489206 100644 --- a/ndc_macros/src/function.rs +++ b/ndc_macros/src/function.rs @@ -198,7 +198,7 @@ fn wrap_single( .build() .expect("expected function creation in proc macro to always succeed"); - env.declare_function(#register_as_function_name, func); + env.declare_global_fn(func); }; WrappedFunction { diff --git a/tests/programs/001_math/019_bit_operations.ndct b/tests/programs/001_math/019_bit_operations.ndct index b5e5cde5..41e9a9f0 100644 --- a/tests/programs/001_math/019_bit_operations.ndct +++ b/tests/programs/001_math/019_bit_operations.ndct @@ -13,10 +13,6 @@ assert(true ~ true == false); assert(true ~ false == true); assert(false ~ false == false); -// Bitwise complement of boolean is evaluated as integers -assert(~true == -2); -assert(~false == -1); - // integers assert(1 & 15 == 1); assert(15 & 0 == 0); From 00dd7fe1dc7e3d8661fb2e15845a7d783c2bdd75 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 3 Jun 2025 16:42:22 +0200 Subject: [PATCH 02/18] Very very shitty concat op --- ndc_lib/src/stdlib/list.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index e57d27f4..332f988b 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -66,6 +66,12 @@ mod inner { list.append(other); } + // TODO: this implementation has pretty terrible performance compared to what we had before + #[function(name = "++")] + pub fn op_concat(left: &[Value], right: &[Value]) -> Vec { + left.iter().chain(right.iter()).cloned().collect() + } + /// Copies elements from `other` to `list` not touching `other`. pub fn extend(list: &mut Vec, other: &[Value]) { list.extend_from_slice(other); From 3c8673e55eaecfe25ae12888e5dda420f54c647a Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Thu, 5 Jun 2025 09:55:11 +0200 Subject: [PATCH 03/18] Implement some more operators fixing some more tests Current TODO: ++ always returns a list which breaks the tuple_concat test --- ndc_lib/src/stdlib/list.rs | 3 +- ndc_lib/src/stdlib/math.rs | 66 +++++++++++++++++++++++++--------- ndc_lib/src/stdlib/sequence.rs | 5 +++ ndc_lib/src/stdlib/string.rs | 14 ++++++++ 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index 332f988b..ab433b0c 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -66,9 +66,10 @@ mod inner { list.append(other); } - // TODO: this implementation has pretty terrible performance compared to what we had before + // TODO: ISSUE: this implementation has pretty terrible performance compared to what we had before #[function(name = "++")] pub fn op_concat(left: &[Value], right: &[Value]) -> Vec { + // TODO: ISSUE: This function always returns a list, even when it's concatenating two tuples left.iter().chain(right.iter()).cloned().collect() } diff --git a/ndc_lib/src/stdlib/math.rs b/ndc_lib/src/stdlib/math.rs index 4970c4c3..fcf55e6c 100644 --- a/ndc_lib/src/stdlib/math.rs +++ b/ndc_lib/src/stdlib/math.rs @@ -205,8 +205,6 @@ mod inner { pub mod f64 { use super::{Environment, Number, ToPrimitive, f64}; - use crate::interpreter::evaluate::EvaluationResult; - use crate::interpreter::function::FunctionCarrier::EvaluationError; use crate::interpreter::function::{ FunctionBody, FunctionBuilder, FunctionCarrier, ParamType, Parameter, TypeSignature, }; @@ -274,10 +272,42 @@ pub mod f64 { impl_cmp!(">", Ordering::Greater); impl_cmp!(">=", Ordering::Greater | Ordering::Equal); - impl_cmp!("==", Ordering::Equal); impl_cmp!("<", Ordering::Less); impl_cmp!("<=", Ordering::Less | Ordering::Equal); - impl_cmp!("!=", Ordering::Less | Ordering::Greater); + + env.declare_global_fn( + FunctionBuilder::default() + .name("==".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Any), + Parameter::new("right", ParamType::Any), + ]), + function: |values, _env| match values { + [left, right] => Ok(Value::Bool(left == right)), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); + + env.declare_global_fn( + FunctionBuilder::default() + .name("!=".to_string()) + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![ + Parameter::new("left", ParamType::Any), + Parameter::new("right", ParamType::Any), + ]), + function: |values, _env| match values { + [left, right] => Ok(Value::Bool(left != right)), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match") + }, + }) + .build() + .expect("must succeed") + ); env.declare_global_fn( FunctionBuilder::default() @@ -373,19 +403,21 @@ pub mod f64 { .expect("must succeed"), ); - env.declare_global_fn( - FunctionBuilder::default() - .body(FunctionBody::GenericFunction { - type_signature: TypeSignature::Exact(vec![Parameter::new("value", ParamType::Bool)]), - function: |values, _env| match values { - [Value::Bool(b)] => Ok(Value::Bool(b.not())), - _ => unreachable!("the type checker should never invoke this function if the argument count does not match"), - }, - }) - .name("!".to_string()) - .build() - .expect("must succeed") - ); + for ident in ["!", "not"] { + env.declare_global_fn( + FunctionBuilder::default() + .body(FunctionBody::GenericFunction { + type_signature: TypeSignature::Exact(vec![Parameter::new("value", ParamType::Bool)]), + function: |values, _env| match values { + [Value::Bool(b)] => Ok(Value::Bool(b.not())), + _ => unreachable!("the type checker should never invoke this function if the argument count does not match"), + }, + }) + .name(ident.to_string()) + .build() + .expect("must succeed") + ); + } env.declare_global_fn( FunctionBuilder::default() diff --git a/ndc_lib/src/stdlib/sequence.rs b/ndc_lib/src/stdlib/sequence.rs index 39e37cd1..2903432c 100644 --- a/ndc_lib/src/stdlib/sequence.rs +++ b/ndc_lib/src/stdlib/sequence.rs @@ -81,6 +81,11 @@ fn try_sort_by( mod inner { use crate::interpreter::{function::FunctionCarrier, iterator::mut_value_to_iterator}; + #[function(name = "in")] + pub fn op_contains(elem: &Value, seq: &Sequence) -> bool { + seq.contains(elem) + } + /// Returns the highest element in the sequence. pub fn max(seq: &Sequence) -> anyhow::Result { match seq { diff --git a/ndc_lib/src/stdlib/string.rs b/ndc_lib/src/stdlib/string.rs index 079b00b6..178f8097 100644 --- a/ndc_lib/src/stdlib/string.rs +++ b/ndc_lib/src/stdlib/string.rs @@ -25,6 +25,14 @@ pub fn join_to_string(list: &mut Sequence, sep: &str) -> anyhow::Result #[export_module] mod inner { + use ndc_macros::function; + + /// The string concat operator + #[function(name = "<>")] + pub fn op_string_concat(left: &Value, right: &Value) -> String { + format!("{left}{right}") + } + /// Returns the Unicode code point of a 1-length string. pub fn ord(string: &str) -> anyhow::Result { if string.chars().count() == 1 { @@ -78,6 +86,12 @@ mod inner { string.push_str(value); } + // TODO: should we optimize something here? + #[function(name = "++")] + pub fn concat(left: &str, right: &str) -> String { + format!("{left}{right}") + } + /// Joins elements of the sequence into a single string using `sep` as the separator. pub fn join(list: &mut Sequence, sep: &str) -> anyhow::Result { join_to_string(list, sep) From 24f843cad8c6d12ba0684eaa24ac89afe5d94b39 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 1 Jul 2025 14:30:58 +0200 Subject: [PATCH 04/18] Repaired some operators --- ndc_lib/src/interpreter/evaluate.rs | 8 +- ndc_lib/src/interpreter/sequence.rs | 8 +- ndc_lib/src/interpreter/value.rs | 21 +++-- ndc_lib/src/stdlib/hash_map.rs | 7 +- ndc_lib/src/stdlib/list.rs | 59 ++++++++---- ndc_lib/src/stdlib/rand.rs | 10 ++- ndc_lib/src/stdlib/regex.rs | 43 +++++---- ndc_lib/src/stdlib/sequence.rs | 127 +++++++++++++------------- ndc_lib/src/stdlib/string.rs | 32 +++---- ndc_lib/src/stdlib/value.rs | 4 +- ndc_macros/src/convert.rs | 111 +++++++++++++++++++++++ ndc_macros/src/function.rs | 135 +++++++++++++++------------- ndc_macros/src/lib.rs | 1 + 13 files changed, 362 insertions(+), 204 deletions(-) create mode 100644 ndc_macros/src/convert.rs diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index d9980d3a..ccd62d96 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -405,7 +405,7 @@ pub(crate) fn evaluate_expression( match &**body { ForBody::Block(_) => Value::unit(), - ForBody::List(_) => Value::from(out_values), + ForBody::List(_) => Value::list(out_values), ForBody::Map { key: _, value: _, @@ -479,7 +479,7 @@ pub(crate) fn evaluate_expression( ); }; - values.to_vec().into() + Value::list(values.to_vec()) // TODO: can we remove to_vec? } } } @@ -994,9 +994,9 @@ fn apply_operator( // TODO: if there is no benefit to this branch we might as wel remove it let mut new_list = left.borrow().clone(); new_list.append(&mut right.borrow_mut()); - Value::from(new_list) + Value::list(new_list) } - Err(right) => Value::from( + Err(right) => Value::list( left.borrow() .iter() .chain(right.borrow().iter()) diff --git a/ndc_lib/src/interpreter/sequence.rs b/ndc_lib/src/interpreter/sequence.rs index 59039889..e829527f 100644 --- a/ndc_lib/src/interpreter/sequence.rs +++ b/ndc_lib/src/interpreter/sequence.rs @@ -8,14 +8,16 @@ use std::collections::VecDeque; use std::fmt; use std::rc::Rc; -pub type DefaultMapMut<'a> = (&'a mut HashMap, Option>); pub type DefaultMap<'a> = (&'a HashMap, Option>); +pub type DefaultMapMut<'a> = (&'a mut HashMap, Option>); +pub type ListRepr = Rc>>; +pub type TupleRepr = Rc>; #[derive(Clone)] pub enum Sequence { String(Rc>), - List(Rc>>), - Tuple(Rc>), + List(ListRepr), + Tuple(TupleRepr), Map(Rc>>, Option>), Iterator(Rc>), MaxHeap(Rc>), diff --git a/ndc_lib/src/interpreter/value.rs b/ndc_lib/src/interpreter/value.rs index 49197a84..f2bdf060 100644 --- a/ndc_lib/src/interpreter/value.rs +++ b/ndc_lib/src/interpreter/value.rs @@ -37,6 +37,14 @@ impl Value { Self::Sequence(Sequence::List(Rc::new(RefCell::new(data.into())))) } + pub(crate) fn collect_list(i: I) -> Self + where + I: Iterator, + V: Into, + { + Self::list(i.map(Into::into).collect::>()) + } + pub(crate) fn tuple>>(data: V) -> Self { Self::Sequence(Sequence::Tuple(Rc::new(data.into()))) } @@ -57,6 +65,10 @@ impl Value { Self::Function(Rc::new(RefCell::new(source.into()))) } + pub(crate) fn number>(source: T) -> Self { + Self::Number(source.into()) + } + /// If this value is a type of `Sequence` it returns the length of the sequence, otherwise it returns `None` #[must_use] pub fn sequence_length(&self) -> Option { @@ -375,15 +387,6 @@ impl<'a> From<&'a mut Value> for &'a Value { } } -// TODO: is this implementation useful, should it be over an Iterator of some kind instead -impl> From> for Value { - fn from(value: Vec) -> Self { - Self::Sequence(Sequence::List(Rc::new(RefCell::new( - value.into_iter().map(Into::into).collect(), - )))) - } -} - impl From for Value { fn from(value: Number) -> Self { Self::Number(value) diff --git a/ndc_lib/src/stdlib/hash_map.rs b/ndc_lib/src/stdlib/hash_map.rs index e1414c2e..52c91a03 100644 --- a/ndc_lib/src/stdlib/hash_map.rs +++ b/ndc_lib/src/stdlib/hash_map.rs @@ -11,14 +11,14 @@ mod inner { /// /// Note that for a set this will return the values in the set. pub fn keys(map: &mut HashMap) -> Value { - map.keys().cloned().collect::>().into() + Value::list(map.keys().cloned().collect::>()) } /// Returns a list of all the values in the map. /// /// Note that for sets this will return a list of unit types, you should use keys if you want the values in the set. pub fn values(map: &mut HashMap) -> Value { - map.values().cloned().collect::>().into() + Value::list(map.values().cloned().collect::>()) } /// Removes a key from the map or a value from a set. @@ -49,6 +49,7 @@ mod inner { /// Returns the union (elements that are in either `left` or `right`) of two maps or sets. /// /// This is the same as evaluating the expression `left | right` + #[function(alias = "|")] pub fn union(left: DefaultMap<'_>, right: &HashMap) -> Value { Value::Sequence(Sequence::Map( Rc::new(RefCell::new(hash_map::union(left.0, right))), @@ -59,6 +60,7 @@ mod inner { /// Returns the intersection (elements that are in both `left and `right`) of two maps or sets. /// /// This is the same as evaluating the expression `left & right`. + #[function(alias = "&")] pub fn intersection(left: DefaultMap<'_>, right: &HashMap) -> Value { Value::Sequence(Sequence::Map( Rc::new(RefCell::new(hash_map::intersection(left.0, right))), @@ -69,6 +71,7 @@ mod inner { /// Returns the symmetric difference (elements that are either in `left` or `right` but not both) of two maps or sets. /// /// This is the same as evaluating the expression `left ~ right`. + #[function(alias = "~")] pub fn symmetric_difference(left: DefaultMap<'_>, right: &HashMap) -> Value { Value::Sequence(Sequence::Map( Rc::new(RefCell::new(hash_map::symmetric_difference(left.0, right))), diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index ab433b0c..4f1a8e55 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -1,7 +1,7 @@ #[ndc_macros::export_module] mod inner { use crate::interpreter::iterator::mut_seq_to_iterator; - use crate::interpreter::sequence::Sequence; + use crate::interpreter::sequence::{ListRepr, Sequence, TupleRepr}; use crate::interpreter::value::Value; use itertools::Itertools; use std::rc::Rc; @@ -9,8 +9,8 @@ mod inner { use anyhow::anyhow; /// Converts any sequence into a list - pub fn list(seq: &mut Sequence) -> Vec { - mut_seq_to_iterator(seq).collect::>() + pub fn list(seq: &mut Sequence) -> Value { + Value::list(mut_seq_to_iterator(seq).collect::>()) } pub fn contains(list: &[Value], elem: &Value) -> bool { @@ -66,11 +66,31 @@ mod inner { list.append(other); } - // TODO: ISSUE: this implementation has pretty terrible performance compared to what we had before #[function(name = "++")] - pub fn op_concat(left: &[Value], right: &[Value]) -> Vec { - // TODO: ISSUE: This function always returns a list, even when it's concatenating two tuples - left.iter().chain(right.iter()).cloned().collect() + pub fn tup_concat(left: TupleRepr, mut right: TupleRepr) -> Value { + match Rc::try_unwrap(left) { + Ok(mut left) => { + left.append(Rc::make_mut(&mut right)); + Value::tuple(left) + } + Err(left) => Value::tuple( + left.iter() + .chain(right.iter()) + .cloned() + .collect::>(), + ), + } + } + + #[function(name = "++")] + pub fn list_concat(left: ListRepr, right: ListRepr) -> Value { + Value::list( + left.borrow() + .iter() + .chain(right.borrow().iter()) + .cloned() + .collect::>(), + ) } /// Copies elements from `other` to `list` not touching `other`. @@ -111,9 +131,10 @@ mod inner { list.remove(0) } - /// Creates a copy of the list with it's elements in reverse order - pub fn reversed(list: &[Value]) -> Vec { - list.iter().rev().cloned().collect::>() + /// Creates a copy of the list with its elements in reverse order + /// TODO: how to deal with tuples? + pub fn reversed(list: &[Value]) -> Value { + Value::list(list.iter().rev().cloned().collect::>()) } /// Removes all values from the list @@ -127,12 +148,12 @@ mod inner { } /// Splits the collection into two at the given index. - pub fn split_off(list: &mut Vec, index: usize) -> anyhow::Result> { + pub fn split_off(list: &mut Vec, index: usize) -> anyhow::Result { if index > list.len() { return Err(anyhow!("index {index} is out of bounds")); } - Ok(list.split_off(index)) + Ok(Value::list(list.split_off(index))) } /// Shortens the list, keeping the first `len` elements and dropping the rest. @@ -167,11 +188,13 @@ mod inner { list.last().cloned().map_or_else(Value::none, Value::some) } - pub fn cartesian_product(list_a: &[Value], list_b: &[Value]) -> Vec { - list_a - .iter() - .cartesian_product(list_b) - .map(|(a, b)| Value::Sequence(Sequence::Tuple(Rc::new(vec![a.clone(), b.clone()])))) - .collect_vec() + pub fn cartesian_product(list_a: &[Value], list_b: &[Value]) -> Value { + Value::list( + list_a + .iter() + .cartesian_product(list_b) + .map(|(a, b)| Value::tuple(vec![a.clone(), b.clone()])) + .collect_vec(), + ) } } diff --git a/ndc_lib/src/stdlib/rand.rs b/ndc_lib/src/stdlib/rand.rs index 2125192b..6b7506c5 100644 --- a/ndc_lib/src/stdlib/rand.rs +++ b/ndc_lib/src/stdlib/rand.rs @@ -33,10 +33,12 @@ mod inner { /// Returns a copy of the input sequence converted to a list with the elements shuffled in random order. /// /// Note: this currently does consume iterators - pub fn shuffled(list: &mut Sequence) -> Vec { - mut_seq_to_iterator(list) - .collect_vec() - .tap_mut(|v| v.shuffle(&mut rand::rng())) + pub fn shuffled(list: &mut Sequence) -> Value { + Value::list( + mut_seq_to_iterator(list) + .collect_vec() + .tap_mut(|v| v.shuffle(&mut rand::rng())), + ) } #[function(name = "randf")] diff --git a/ndc_lib/src/stdlib/regex.rs b/ndc_lib/src/stdlib/regex.rs index dd4bd85f..ad161574 100644 --- a/ndc_lib/src/stdlib/regex.rs +++ b/ndc_lib/src/stdlib/regex.rs @@ -6,26 +6,23 @@ use regex::Regex; mod inner { /// Extracts all signed integers from the given string. - pub fn nums(haystack: &str) -> Vec { + pub fn nums(haystack: &str) -> Value { static RE: Lazy = Lazy::new(|| Regex::new(r"-?\d+").unwrap()); - RE.captures_iter(haystack) - .filter_map(|cap| { - let (full, []) = cap.extract(); - full.parse::().ok() - }) - .collect() + Value::collect_list(RE.captures_iter(haystack).filter_map(|cap| { + let (full, []) = cap.extract(); + full.parse::().ok() + })) } /// Extracts all unsigned integers from the given string. - pub fn unsigned_nums(haystack: &str) -> Vec { + pub fn unsigned_nums(haystack: &str) -> Value { static RE: Lazy = Lazy::new(|| Regex::new(r"\d+").unwrap()); - RE.captures_iter(haystack) - .filter_map(|cap| { - let (full, []) = cap.extract(); - full.parse::().ok() - }) - .collect() + + Value::collect_list(RE.captures_iter(haystack).filter_map(|cap| { + let (full, []) = cap.extract(); + full.parse::().ok() + })) } /// Returns `true` if the string matches the given regular expression. @@ -41,15 +38,16 @@ mod inner { let list = r .captures_iter(haystack) .map(|captures| { - captures - .iter() - .filter_map(|x| x.map(|x| Value::from(x.as_str()))) - .collect::>() - .into() + Value::list( + captures + .iter() + .filter_map(|x| x.map(|x| Value::from(x.as_str()))) + .collect::>(), + ) }) .collect::>(); - Ok(Value::from(list)) + Ok(Value::list(list)) } /// Returns the first capture group from the first match of the regular expression. @@ -63,9 +61,8 @@ mod inner { let list = captures .iter() .filter_map(|x| x.map(|x| Value::from(x.as_str()))) - .collect::>() - .into(); + .collect::>(); - Ok(list) + Ok(Value::list(list)) } } diff --git a/ndc_lib/src/stdlib/sequence.rs b/ndc_lib/src/stdlib/sequence.rs index 2903432c..73e56dcd 100644 --- a/ndc_lib/src/stdlib/sequence.rs +++ b/ndc_lib/src/stdlib/sequence.rs @@ -216,33 +216,28 @@ mod inner { /// Enumerates the given sequence returning a list of tuples where the first element of the tuple is the index of the element in the input sequence. pub fn enumerate(seq: &mut Sequence) -> Value { match seq { - Sequence::String(s) => s - .borrow() - .chars() - .enumerate() - .map(|(index, char)| { - Value::Sequence(Sequence::Tuple(Rc::new(vec![ - Value::from(index), - Value::from(char), - ]))) - }) - .collect::>() - .into(), - Sequence::Map(map, _) => map - .borrow() - .iter() - .enumerate() - .map(|(index, (key, value))| { - Value::Sequence(Sequence::Tuple(Rc::new(vec![ - Value::from(index), + Sequence::String(s) => Value::list( + s.borrow() + .chars() + .enumerate() + .map(|(index, char)| Value::tuple(vec![Value::from(index), Value::from(char)])) + .collect::>(), + ), + Sequence::Map(map, _) => Value::list( + map.borrow() + .iter() + .enumerate() + .map(|(index, (key, value))| { Value::Sequence(Sequence::Tuple(Rc::new(vec![ - Value::clone(key), - Value::clone(value), - ]))), - ]))) - }) - .collect::>() - .into(), + Value::from(index), + Value::Sequence(Sequence::Tuple(Rc::new(vec![ + Value::clone(key), + Value::clone(value), + ]))), + ]))) + }) + .collect::>(), + ), // TODO: This entire branch is so cringe, why are we even trying to use iterators if we do shit like this Sequence::Iterator(rc) => { let mut iter = rc.borrow_mut(); @@ -254,7 +249,7 @@ mod inner { ])))); } - out.into() + Value::list(out) } seq => Value::list( mut_seq_to_iterator(seq) @@ -297,7 +292,7 @@ mod inner { } } - Ok(out.into()) + Ok(Value::list(out)) } /// Returns the number of elements in the input sequence for which the given `predicate` returns `true`. @@ -315,7 +310,7 @@ mod inner { } } - Ok(out.into()) + Ok(Value::number(out)) } /// Returns the value of the first element for which the `predicate` is true for the given input sequence. @@ -426,7 +421,7 @@ mod inner { out.push(function.call(&mut [item])?); } - Ok(Value::from(out)) + Ok(Value::list(out)) } /// Applies a function to each item in a sequence, flattens the resulting sequences, and returns a single combined sequence. @@ -448,7 +443,7 @@ mod inner { } } - Ok(Value::from(out)) + Ok(Value::list(out)) } /// Returns the first element of the sequence or the `default` value otherwise. @@ -566,24 +561,28 @@ mod inner { } /// Return a list of all windows, wrapping back to the first elements when the window would otherwise exceed the length of source list, producing tuples of size 2. - pub fn circular_tuple_windows(seq: &mut Sequence) -> Vec { + pub fn circular_tuple_windows(seq: &mut Sequence) -> Value { // TODO: this implementation probably clones a bit more than it needs to, but it's better tol // have something than nothing - mut_seq_to_iterator(seq) - .collect::>() - .iter() - .circular_tuple_windows::<(_, _)>() - .map(|(a, b)| Value::tuple(vec![a.clone(), b.clone()])) - .collect::>() + Value::list( + mut_seq_to_iterator(seq) + .collect::>() + .iter() + .circular_tuple_windows::<(_, _)>() + .map(|(a, b)| Value::tuple(vec![a.clone(), b.clone()])) + .collect::>(), + ) } /// Returns a list of all size-2 windows in `seq`. - pub fn pairwise(seq: &mut Sequence) -> Vec { - mut_seq_to_iterator(seq) - .collect::>() - .windows(2) - .map(Value::tuple) - .collect::>() + pub fn pairwise(seq: &mut Sequence) -> Value { + Value::list( + mut_seq_to_iterator(seq) + .collect::>() + .windows(2) + .map(Value::tuple) + .collect::>(), + ) } /// Applies a function to each pair of consecutive elements in a sequence and returns the results as a list. @@ -600,33 +599,39 @@ mod inner { } /// Returns a list of all contiguous windows of `length` size. The windows overlap. If the `seq` is shorter than size, the iterator returns no values. - pub fn windows(seq: &mut Sequence, length: usize) -> Vec { - mut_seq_to_iterator(seq) - .collect::>() - .windows(length) - .map(Value::list) - .collect() + pub fn windows(seq: &mut Sequence, length: usize) -> Value { + Value::list( + mut_seq_to_iterator(seq) + .collect::>() + .windows(length) + .map(Value::list) + .collect::>(), + ) } /// Return a list that represents the powerset of the elements of `seq`. /// /// The powerset of a set contains all subsets including the empty set and the full input set. A powerset has length `2^n` where `n` is the length of the input set. /// Each list produced by this function represents a subset of the elements in the source sequence. - pub fn subsequences(seq: &mut Sequence) -> Vec { - mut_seq_to_iterator(seq) - .powerset() - .map(Value::list) - .collect::>() + pub fn subsequences(seq: &mut Sequence) -> Value { + Value::list( + mut_seq_to_iterator(seq) + .powerset() + .map(Value::list) + .collect::>(), + ) } /// Return a list that represents the powerset of the elements of `seq` that are exactly `length` long. #[function(name = "subsequences")] - pub fn subsequences_len(seq: &mut Sequence, length: usize) -> Vec { - mut_seq_to_iterator(seq) - .powerset() - .filter(|x| x.len() == length) - .map(Value::list) - .collect::>() + pub fn subsequences_len(seq: &mut Sequence, length: usize) -> Value { + Value::list( + mut_seq_to_iterator(seq) + .powerset() + .filter(|x| x.len() == length) + .map(Value::list) + .collect::>(), + ) } /// Computes the Cartesian product of multiple iterables, returning a list of all possible combinations where each combination contains one element from each iterable. diff --git a/ndc_lib/src/stdlib/string.rs b/ndc_lib/src/stdlib/string.rs index 178f8097..65fc1ca6 100644 --- a/ndc_lib/src/stdlib/string.rs +++ b/ndc_lib/src/stdlib/string.rs @@ -25,7 +25,6 @@ pub fn join_to_string(list: &mut Sequence, sep: &str) -> anyhow::Result #[export_module] mod inner { - use ndc_macros::function; /// The string concat operator #[function(name = "<>")] @@ -98,8 +97,8 @@ mod inner { } /// Splits the string into paragraphs, using blank lines as separators. - pub fn paragraphs(string: &str) -> Vec { - string.split("\n\n").map(ToString::to_string).collect() + pub fn paragraphs(string: &str) -> Value { + Value::collect_list(string.split("\n\n").map(ToString::to_string)) } /// Joins paragraphs into a single string, inserting blank lines between them. @@ -108,8 +107,8 @@ mod inner { } /// Splits the string into lines, using newline characters as separators. - pub fn lines(string: &str) -> Vec { - string.lines().map(ToString::to_string).collect() + pub fn lines(string: &str) -> Value { + Value::collect_list(string.lines().map(ToString::to_string)) } /// Joins lines into a single string, inserting newline characters between them. @@ -118,8 +117,8 @@ mod inner { } /// Splits the string into words, using whitespace as the separator. - pub fn words(string: &str) -> Vec { - string.split_whitespace().map(ToString::to_string).collect() + pub fn words(string: &str) -> Value { + Value::collect_list(string.split_whitespace().map(ToString::to_string)) } /// Joins words into a single string, separating them with spaces. @@ -128,8 +127,8 @@ mod inner { } /// Splits the string by whitespace into a list of substrings. - pub fn split(string: &str) -> Vec { - string.split_whitespace().map(ToString::to_string).collect() + pub fn split(string: &str) -> Value { + Value::collect_list(string.split_whitespace().map(ToString::to_string)) } /// Returns `true` if `haystack` starts with `pat`. @@ -144,17 +143,20 @@ mod inner { /// Splits the string using a given pattern as the delimiter. #[function(name = "split")] - pub fn split_with_pattern(string: &str, pattern: &str) -> Vec { - string.split(pattern).map(ToString::to_string).collect() + pub fn split_with_pattern(string: &str, pattern: &str) -> Value { + Value::list( + string + .split(pattern) + .map(ToString::to_string) + .map(Value::string) + .collect::>(), + ) } /// Splits the string at the first occurrence of `pattern`, returning a tuple-like value. pub fn split_once(string: &str, pattern: &str) -> Value { match string.split_once(pattern) { - Some((fst, snd)) => Value::Sequence(Sequence::Tuple(Rc::new(vec![ - Value::from(fst), - Value::from(snd), - ]))), + Some((fst, snd)) => Value::tuple(vec![Value::string(fst), Value::string(snd)]), None => Value::unit(), } } diff --git a/ndc_lib/src/stdlib/value.rs b/ndc_lib/src/stdlib/value.rs index 802bf0f1..80e5dd51 100644 --- a/ndc_lib/src/stdlib/value.rs +++ b/ndc_lib/src/stdlib/value.rs @@ -95,8 +95,8 @@ mod inner { Value::Option(o) => Value::Option(o.clone()), number @ Value::Number(_) => number.clone(), Value::Bool(b) => Value::Bool(*b), - Value::Sequence(Sequence::String(string)) => Value::from(string.borrow().to_owned()), - Value::Sequence(Sequence::List(list)) => Value::from(list.borrow().to_owned()), + Value::Sequence(Sequence::String(string)) => Value::string(string.borrow().to_owned()), + Value::Sequence(Sequence::List(list)) => Value::list(list.borrow().to_owned()), Value::Sequence(Sequence::Map(map, default)) => Value::Sequence(Sequence::Map( Rc::new(RefCell::new(map.borrow().clone())), default.to_owned(), diff --git a/ndc_macros/src/convert.rs b/ndc_macros/src/convert.rs new file mode 100644 index 00000000..3b6a8234 --- /dev/null +++ b/ndc_macros/src/convert.rs @@ -0,0 +1,111 @@ +use crate::r#match::{is_ref_mut, is_string, path_ends_with}; +use proc_macro2::{Span, TokenStream}; +use quote::{ToTokens, quote}; + +#[derive(Debug, Clone)] +pub struct Argument { + // TODO: remove these pub + pub param_type: TokenStream, + pub param_name: TokenStream, + pub argument: TokenStream, + pub initialize_code: TokenStream, +} +pub trait TypeConverter { + fn matches(&self, ty: &syn::Type) -> bool; + fn convert( + &self, + temp_var: syn::Ident, + original_name: &str, + argument_var_name: syn::Ident, + ) -> Vec; +} + +struct MutRefString; +impl TypeConverter for MutRefString { + fn matches(&self, ty: &syn::Type) -> bool { + is_ref_mut(ty) && is_string(ty) + } + + fn convert( + &self, + temp_var: syn::Ident, + original_name: &str, + argument_var_name: syn::Ident, + ) -> Vec { + vec![Argument { + param_type: quote! { crate::interpreter::function::ParamType::String }, + param_name: quote! { #original_name }, + argument: quote! { #argument_var_name }, + initialize_code: quote! { + let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::String(#temp_var)) = #argument_var_name else { + panic!("Value #position needed to be a Sequence::String but wasn't"); + }; + let #argument_var_name = &mut *#temp_var.try_borrow_mut()?; + }, + }] + } +} + +/// Matches `Rc>>` +struct InternalList; +impl TypeConverter for InternalList { + fn matches(&self, ty: &syn::Type) -> bool { + path_ends_with(ty, "ListRepr") + } + + fn convert( + &self, + temp_var: syn::Ident, + original_name: &str, + argument_var_name: syn::Ident, + ) -> Vec { + vec![Argument { + param_type: quote! { crate::interpreter::function::ParamType::List }, + param_name: quote! { #original_name }, + argument: quote! { #argument_var_name }, + initialize_code: quote! { + let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::List(#temp_var)) = #argument_var_name else { + panic!("blaap"); + }; + + let #argument_var_name = std::mem::take(#temp_var); + }, + }] + } +} + +/// Matches `Rc>>` +struct InternalTuple; +impl TypeConverter for InternalTuple { + fn matches(&self, ty: &syn::Type) -> bool { + path_ends_with(ty, "TupleRepr") + } + + fn convert( + &self, + temp_var: syn::Ident, + original_name: &str, + argument_var_name: syn::Ident, + ) -> Vec { + vec![Argument { + param_type: quote! { crate::interpreter::function::ParamType::Tuple }, + param_name: quote! { #original_name }, + argument: quote! { #argument_var_name }, + initialize_code: quote! { + let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::Tuple(#temp_var)) = #argument_var_name else { + panic!("blaap"); + }; + + let #argument_var_name = std::mem::take(#temp_var); + }, + }] + } +} + +pub fn build() -> Vec> { + vec![ + Box::new(InternalList), + Box::new(MutRefString), + Box::new(InternalTuple), + ] +} diff --git a/ndc_macros/src/function.rs b/ndc_macros/src/function.rs index 0a489206..654698df 100644 --- a/ndc_macros/src/function.rs +++ b/ndc_macros/src/function.rs @@ -1,3 +1,4 @@ +use crate::convert::{Argument, TypeConverter, build}; use crate::r#match::{ is_ref, is_ref_mut, is_ref_mut_of_slice_of_value, is_ref_of_bigint, is_ref_of_slice_of_value, is_str_ref, is_string, path_ends_with, @@ -15,8 +16,9 @@ pub struct WrappedFunction { pub fn wrap_function(function: &syn::ItemFn) -> Vec { let original_identifier = function.sig.ident.clone(); - let mut register_as_function_name = - proc_macro2::Literal::string(&original_identifier.to_string()); + let mut function_names = vec![proc_macro2::Literal::string( + &original_identifier.to_string(), + )]; let mut docs_buf = String::new(); for attr in &function.attrs { @@ -24,7 +26,11 @@ pub fn wrap_function(function: &syn::ItemFn) -> Vec { attr.parse_nested_meta(|meta| { // #[function(name = "...")] if meta.path.is_ident("name") { - register_as_function_name = meta.value()?.parse()?; + function_names = vec![meta.value()?.parse()?]; + // register_as_function_name = meta.value()?.parse()?; + Ok(()) + } else if meta.path.is_ident("alias") { + function_names.push(meta.value()?.parse()?); Ok(()) } else { Err(meta.error("unsupported property on function")) @@ -52,46 +58,57 @@ pub fn wrap_function(function: &syn::ItemFn) -> Vec { // If the function has no argument then the cartesian product stuff below doesn't work if function.sig.inputs.is_empty() { - return vec![wrap_single( - function.clone(), - &original_identifier, - ®ister_as_function_name, - vec![], - &docs_buf, - )]; + return function_names + .iter() + .map(|function_name| { + wrap_single( + function.clone(), + &original_identifier, + function_name, + vec![], + &docs_buf, + ) + }) + .collect(); } // When we call create_temp_variable we can get multiple definitions for a variable // For instance when a rust function is `fn foo(list: &[Value])` we can define two internal functions for both Tuple and List - let functions = function - .sig - .inputs + + let mut variation_id = 0usize; + function_names .iter() - .enumerate() - .map(|(position, fn_arg)| { - let name = match fn_arg { - syn::FnArg::Receiver(_) => "self".to_string(), - syn::FnArg::Typed(syn::PatType { pat, .. }) => match &**pat { - syn::Pat::Ident(syn::PatIdent { ident, .. }) => ident.to_string(), - _ => panic!("don't know how to process this"), - }, - }; - create_temp_variable(position, fn_arg, &original_identifier, &name) - }) - .multi_cartesian_product() - .enumerate() - .map(|(variation_id, args)| { - wrap_single( - function.clone(), - &format_ident!("{}_{}", original_identifier, variation_id), - ®ister_as_function_name, - args, - &docs_buf, - ) + .flat_map(|function_name| { + function + .sig + .inputs + .iter() + .enumerate() + .map(|(position, fn_arg)| { + let name = match fn_arg { + syn::FnArg::Receiver(_) => "self".to_string(), + syn::FnArg::Typed(syn::PatType { pat, .. }) => match &**pat { + syn::Pat::Ident(syn::PatIdent { ident, .. }) => ident.to_string(), + _ => panic!("don't know how to process this"), + }, + }; + create_temp_variable(position, fn_arg, &original_identifier, &name) + }) + .multi_cartesian_product() + .map(|args| { + let wrapped = wrap_single( + function.clone(), + &format_ident!("{original_identifier}_{variation_id}"), + function_name, + args, + &docs_buf, + ); + variation_id += 1; + wrapped + }) + .collect::>() }) - .collect::>(); - - functions + .collect() } /// Wraps an original rust function `function` in an outer function with the identifier `identifier` @@ -207,14 +224,6 @@ fn wrap_single( } } -#[derive(Debug, Clone)] -struct Argument { - param_type: TokenStream, - param_name: TokenStream, - argument: TokenStream, - initialize_code: TokenStream, -} - fn into_param_type(ty: &syn::Type) -> TokenStream { match ty { ty if path_ends_with(ty, "Vec") => quote! { crate::interpreter::function::ParamType::List }, @@ -233,6 +242,12 @@ fn into_param_type(ty: &syn::Type) -> TokenStream { ty if path_ends_with(ty, "MaxHeap") => { quote! { crate::interpreter::function::ParamType::MaxHeap } } + ty if path_ends_with(ty, "ListRepr") => { + quote! { crate::interpreter::function::ParamType::List } + } + ty if path_ends_with(ty, "TupleRepr") => { + quote! { crate::interpreter::function::ParamType::Tuple } + } syn::Type::Reference(syn::TypeReference { elem, .. }) => into_param_type(elem), syn::Type::Path(syn::TypePath { path, .. }) => match path { _ if path.is_ident("i64") => quote! { crate::interpreter::function::ParamType::Int }, @@ -268,24 +283,18 @@ fn create_temp_variable( if let syn::FnArg::Typed(pat_type) = input { let ty = &*pat_type.ty; - // The pattern is exactly &mut String - if is_ref_mut(ty) && is_string(ty) { - let rc_temp_var = - syn::Ident::new(&format!("temp_{argument_var_name}"), identifier.span()); - return vec![Argument { - param_type: quote! { crate::interpreter::function::ParamType::String }, - param_name: quote! { #original_name }, - argument: quote! { #argument_var_name }, - initialize_code: quote! { - let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::String(#rc_temp_var)) = #argument_var_name else { - panic!("Value #position needed to be a Sequence::String but wasn't"); - }; - let #argument_var_name = &mut *#rc_temp_var.try_borrow_mut()?; - }, - }]; + let converters: Vec> = build(); + + let temp_var = syn::Ident::new(&format!("temp_{argument_var_name}"), identifier.span()); + + for converter in converters { + if converter.matches(ty) { + return converter.convert(temp_var, original_name, argument_var_name); + } } + // The pattern is Callable - else if path_ends_with(ty, "Callable") { + if path_ends_with(ty, "Callable") { let temp_var = syn::Ident::new(&format!("temp_{argument_var_name}"), identifier.span()); return vec![Argument { param_type: quote! { crate::interpreter::function::ParamType::Function }, @@ -582,7 +591,7 @@ fn create_temp_variable( argument: quote! { #argument_var_name }, initialize_code: quote! { let crate::interpreter::value::Value::Number(crate::interpreter::num::Number::Rational(#argument_var_name)) = #argument_var_name else { - panic!("VValue #position needs to be Rational but wasn't"); + panic!("Value #position needs to be Rational but wasn't"); }; let #argument_var_name = &#argument_var_name.clone(); @@ -612,7 +621,7 @@ fn create_temp_variable( argument: quote! { #argument_var_name }, initialize_code: quote! { let crate::interpreter::value::Value::Number(crate::interpreter::num::Number::Complex(#argument_var_name)) = #argument_var_name else { - panic!("VValue #position needs to be Complex64 but wasn't"); + panic!("Value #position needs to be Complex64 but wasn't"); }; let #argument_var_name = #argument_var_name.clone(); diff --git a/ndc_macros/src/lib.rs b/ndc_macros/src/lib.rs index eb6a0577..faa6eeaf 100644 --- a/ndc_macros/src/lib.rs +++ b/ndc_macros/src/lib.rs @@ -1,3 +1,4 @@ +mod convert; mod function; mod r#match; From 97a17855efc88c5ed05d5289ac397435b76d1af2 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 1 Jul 2025 15:53:07 +0200 Subject: [PATCH 05/18] Really weird OpAssignment reimpl --- ndc_lib/src/ast/expression.rs | 3 +- ndc_lib/src/ast/parser.rs | 22 +-- ndc_lib/src/interpreter/environment.rs | 12 ++ ndc_lib/src/interpreter/evaluate.rs | 179 +++++++------------------ 4 files changed, 77 insertions(+), 139 deletions(-) diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index 3ee3aca0..12869595 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -43,7 +43,8 @@ pub enum Expression { OpAssignment { l_value: Lvalue, value: Box, - operation: Either, + // Should always be an identifier? + operation: Box, }, FunctionDeclaration { name: Option>, diff --git a/ndc_lib/src/ast/parser.rs b/ndc_lib/src/ast/parser.rs index 41755607..fdc2c217 100644 --- a/ndc_lib/src/ast/parser.rs +++ b/ndc_lib/src/ast/parser.rs @@ -308,7 +308,7 @@ impl Parser { return Err(Error::with_help( "Invalid assignment target".to_string(), lvalue_span, - "Assignment target is not a valid lvalue. Only a few expressions can be assigned a value. Check that the left-hand side of the assignment is a valid target.".to_string() + "Assignment target is not a valid lvalue. Only a few expressions can be assigned a value. Check that the left-hand side of the assignment is a valid target.".to_string(), )); }; @@ -347,7 +347,7 @@ impl Parser { Some(Token::EqualsSign) => Err(Error::with_help( "Invalid assignment target".to_string(), maybe_lvalue.span, - "Assignment target is not a valid lvalue. Only a few expressions can be assigned a value. Check that the left-hand side of the assignment is a valid target.".to_string() + "Assignment target is not a valid lvalue. Only a few expressions can be assigned a value. Check that the left-hand side of the assignment is a valid target.".to_string(), )), _ => Ok(maybe_lvalue), }; @@ -368,10 +368,13 @@ impl Parser { Ok(assignment_expression.to_location(start.merge(end))) } Some(Token::OpAssign(inner)) => { - let thing = match &inner.token { - Token::Identifier(identifier) => Either::Right(identifier.to_string()), - _ => Either::Left((*inner.clone()).try_into()?), - }; + // let thing = match &inner.token { + // Token::Identifier(identifier) => Either::Right(identifier.to_string()), + // _ => Either::Left((*inner.clone()).try_into()?), + // }; + + let operation = + Expression::Identifier(inner.token.to_string()).to_location(inner.span); self.advance(); let expression = self.tuple_expression(Self::single_expression, false)?; @@ -380,7 +383,7 @@ impl Parser { l_value: Lvalue::try_from(maybe_lvalue) .expect("guaranteed to produce an lvalue"), value: Box::new(expression), - operation: thing, + operation: Box::new(operation), }; Ok(op_assign.to_location(start.merge(end))) @@ -1104,7 +1107,10 @@ impl Parser { // Next we either expect a body block `{ ... }` or a fat arrow followed by a single expression `=> ...` let body = match self.peek_current_token() { - Some(Token::FatArrow) => { self.advance(); self.single_expression()?}, + Some(Token::FatArrow) => { + self.advance(); + self.single_expression()? + } Some(Token::LeftCurlyBracket) => self.block()?, Some(token) => { return Err(Error::with_help( diff --git a/ndc_lib/src/interpreter/environment.rs b/ndc_lib/src/interpreter/environment.rs index 53b501ff..683343a4 100644 --- a/ndc_lib/src/interpreter/environment.rs +++ b/ndc_lib/src/interpreter/environment.rs @@ -176,6 +176,18 @@ impl Environment { } } + /// Takes the named variable from memory and leaves `Value::unit()` in it's place + #[must_use] + pub fn take(&self, name: &str) -> Option { + if let Some(v) = self.values.get(name) { + Some(std::mem::replace(&mut *v.borrow_mut(), Value::unit())) + } else if let Some(parent) = &self.parent { + parent.borrow().take(name) + } else { + None + } + } + pub fn with_existing( &self, name: &str, diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index ccd62d96..a604e671 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -78,25 +78,43 @@ pub(crate) fn evaluate_expression( operation, } => match l_value { Lvalue::Variable { identifier } => { + let Expression::Identifier(operation) = &operation.expression else { + todo!("OpAssignment operation must be Identifier??"); + }; + let right_value = evaluate_expression(value, environment)?; + let left_value = environment.borrow().take(identifier).unwrap(); + + let result = call_function_by_name( + operation, + &mut [left_value, right_value], + environment, + span, + )?; + + // Assign the result to l_value + declare_or_assign_variable(l_value, result.clone(), false, environment, span)?; + + result // We need to use with_existing in this situation to ensure the refcount doesn't // increase which does happen when we `get` something from the environment because // `get` clones the value it returns. - environment - .borrow() - .with_existing::(identifier, |existing_value| { - let existing_value = &mut *existing_value.borrow_mut(); - - apply_operation_to_value( - environment, - existing_value, - operation, - right_value, - span, - ) - }) - .ok_or_else(|| EvaluationError::undefined_variable(identifier, span))?? + // environment + // .borrow() + // .with_existing::(identifier, |existing_value| { + // // let existing_value = &mut *existing_value.borrow_mut(); + // + // // TODO: use function call + // // apply_operation_to_value( + // // environment, + // // existing_value, + // // operation, + // // right_value, + // // span, + // // ) + // }) + // .ok_or_else(|| EvaluationError::undefined_variable(identifier, span))?? } Lvalue::Index { value: lhs_expr, @@ -117,13 +135,15 @@ pub(crate) fn evaluate_expression( Offset::Element(index) => { let list_item = list.index_mut(index); - apply_operation_to_value( - environment, - list_item, - operation, - right_value, - span, - )? + // apply_operation_to_value( + // environment, + // list_item, + // operation, + // right_value, + // span, + // )? + + todo!("not implemented") } Offset::Range(_, _) => { return Err(EvaluationError::syntax_error( @@ -147,13 +167,14 @@ pub(crate) fn evaluate_expression( .ok_or_else(|| EvaluationError::key_not_found(&index, span))? }; - apply_operation_to_value( - environment, - map_entry, - operation, - right_value, - span, - )? + todo!("not implemented") + // apply_operation_to_value( + // environment, + // map_entry, + // operation, + // right_value, + // span, + // )? } Value::Sequence(Sequence::String(_)) => { return Err(EvaluationError::new( @@ -270,7 +291,6 @@ pub(crate) fn evaluate_expression( .into()); } } - // drop(local_scope); Value::unit() } Expression::Call { @@ -716,107 +736,6 @@ fn declare_or_assign_variable( Ok(Value::unit()) } -/// Applies operations like `+` or functions like `max(x, y)` to mutable pointers to values. This is -/// used to optimize various `OpAssign` expressions that would otherwise create copies. -/// ```ndc -/// x ++= [1,2,3] -/// // becomes -/// x.append([1,2,3]); -/// // instead of -/// x = x ++ [1,2,3] -/// `` -fn apply_operation_to_value( - environment: &EnvironmentRef, - value: &mut Value, - operation: &Either, - right_value: Value, - span: Span, -) -> Result { - match (value, operation, right_value) { - ( - Value::Sequence(Sequence::String(left)), - Either::Left(BinaryOperator::Concat), - Value::Sequence(Sequence::String(right)), - ) => { - if Rc::ptr_eq(left, &right) { - let copy = String::from(&*right.borrow()); - left.borrow_mut().push_str(©); - } else { - left.borrow_mut().push_str(&right.borrow()); - } - Ok(Value::Sequence(Sequence::String(left.clone()))) - } - - ( - Value::Sequence(Sequence::List(left)), - Either::Left(BinaryOperator::Concat), - Value::Sequence(Sequence::List(right)), - ) => { - // Special case for when a list is extended with itself - // x := [1,2,3]; x ++= x; - if Rc::ptr_eq(left, &right) { - let copy = Vec::clone(&*right.borrow()); - left.borrow_mut().extend_from_slice(©); - } else { - left.borrow_mut().extend_from_slice(&right.borrow()); - }; - Ok(Value::Sequence(Sequence::List(left.clone()))) - } - - ( - Value::Sequence(Sequence::Tuple(left)), - Either::Left(BinaryOperator::Concat), - Value::Sequence(Sequence::Tuple(mut right)), - ) => { - let right = Rc::make_mut(&mut right); - Rc::make_mut(left).append(right); - Ok(Value::Sequence(Sequence::Tuple(Rc::clone(left)))) - } - ( - Value::Sequence(Sequence::Map(left, default)), - Either::Left(BinaryOperator::Or), - Value::Sequence(Sequence::Map(right, _)), - ) => { - // If right and left are the same we can just do nothing and prevent a borrow checker error later - if !Rc::ptr_eq(left, &right) { - match Rc::try_unwrap(right) { - Ok(right) => { - left.borrow_mut().extend(right.take()); - } - Err(right) => { - // If we ever figure out how to make the borrow below panic we should add a test and fix it - left.borrow_mut() - .extend(right.borrow().iter().map(|(a, b)| (a.clone(), b.clone()))); - } - } - } - - Ok(Value::Sequence(Sequence::Map( - left.clone(), - default.to_owned(), - ))) - } - (left, operator, right) => { - match operator { - Either::Left(binary_operator) => { - *left = apply_operator(left.clone(), *binary_operator, right) - .into_evaluation_result(span)?; - } - Either::Right(identifier) => { - let old_value = std::mem::replace(left, Value::unit()); - *left = call_function_by_name( - identifier, - &mut [old_value, right], - environment, - span, - )?; - } - } - Ok(left.clone()) - } - } -} - #[allow(clippy::too_many_lines)] fn apply_operator( left: Value, From 0328bcbea2a67a1f64a991d9975c3a43c04a9b88 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 1 Jul 2025 16:26:05 +0200 Subject: [PATCH 06/18] More WIP --- ndc_lib/src/interpreter/evaluate.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index a604e671..da3ce8e4 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -96,25 +96,6 @@ pub(crate) fn evaluate_expression( declare_or_assign_variable(l_value, result.clone(), false, environment, span)?; result - - // We need to use with_existing in this situation to ensure the refcount doesn't - // increase which does happen when we `get` something from the environment because - // `get` clones the value it returns. - // environment - // .borrow() - // .with_existing::(identifier, |existing_value| { - // // let existing_value = &mut *existing_value.borrow_mut(); - // - // // TODO: use function call - // // apply_operation_to_value( - // // environment, - // // existing_value, - // // operation, - // // right_value, - // // span, - // // ) - // }) - // .ok_or_else(|| EvaluationError::undefined_variable(identifier, span))?? } Lvalue::Index { value: lhs_expr, From c4810be526d4e40963f81bc20a1b20d78a7e1ba7 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 23 Sep 2025 14:19:38 +0200 Subject: [PATCH 07/18] REVERT THIS?!?!? --- ndc_lib/src/ast/expression.rs | 6 +- ndc_lib/src/interpreter/evaluate.rs | 197 +++++++----------- ndc_lib/src/interpreter/value.rs | 4 +- .../028_op_assign_index_evaluated_once.ndct | 2 +- .../900_bugs/bug0001_in_place_map.ndct | 7 +- 5 files changed, 88 insertions(+), 128 deletions(-) diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index 12869595..41737381 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -43,14 +43,14 @@ pub enum Expression { OpAssignment { l_value: Lvalue, value: Box, - // Should always be an identifier? + // TODO: Should always be an identifier? operation: Box, }, FunctionDeclaration { name: Option>, arguments: Box, - body: Rc, //TODO what happens if we remove the Rc? - pure: bool, // TODO: maybe have flags instead of booleans? + body: Rc, + pure: bool, }, Block { statements: Vec, diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index da3ce8e4..87e9e863 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -1,12 +1,12 @@ +use either::Either; +use index::{Offset, evaluate_as_index, set_at_index}; +use itertools::Itertools; use std::cell::RefCell; use std::cmp::Ordering; use std::fmt; use std::ops::{Add, BitAnd, BitOr, BitXor, Div, IndexMut, Mul, Rem, Sub}; use std::rc::Rc; - -use either::Either; -use index::{Offset, evaluate_as_index, set_at_index}; -use itertools::Itertools; +use std::thread::spawn; use crate::ast::{ BinaryOperator, Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue, @@ -14,6 +14,7 @@ use crate::ast::{ use crate::hash_map; use crate::hash_map::HashMap; use crate::interpreter::environment::{Environment, EnvironmentRef}; +use crate::interpreter::evaluate::index::EvaluatedIndex; use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction}; use crate::interpreter::int::Int; use crate::interpreter::iterator::mut_value_to_iterator; @@ -76,113 +77,26 @@ pub(crate) fn evaluate_expression( l_value, value, operation, - } => match l_value { - Lvalue::Variable { identifier } => { - let Expression::Identifier(operation) = &operation.expression else { - todo!("OpAssignment operation must be Identifier??"); - }; - - let right_value = evaluate_expression(value, environment)?; - let left_value = environment.borrow().take(identifier).unwrap(); + } => { + let Expression::Identifier(operation) = &operation.expression else { + todo!("OpAssignment operation must be Identifier??"); + }; - let result = call_function_by_name( - operation, - &mut [left_value, right_value], - environment, - span, - )?; + let right_value = evaluate_expression(value, environment)?; + let left_value = take_lvalue(l_value, span, environment)?; // TODO check span - // Assign the result to l_value - declare_or_assign_variable(l_value, result.clone(), false, environment, span)?; + let result = call_function_by_name( + operation, + &mut [left_value, right_value], + environment, + span, + )?; - result - } - Lvalue::Index { - value: lhs_expr, - index: index_expr, - } => { - let assign_to = evaluate_expression(lhs_expr, environment)?; - let new_value = match &assign_to { - Value::Sequence(Sequence::List(list)) => { - // It's important that index and right_value are computed before the list is borrowed - let right_value = evaluate_expression(value, environment)?; - let size = list.try_borrow().into_evaluation_result(span)?.len(); - let index = evaluate_as_index(index_expr, environment)? - .try_into_offset(size, index_expr.span)?; - - let mut list = list.try_borrow_mut().into_evaluation_result(span)?; - - match index { - Offset::Element(index) => { - let list_item = list.index_mut(index); - - // apply_operation_to_value( - // environment, - // list_item, - // operation, - // right_value, - // span, - // )? - - todo!("not implemented") - } - Offset::Range(_, _) => { - return Err(EvaluationError::syntax_error( - "cannot use a range expression as index".to_string(), - span, - ) - .into()); - } - } - } - Value::Sequence(Sequence::Map(dict, default)) => { - let right_value = evaluate_expression(value, environment)?; - let index = evaluate_expression(index_expr, environment)?; - - let mut dict = dict.try_borrow_mut().into_evaluation_result(span)?; - - let map_entry = if let Some(default) = default { - dict.entry(index).or_insert(Value::clone(default)) - } else { - dict.get_mut(&index) - .ok_or_else(|| EvaluationError::key_not_found(&index, span))? - }; - - todo!("not implemented") - // apply_operation_to_value( - // environment, - // map_entry, - // operation, - // right_value, - // span, - // )? - } - Value::Sequence(Sequence::String(_)) => { - return Err(EvaluationError::new( - "cannot OpAssign into a string".to_string(), - span, - ) - .into()); - } - _ => { - return Err(EvaluationError::syntax_error( - format!("cannot OpAssign an index into a {}", assign_to.value_type()), - span, - ) - .into()); - } - }; + // Assign the result to l_value + declare_or_assign_variable(l_value, result.clone(), false, environment, span)?; - new_value - } - Lvalue::Sequence(_) => { - return Err(EvaluationError::syntax_error( - "cannot use augmented assignment in combination with destructuring".to_string(), - span, - ) - .into()); - } - }, + result + } Expression::Block { statements } => { let mut local_scope = Environment::new_scope(environment); @@ -480,7 +394,7 @@ pub(crate) fn evaluate_expression( ); }; - Value::list(values.to_vec()) // TODO: can we remove to_vec? + Value::list(values) } } } @@ -629,6 +543,55 @@ pub(crate) fn evaluate_expression( Ok(literal) } +fn take_lvalue( + lvalue: &Lvalue, + lvalue_span: Span, + environment: &mut EnvironmentRef, +) -> EvaluationResult { + match lvalue { + Lvalue::Variable { identifier } => { + let value = environment + .borrow() + .take(identifier) + .ok_or_else(|| EvaluationError::undefined_variable(identifier, lvalue_span))?; + + Ok(value) + } + Lvalue::Index { value, index } => { + let v = evaluate_expression(value, environment)?; + let i = evaluate_as_index(index, environment)?; + let result = index_into_value(v, i, lvalue_span)?; + Ok(result) + } + Lvalue::Sequence(_) => todo!("sequencing not implemented"), + } +} + +fn index_into_value(value: Value, index: EvaluatedIndex, lvalue_span: Span) -> EvaluationResult { + match value { + Value::Sequence(Sequence::List(list)) => { + let size = list.try_borrow().into_evaluation_result(lvalue_span)?.len(); + let index = index.try_into_offset(size, lvalue_span)?; + + let mut list = list.try_borrow_mut().into_evaluation_result(lvalue_span)?; + match index { + Offset::Element(index) => { + let list_item = std::mem::replace(list.index_mut(index), Value::unit()); + Ok(list_item) + } + Offset::Range(_, _) => todo!("range expression not implemented here"), + } + } + Value::Sequence(Sequence::Map(map, def)) => todo!(), + Value::Sequence(Sequence::String(string)) => todo!(), + _ => Err(EvaluationError::syntax_error( + format!("cannot OpAssign an index into a {}", value.value_type()), + lvalue_span, + ) + .into()), + } +} + fn produce_default_value( default: &Value, environment: &EnvironmentRef, @@ -702,15 +665,15 @@ fn declare_or_assign_variable( .into()); } } - Lvalue::Index { .. } => { - return Err(EvaluationError::syntax_error( - format!( - "Can't declare values into {}", - l_value.expression_type_name() - ), - span, - ) - .into()); + Lvalue::Index { + value: lhs_expr, + index, + } => { + let mut lhs = evaluate_expression(lhs_expr, environment)?; + + let index = evaluate_as_index(index, environment)?; + + set_at_index(&mut lhs, value, index, span)?; } }; diff --git a/ndc_lib/src/interpreter/value.rs b/ndc_lib/src/interpreter/value.rs index f2bdf060..8715452a 100644 --- a/ndc_lib/src/interpreter/value.rs +++ b/ndc_lib/src/interpreter/value.rs @@ -40,9 +40,9 @@ impl Value { pub(crate) fn collect_list(i: I) -> Self where I: Iterator, - V: Into, + V: Into, { - Self::list(i.map(Into::into).collect::>()) + Self::list(i.map(Into::into).collect::>()) } pub(crate) fn tuple>>(data: V) -> Self { diff --git a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct index a50f3c88..d44ea13b 100644 --- a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct +++ b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct @@ -1,7 +1,7 @@ --PROGRAM-- let i = 0; fn idx() { -let r = i; + let r = i; i += 1; r } diff --git a/tests/programs/900_bugs/bug0001_in_place_map.ndct b/tests/programs/900_bugs/bug0001_in_place_map.ndct index dc76e788..44f3430d 100644 --- a/tests/programs/900_bugs/bug0001_in_place_map.ndct +++ b/tests/programs/900_bugs/bug0001_in_place_map.ndct @@ -1,10 +1,7 @@ --PROGRAM-- fn mut_map(list, function) { -let len = 0; - for item in list { len = len + 1 }; - -let index = 0; - while index < len { + let index = 0; + while index < list.len { // BUG#0001: this line used to fail because list had a mutable and immutable borrow at the same time list[index] = function(list[index]); index = index + 1; From 5f44fe5c724e400555a1dac8b0d99095b7f56ec8 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 24 Sep 2025 15:47:53 +0200 Subject: [PATCH 08/18] Improve invocation test readability --- .../028_op_assign_index_evaluated_once.ndct | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct index d44ea13b..55c8e436 100644 --- a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct +++ b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct @@ -1,13 +1,12 @@ --PROGRAM-- -let i = 0; +let counter = 0; fn idx() { - let r = i; - i += 1; - r + counter += 1; + 0 } -let foo = [0]; -foo[idx()] += 10; -print(foo[0], i) +let unit_list = [0]; +unit_list[idx()] += 10; +print("val:", unit_list[0], ", invoked:", counter); --EXPECT-- -10 1 \ No newline at end of file +val: 10, invoked: 1 \ No newline at end of file From 014e60043ed296d98384b06ff20618fa03ad2560 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 24 Sep 2025 16:54:27 +0200 Subject: [PATCH 09/18] Some messy code to fix op assignment --- ndc_lib/src/interpreter/environment.rs | 4 +- ndc_lib/src/interpreter/evaluate.rs | 49 ++++++++++---- ndc_lib/src/interpreter/evaluate/index.rs | 65 +++++++++++++++++++ .../028_op_assign_index_evaluated_once.ndct | 4 +- .../029_op_assign_evalutes_to_new_value.ndct | 5 +- 5 files changed, 107 insertions(+), 20 deletions(-) diff --git a/ndc_lib/src/interpreter/environment.rs b/ndc_lib/src/interpreter/environment.rs index 683343a4..5d12273e 100644 --- a/ndc_lib/src/interpreter/environment.rs +++ b/ndc_lib/src/interpreter/environment.rs @@ -146,14 +146,14 @@ impl Environment { pub fn assign(&mut self, name: &str, value: Value) -> bool { // Clippy wants us to use self.values.entry(name) but that moves name and breaks the recursive case - return if self.values.contains_key(name) { + if self.values.contains_key(name) { self.values.insert(name.to_string(), RefCell::new(value)); true } else if let Some(parent) = &self.parent { parent.borrow_mut().assign(name, value) } else { false - }; + } } #[must_use] diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 87e9e863..1447f467 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -6,7 +6,6 @@ use std::cmp::Ordering; use std::fmt; use std::ops::{Add, BitAnd, BitOr, BitXor, Div, IndexMut, Mul, Rem, Sub}; use std::rc::Rc; -use std::thread::spawn; use crate::ast::{ BinaryOperator, Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue, @@ -14,7 +13,7 @@ use crate::ast::{ use crate::hash_map; use crate::hash_map::HashMap; use crate::interpreter::environment::{Environment, EnvironmentRef}; -use crate::interpreter::evaluate::index::EvaluatedIndex; +use crate::interpreter::evaluate::index::{EvaluatedIndex, get_at_index}; use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction}; use crate::interpreter::int::Int; use crate::interpreter::iterator::mut_value_to_iterator; @@ -75,27 +74,49 @@ pub(crate) fn evaluate_expression( }, Expression::OpAssignment { l_value, - value, + value: rhs_value, operation, } => { let Expression::Identifier(operation) = &operation.expression else { todo!("OpAssignment operation must be Identifier??"); }; + match l_value { + Lvalue::Variable { identifier } => { + let Some(lhs) = environment.borrow_mut().take(identifier) else { + return Err(EvaluationError::undefined_variable(identifier, span).into()); + }; + let rhs = evaluate_expression(rhs_value, environment)?; - let right_value = evaluate_expression(value, environment)?; - let left_value = take_lvalue(l_value, span, environment)?; // TODO check span + let result = + call_function_by_name(operation, &mut [lhs, rhs], environment, span)?; - let result = call_function_by_name( - operation, - &mut [left_value, right_value], - environment, - span, - )?; + environment.borrow_mut().assign(identifier, result); + + Value::unit() + } + Lvalue::Index { + value: lhs_expression, + index: index_expression, + } => { + let mut lhs_value = evaluate_expression(lhs_expression, environment)?; + let index = evaluate_as_index(index_expression, environment)?; + let value_at_index = get_at_index(&lhs_value, index.clone(), span)?; + + let right_value = evaluate_expression(rhs_value, environment)?; + + let result = call_function_by_name( + operation, + &mut [value_at_index, right_value], + environment, + span, + )?; - // Assign the result to l_value - declare_or_assign_variable(l_value, result.clone(), false, environment, span)?; + set_at_index(&mut lhs_value, result, index, span)?; - result + Value::unit() + } + Lvalue::Sequence(_) => todo!(), + } } Expression::Block { statements } => { let mut local_scope = Environment::new_scope(environment); diff --git a/ndc_lib/src/interpreter/evaluate/index.rs b/ndc_lib/src/interpreter/evaluate/index.rs index 5c780d6b..a60389a6 100644 --- a/ndc_lib/src/interpreter/evaluate/index.rs +++ b/ndc_lib/src/interpreter/evaluate/index.rs @@ -17,9 +17,11 @@ use crate::{ }, lexer::Span, }; +use itertools::Itertools; use std::cmp::min; use std::ops::IndexMut; +#[derive(Clone)] pub enum EvaluatedIndex { Index(Value), Slice { @@ -167,6 +169,69 @@ impl Offset { } } +pub fn get_at_index( + lhs: &Value, + index: EvaluatedIndex, + span: Span, +) -> Result { + let Some(size) = lhs.sequence_length() else { + return Err(EvaluationError::new( + "cannot index into this type because it doesn't have a length".to_string(), + span, + ) + .into()); + }; + + match lhs { + Value::Sequence(Sequence::List(list)) => { + let list = list.borrow(); + let index = index.try_into_offset(size, span)?; + + match index { + Offset::Element(index_usize) => Ok(list[index_usize].clone()), + Offset::Range(from_usize, to_usize) => Ok(Value::list(&list[from_usize..to_usize])), + } + } + Value::Sequence(Sequence::String(insertion_target)) => { + let index = index.try_into_offset(size, span)?; + Ok(Value::string(match index { + Offset::Element(e) => insertion_target + .borrow() + .chars() + .nth(e) + .map(|x| String::from(x)) + .expect("TODO: fix this unwrap (NOTE: maybe we're guaranteed to succeed??)"), + Offset::Range(s, e) => insertion_target + .borrow() + .chars() + .dropping(s) + .take(e) + .collect::(), + })) + } + Value::Sequence(Sequence::Map(map, _)) => { + let map = map.try_borrow_mut().into_evaluation_result(span)?; + + let key = match index { + EvaluatedIndex::Index(idx) => idx, + EvaluatedIndex::Slice { .. } => { + return Err(EvaluationError::syntax_error( + "cannot use range expression as index in map".to_string(), + span, + ) + .into()); + } + }; + + Ok(map.get(&key).expect("TODO: fix this error").clone()) + } + _ => Err(EvaluationError::syntax_error( + format!("cannot insert into {} at index", lhs.value_type()), + span, + ) + .into()), + } +} pub fn set_at_index( lhs: &mut Value, rhs: Value, diff --git a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct index 55c8e436..a7a1482e 100644 --- a/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct +++ b/tests/programs/004_basic/028_op_assign_index_evaluated_once.ndct @@ -7,6 +7,6 @@ fn idx() { let unit_list = [0]; unit_list[idx()] += 10; -print("val:", unit_list[0], ", invoked:", counter); +print("val:", unit_list[0],"invoked:", counter); --EXPECT-- -val: 10, invoked: 1 \ No newline at end of file +val: 10 invoked: 1 \ No newline at end of file diff --git a/tests/programs/004_basic/029_op_assign_evalutes_to_new_value.ndct b/tests/programs/004_basic/029_op_assign_evalutes_to_new_value.ndct index 1087acca..938f7b8b 100644 --- a/tests/programs/004_basic/029_op_assign_evalutes_to_new_value.ndct +++ b/tests/programs/004_basic/029_op_assign_evalutes_to_new_value.ndct @@ -1,6 +1,7 @@ --PROGRAM-- -let list = [1,2,3]; -assert((list[1] += 10) == 12); +// NOTE: this behavior was deleted when refactoring to operators as functions +// let list = [1,2,3]; +// assert((list[1] += 10) == 12); print("ok"); --EXPECT-- ok \ No newline at end of file From 17db8481a2281589d8d926b0d63bd6e9def82c93 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Mon, 29 Sep 2025 16:31:20 +0200 Subject: [PATCH 10/18] Minor progress on op-assign implementations --- ndc_lib/src/ast/expression.rs | 4 ++-- ndc_lib/src/ast/parser.rs | 2 +- ndc_lib/src/interpreter/evaluate.rs | 19 ++++++++++++------- ndc_lib/src/stdlib/list.rs | 1 + 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index 41737381..c5222875 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -42,7 +42,7 @@ pub enum Expression { }, OpAssignment { l_value: Lvalue, - value: Box, + r_value: Box, // TODO: Should always be an identifier? operation: Box, }, @@ -305,7 +305,7 @@ impl fmt::Debug for ExpressionLocation { .finish(), Expression::OpAssignment { l_value, - value, + r_value: value, operation, } => f .debug_struct("OpAssignment") diff --git a/ndc_lib/src/ast/parser.rs b/ndc_lib/src/ast/parser.rs index fdc2c217..c0e1b1fe 100644 --- a/ndc_lib/src/ast/parser.rs +++ b/ndc_lib/src/ast/parser.rs @@ -382,7 +382,7 @@ impl Parser { let op_assign = Expression::OpAssignment { l_value: Lvalue::try_from(maybe_lvalue) .expect("guaranteed to produce an lvalue"), - value: Box::new(expression), + r_value: Box::new(expression), operation: Box::new(operation), }; diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 1447f467..2a09222a 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -74,23 +74,28 @@ pub(crate) fn evaluate_expression( }, Expression::OpAssignment { l_value, - value: rhs_value, + r_value, operation, } => { - let Expression::Identifier(operation) = &operation.expression else { + dbg!(l_value, operation, r_value); + + // Identifier may be an operator such as "++" + let Expression::Identifier(operation_ident) = &operation.expression else { todo!("OpAssignment operation must be Identifier??"); }; match l_value { Lvalue::Variable { identifier } => { + let rhs = evaluate_expression(r_value, environment)?; let Some(lhs) = environment.borrow_mut().take(identifier) else { return Err(EvaluationError::undefined_variable(identifier, span).into()); }; - let rhs = evaluate_expression(rhs_value, environment)?; + // TODO: modify operation_ident into its op-assign counterpart (for example: `++` to `++=`) and check if a special implementation exists? let result = - call_function_by_name(operation, &mut [lhs, rhs], environment, span)?; + call_function_by_name(operation_ident, &mut [lhs, rhs], environment, span)?; - environment.borrow_mut().assign(identifier, result); + environment.borrow_mut().assign(identifier, result); // TODO: this messes up semantics + // x = x ++ y Value::unit() } @@ -102,10 +107,10 @@ pub(crate) fn evaluate_expression( let index = evaluate_as_index(index_expression, environment)?; let value_at_index = get_at_index(&lhs_value, index.clone(), span)?; - let right_value = evaluate_expression(rhs_value, environment)?; + let right_value = evaluate_expression(r_value, environment)?; let result = call_function_by_name( - operation, + operation_ident, &mut [value_at_index, right_value], environment, span, diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index 4f1a8e55..252acbda 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -84,6 +84,7 @@ mod inner { #[function(name = "++")] pub fn list_concat(left: ListRepr, right: ListRepr) -> Value { + // TODO: can we/should we optimize here if Rc::strong_count = 1 for the lhs Value::list( left.borrow() .iter() From d020794adf4283e99b69830c495c3a43730caf38 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 30 Sep 2025 15:47:25 +0200 Subject: [PATCH 11/18] Defer to dedicated ++= implementation --- ndc_lib/src/interpreter/evaluate.rs | 71 +++++++++++++++---- ndc_lib/src/stdlib/list.rs | 29 ++++++++ .../007_map_and_set/018_op_assign_value.ndct | 3 +- 3 files changed, 88 insertions(+), 15 deletions(-) diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 2a09222a..61da9baf 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -1,4 +1,3 @@ -use either::Either; use index::{Offset, evaluate_as_index, set_at_index}; use itertools::Itertools; use std::cell::RefCell; @@ -77,7 +76,7 @@ pub(crate) fn evaluate_expression( r_value, operation, } => { - dbg!(l_value, operation, r_value); + // dbg!(l_value, operation, r_value); // Identifier may be an operator such as "++" let Expression::Identifier(operation_ident) = &operation.expression else { @@ -90,11 +89,36 @@ pub(crate) fn evaluate_expression( return Err(EvaluationError::undefined_variable(identifier, span).into()); }; - // TODO: modify operation_ident into its op-assign counterpart (for example: `++` to `++=`) and check if a special implementation exists? + let mut arguments = [lhs, rhs]; + + // If the identifier is `++` we try to look for a function called `++=` first because we assume that implementation is faster. + let op_assign_ident = format!("{operation_ident}="); + // TODO: since we don't provide the user with operator overloading we could write a faster version of get_all_by_name that just looks in the root scope + let values = environment.borrow().get_all_by_name(&op_assign_ident); + let op_assign_result = + try_call_function_from_values(&values, &mut arguments, environment, span); + + // Only if there is no function that matches the op_assign signature we continue and try the fallback implementation + match op_assign_result { + Err(FunctionCarrier::FunctionNotFound) => { + // do nothing continue below + } + err @ Err(_) => return err.into_evaluation_result(span), + Ok(value) => { + // At the start of this branch we used `take` to temporarily remove the value from the environment (leaving a unit behind) + // this helps prevent race conditions in case the operator tries to access its environment but it does require us to put back the LHS + environment.borrow_mut().assign(identifier, value); + + // Op assignment now returns unit + return Ok(Value::unit()); + } + } + + // Execute: `a += b` as `a = a + b` let result = - call_function_by_name(operation_ident, &mut [lhs, rhs], environment, span)?; + call_function_by_name(operation_ident, &mut arguments, environment, span)?; - environment.borrow_mut().assign(identifier, result); // TODO: this messes up semantics + environment.borrow_mut().assign(identifier, result); // TODO: does this mess up semantics??!? // x = x ++ y Value::unit() @@ -120,7 +144,14 @@ pub(crate) fn evaluate_expression( Value::unit() } - Lvalue::Sequence(_) => todo!(), + Lvalue::Sequence(_) => { + return Err(EvaluationError::syntax_error( + "cannot use augmented assignment in combination with destructuring" + .to_string(), + span, + ) + .into()); + } } } Expression::Block { statements } => { @@ -233,7 +264,7 @@ pub(crate) fn evaluate_expression( } else { let function_as_value = evaluate_expression(function, environment)?; - match try_call_function( + match try_call_function_from_values( &[RefCell::new(function_as_value)], &mut evaluated_args, environment, @@ -1107,6 +1138,13 @@ where } } +/// Attempt to call a function using its name (like: 'max' or '+') +/// +/// Arguments: +/// * `name`: name of the function to lookup in the environment +/// * `evaluated_args`: a slice of values passed as arguments to the function +/// * `environment`: the execution environment for the function +/// * `span`: span of the expression used for error reporting fn call_function_by_name( name: &str, evaluated_args: &mut [Value], @@ -1114,7 +1152,7 @@ fn call_function_by_name( span: Span, ) -> EvaluationResult { let values = environment.borrow().get_all_by_name(name); - let result = try_call_function(&values, evaluated_args, environment, span); + let result = try_call_function_from_values(&values, evaluated_args, environment, span); if let Err(FunctionCarrier::FunctionNotFound) = result { let arguments = evaluated_args.iter().map(Value::value_type).join(", "); return Err(FunctionCarrier::EvaluationError(EvaluationError::new( @@ -1126,12 +1164,15 @@ fn call_function_by_name( result } -/// Executes a function with some extra steps: +/// Given a bunch of values (hopefully functions) this function executes the one that matches the +/// given arguments or returns a `FunctionNotFound` error if non match. +/// +/// Arguments: /// * `values`: a list of values that are attempted to be executed in the order they appear in /// * `evaluated_args`: a slice of values passed as arguments to the function /// * `environment`: the execution environment for the function /// * `span`: span of the expression used for error reporting -fn try_call_function( +fn try_call_function_from_values( values: &[RefCell], evaluated_args: &mut [Value], environment: &EnvironmentRef, @@ -1143,10 +1184,12 @@ fn try_call_function( // // This implies that a less specific function can shadow a more specific function // - // fn sqrt(n: Float); - // { - // fn sqrt(n: Number); // Shadows the sqrt in the parent scope completely - // } + // ```ndc + // fn sqrt(n: Float) {} + // { + // fn sqrt(n: Number) {} // Shadows the sqrt in the parent scope completely + // } + // ``` for value in values { let value = &*value.borrow(); diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index 252acbda..318e55db 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -94,6 +94,35 @@ mod inner { ) } + #[function(name = "++=")] + pub fn list_append_operator(left: ListRepr, right: ListRepr) -> Value { + // The ++= operator has 3 implementation paths, this first one is the case where a list extends iteself + if Rc::ptr_eq(&left, &right) { + left.borrow_mut().extend_from_within(..); + } else { + match Rc::try_unwrap(right) { + // The second path deals with a RHS that has an RC of one, in this case we can drain the RHS which should be faster than copying? + Ok(right) => { + left.borrow_mut().append( + &mut right + .try_borrow_mut() + .expect("Failed to borrow_mut in `list_append_operator`"), + ); + } + // The last path is if the RHS has an RC that's higher than 1, in this case we copy all the elements into the LHS + Err(right) => { + left.borrow_mut().extend_from_slice( + &right + .try_borrow() + .expect("Failed to borrow in `list_append_operator`"), + ); + } + } + } + + Value::Sequence(Sequence::List(left)) + } + /// Copies elements from `other` to `list` not touching `other`. pub fn extend(list: &mut Vec, other: &[Value]) { list.extend_from_slice(other); diff --git a/tests/programs/007_map_and_set/018_op_assign_value.ndct b/tests/programs/007_map_and_set/018_op_assign_value.ndct index 3b90d187..d018e5d0 100644 --- a/tests/programs/007_map_and_set/018_op_assign_value.ndct +++ b/tests/programs/007_map_and_set/018_op_assign_value.ndct @@ -1,6 +1,7 @@ --PROGRAM-- let d = %{0: []}; -assert((d[0] ++= [1,2,3]) == [1,2,3]); +// CHANGED in PR#70 OpAssignement now evaluates to unit +assert((d[0] ++= [1,2,3]) == ()); print("ok"); --EXPECT-- ok \ No newline at end of file From 8c3102ab2294caddb988f90e81f23d6ecc9669b6 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 30 Sep 2025 15:56:50 +0200 Subject: [PATCH 12/18] In place mutation using ++= operator --- .../007_map_and_set/010_union_operator.ndct | 2 +- .../021_op_assign_modifies_internal.ndct | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct diff --git a/tests/programs/007_map_and_set/010_union_operator.ndct b/tests/programs/007_map_and_set/010_union_operator.ndct index 917abb3d..f8d26e1d 100644 --- a/tests/programs/007_map_and_set/010_union_operator.ndct +++ b/tests/programs/007_map_and_set/010_union_operator.ndct @@ -2,7 +2,7 @@ // Regular operator assert(%{1,2,3} | %{2,3,4} == %{1,2,3,4}); -// Op assign +// Test that the op assign operator modifies the set let set = %{1,2,3}; set |= %{2,3,4}; assert(set == %{4,3,2,1}); diff --git a/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct b/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct new file mode 100644 index 00000000..7c036f1d --- /dev/null +++ b/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct @@ -0,0 +1,19 @@ +--PROGRAM-- +// This test validates that using the |= operator modifies the variable in place + +let x = %{ + "foo": 1, + "bar": 2, +}; + +let y = x; + +x |= %{"baz": 3}; + +assert(x == y); +assert(x.len == 3); +assert(y.len == 3); + +print("ok") +--EXPECT-- +ok From 458aa099ca6edef512f89dad373fc6af47bfc37c Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Thu, 2 Oct 2025 15:49:08 +0200 Subject: [PATCH 13/18] Wrote special implementations for some of the assignment operators for map --- ndc_lib/src/hash_map.rs | 11 ++-- ndc_lib/src/interpreter/evaluate.rs | 13 +++-- ndc_lib/src/interpreter/sequence.rs | 1 + ndc_lib/src/stdlib/hash_map.rs | 55 ++++++++++++++++++- ndc_lib/src/stdlib/list.rs | 41 ++++++-------- ndc_macros/src/convert.rs | 34 +++++++++++- ndc_macros/src/function.rs | 3 + .../021_op_assign_modifies_internal.ndct | 7 +++ 8 files changed, 126 insertions(+), 39 deletions(-) diff --git a/ndc_lib/src/hash_map.rs b/ndc_lib/src/hash_map.rs index e6673174..84f03969 100644 --- a/ndc_lib/src/hash_map.rs +++ b/ndc_lib/src/hash_map.rs @@ -10,15 +10,14 @@ pub use std::hash::DefaultHasher; use std::hash::Hash; -#[allow(dead_code)] pub trait HashMapExt { - fn union(&mut self, right: HashMap) + fn union(&mut self, right: Self) where K: Hash + Eq; - fn difference(&mut self, right: HashMap) + fn difference(&mut self, right: &Self) where K: Hash + Eq; - fn intersection(&mut self, right: HashMap) + fn intersection(&mut self, right: &Self) where K: Hash + Eq; } @@ -33,14 +32,14 @@ impl HashMapExt for HashMap { } } - fn difference(&mut self, other: Self) + fn difference(&mut self, other: &Self) where K: Hash + Eq, { self.retain(|key, _| !other.contains_key(key)); } - fn intersection(&mut self, other: Self) + fn intersection(&mut self, other: &Self) where K: Hash + Eq, { diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 61da9baf..7d14afab 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -38,7 +38,6 @@ pub(crate) fn evaluate_expression( Expression::BigIntLiteral(n) => Value::Number(Number::Int(Int::BigInt(n.clone()))), Expression::Float64Literal(n) => Value::Number(Number::Float(*n)), Expression::ComplexLiteral(n) => Value::Number(Number::Complex(*n)), - // Expression::Unary { Expression::Grouping(expr) => evaluate_expression(expr, environment)?, Expression::VariableDeclaration { l_value, value } => { let value = evaluate_expression(value, environment)?; @@ -76,8 +75,6 @@ pub(crate) fn evaluate_expression( r_value, operation, } => { - // dbg!(l_value, operation, r_value); - // Identifier may be an operator such as "++" let Expression::Identifier(operation_ident) = &operation.expression else { todo!("OpAssignment operation must be Identifier??"); @@ -86,6 +83,7 @@ pub(crate) fn evaluate_expression( Lvalue::Variable { identifier } => { let rhs = evaluate_expression(r_value, environment)?; let Some(lhs) = environment.borrow_mut().take(identifier) else { + // TODO: this statement does damage which isn't reverted when for instance we can't find the function return Err(EvaluationError::undefined_variable(identifier, span).into()); }; @@ -104,14 +102,19 @@ pub(crate) fn evaluate_expression( // do nothing continue below } err @ Err(_) => return err.into_evaluation_result(span), - Ok(value) => { + Ok(x) if x == Value::unit() => { + // TODO is this check to slow // At the start of this branch we used `take` to temporarily remove the value from the environment (leaving a unit behind) // this helps prevent race conditions in case the operator tries to access its environment but it does require us to put back the LHS - environment.borrow_mut().assign(identifier, value); + let [lhs, _] = arguments; + environment.borrow_mut().assign(identifier, lhs); // Op assignment now returns unit return Ok(Value::unit()); } + Ok(_) => panic!( + "OpAssign implementation is not meant to return a non unit value" + ), } // Execute: `a += b` as `a = a + b` diff --git a/ndc_lib/src/interpreter/sequence.rs b/ndc_lib/src/interpreter/sequence.rs index e829527f..f7637a9a 100644 --- a/ndc_lib/src/interpreter/sequence.rs +++ b/ndc_lib/src/interpreter/sequence.rs @@ -12,6 +12,7 @@ pub type DefaultMap<'a> = (&'a HashMap, Option>); pub type DefaultMapMut<'a> = (&'a mut HashMap, Option>); pub type ListRepr = Rc>>; pub type TupleRepr = Rc>; +pub type MapRepr = Rc>>; #[derive(Clone)] pub enum Sequence { diff --git a/ndc_lib/src/stdlib/hash_map.rs b/ndc_lib/src/stdlib/hash_map.rs index 52c91a03..8cdf5bc1 100644 --- a/ndc_lib/src/stdlib/hash_map.rs +++ b/ndc_lib/src/stdlib/hash_map.rs @@ -1,12 +1,14 @@ use crate::hash_map; use crate::hash_map::HashMap; -use crate::interpreter::sequence::{DefaultMap, Sequence}; +use crate::hash_map::HashMapExt; +use crate::interpreter::sequence::{DefaultMap, MapRepr, Sequence}; use crate::interpreter::value::Value; use std::cell::RefCell; use std::rc::Rc; #[ndc_macros::export_module] mod inner { + /// Returns a list of all the keys in the map or set. /// /// Note that for a set this will return the values in the set. @@ -46,6 +48,57 @@ mod inner { map.insert(key, Value::unit()); } + #[function(name = "&=")] + pub fn intersect_assign(lhs: &mut MapRepr, rhs: &mut MapRepr) { + let left_map: &mut HashMap = &mut lhs + .try_borrow_mut() + .expect("Failed to mutably borrow the lhs of &= operator"); + + left_map.intersection( + &*rhs + .try_borrow() + .expect("Failed borrow the rhs of &= operator"), + ); + } + + #[function(name = "|=")] + pub fn union_assign(lhs: &mut MapRepr, rhs: &mut MapRepr) { + let left_map: &mut HashMap = &mut lhs.borrow_mut(); + + if Rc::strong_count(rhs) == 1 { + // Take ownership + let rhs = std::mem::take(&mut *rhs.borrow_mut()); + left_map.union(rhs); + } else { + let right = rhs.borrow(); + for (key, value) in right.iter() { + left_map.insert(key.clone(), value.clone()); + } + } + } + + #[function(name = "-=")] + pub fn difference_assign(lhs: &mut MapRepr, rhs: &mut MapRepr) { + let left_map: &mut HashMap = &mut lhs.borrow_mut(); + + left_map.difference(&*rhs.borrow()); + } + + #[function(name = "~=")] + pub fn symmetric_difference_assign(lhs: &mut MapRepr, rhs: &mut MapRepr) { + // TODO: is this implementation optimal enough? + let diff = hash_map::symmetric_difference( + &*lhs + .try_borrow() + .expect("Failed to borrow the lhs of ~= operator"), + &*rhs + .try_borrow() + .expect("Failed borrow the rhs of ~= operator"), + ); + + *lhs.borrow_mut() = diff; + } + /// Returns the union (elements that are in either `left` or `right`) of two maps or sets. /// /// This is the same as evaluating the expression `left | right` diff --git a/ndc_lib/src/stdlib/list.rs b/ndc_lib/src/stdlib/list.rs index 318e55db..582847fa 100644 --- a/ndc_lib/src/stdlib/list.rs +++ b/ndc_lib/src/stdlib/list.rs @@ -83,7 +83,7 @@ mod inner { } #[function(name = "++")] - pub fn list_concat(left: ListRepr, right: ListRepr) -> Value { + pub fn list_concat(left: &mut ListRepr, right: &mut ListRepr) -> Value { // TODO: can we/should we optimize here if Rc::strong_count = 1 for the lhs Value::list( left.borrow() @@ -95,32 +95,25 @@ mod inner { } #[function(name = "++=")] - pub fn list_append_operator(left: ListRepr, right: ListRepr) -> Value { - // The ++= operator has 3 implementation paths, this first one is the case where a list extends iteself - if Rc::ptr_eq(&left, &right) { + pub fn list_append_operator(left: &mut ListRepr, right: &mut ListRepr) { + // The ++= operator has 3 implementation paths, this first one is the case where a list extends itself + if Rc::ptr_eq(left, right) { left.borrow_mut().extend_from_within(..); + } else if Rc::strong_count(right) == 1 { + // The second path deals with a RHS that has an RC of one, in this case we can drain the RHS which should be faster than copying? + left.borrow_mut().append( + &mut right + .try_borrow_mut() + .expect("Failed to borrow_mut in `list_append_operator`"), + ); + // The last path is if the RHS has an RC that's higher than 1, in this case we copy all the elements into the LHS } else { - match Rc::try_unwrap(right) { - // The second path deals with a RHS that has an RC of one, in this case we can drain the RHS which should be faster than copying? - Ok(right) => { - left.borrow_mut().append( - &mut right - .try_borrow_mut() - .expect("Failed to borrow_mut in `list_append_operator`"), - ); - } - // The last path is if the RHS has an RC that's higher than 1, in this case we copy all the elements into the LHS - Err(right) => { - left.borrow_mut().extend_from_slice( - &right - .try_borrow() - .expect("Failed to borrow in `list_append_operator`"), - ); - } - } + left.borrow_mut().extend_from_slice( + &right + .try_borrow() + .expect("Failed to borrow in `list_append_operator`"), + ); } - - Value::Sequence(Sequence::List(left)) } /// Copies elements from `other` to `list` not touching `other`. diff --git a/ndc_macros/src/convert.rs b/ndc_macros/src/convert.rs index 3b6a8234..9d1c40b2 100644 --- a/ndc_macros/src/convert.rs +++ b/ndc_macros/src/convert.rs @@ -46,6 +46,33 @@ impl TypeConverter for MutRefString { } } +/// Matches `Rc>>` +struct InternalMap; +impl TypeConverter for InternalMap { + fn matches(&self, ty: &syn::Type) -> bool { + path_ends_with(ty, "MapRepr") + } + + fn convert( + &self, + temp_var: syn::Ident, + original_name: &str, + argument_var_name: syn::Ident, + ) -> Vec { + vec![Argument { + param_type: quote! { crate::interpreter::function::ParamType::Map }, + param_name: quote! { #original_name }, + argument: quote! { #argument_var_name }, + initialize_code: quote! { + let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::Map(#temp_var, _)) = #argument_var_name else { + panic!("Value #position needed to be Sequence::Map but wasn't"); + }; + + let #argument_var_name = #temp_var; + }, + }] + } +} /// Matches `Rc>>` struct InternalList; impl TypeConverter for InternalList { @@ -65,10 +92,10 @@ impl TypeConverter for InternalList { argument: quote! { #argument_var_name }, initialize_code: quote! { let crate::interpreter::value::Value::Sequence(crate::interpreter::sequence::Sequence::List(#temp_var)) = #argument_var_name else { - panic!("blaap"); + panic!("Value #position needed to be Sequence::List but wasn't"); }; - let #argument_var_name = std::mem::take(#temp_var); + let #argument_var_name = #temp_var; }, }] } @@ -96,7 +123,7 @@ impl TypeConverter for InternalTuple { panic!("blaap"); }; - let #argument_var_name = std::mem::take(#temp_var); + let #argument_var_name = std::mem::take(#temp_var); // TODO: is std::mem::take appropriate here? }, }] } @@ -107,5 +134,6 @@ pub fn build() -> Vec> { Box::new(InternalList), Box::new(MutRefString), Box::new(InternalTuple), + Box::new(InternalMap), ] } diff --git a/ndc_macros/src/function.rs b/ndc_macros/src/function.rs index 654698df..69a5db65 100644 --- a/ndc_macros/src/function.rs +++ b/ndc_macros/src/function.rs @@ -248,6 +248,9 @@ fn into_param_type(ty: &syn::Type) -> TokenStream { ty if path_ends_with(ty, "TupleRepr") => { quote! { crate::interpreter::function::ParamType::Tuple } } + ty if path_ends_with(ty, "MapRepr") => { + quote! { crate::interpreter::function::ParamType::Map } + } syn::Type::Reference(syn::TypeReference { elem, .. }) => into_param_type(elem), syn::Type::Path(syn::TypePath { path, .. }) => match path { _ if path.is_ident("i64") => quote! { crate::interpreter::function::ParamType::Int }, diff --git a/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct b/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct index 7c036f1d..c5448272 100644 --- a/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct +++ b/tests/programs/007_map_and_set/021_op_assign_modifies_internal.ndct @@ -14,6 +14,13 @@ assert(x == y); assert(x.len == 3); assert(y.len == 3); +let z = x; + +z &= %{"bar": 10}; + +assert(x == y and y == z); +assert(x.len == 1 and y.len == 1 and z.len == 1); + print("ok") --EXPECT-- ok From 8628b2ff5b5faa0a8c6acd52c4d851fa6298ac4a Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 18 Nov 2025 13:39:09 +0100 Subject: [PATCH 14/18] implement vectorization (but at what cost) --- Cargo.lock | 1 - Cargo.toml | 1 - ndc_lib/Cargo.toml | 1 - ndc_lib/src/ast/expression.rs | 3 +- ndc_lib/src/ast/parser.rs | 1 - ndc_lib/src/interpreter/evaluate.rs | 54 +-------- ndc_lib/src/interpreter/function.rs | 112 ++++++++++++------ ndc_lib/src/stdlib/math.rs | 2 +- ndc_lib/src/stdlib/string.rs | 1 - ndc_macros/src/convert.rs | 4 +- ndc_macros/src/function.rs | 2 +- .../013_vector_math/002_vector_error.ndct | 2 +- .../013_vector_math/003_vector_error2.ndct | 2 +- .../999_cursed/comical_factorial.ndct | 8 +- 14 files changed, 94 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a3d9ff7c..98f5513e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -881,7 +881,6 @@ dependencies = [ "anyhow", "derive_builder", "derive_more", - "either", "factorial", "itertools 0.14.0", "md5", diff --git a/Cargo.toml b/Cargo.toml index 01066a83..139d39b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,6 @@ clap = { version = "4.5.39", features = ["derive"] } criterion = { version = "0.6.0", features = ["html_reports"] } derive_more = { version = "2.0.1", features = ["deref", "deref_mut", "from", "constructor"] } derive_builder = { version = "0.20.2" } -either = "1.15.0" strsim = "0.11.1" factorial = "0.4.0" itertools = "0.14.0" diff --git a/ndc_lib/Cargo.toml b/ndc_lib/Cargo.toml index 1e0f5b0f..06fa29a5 100644 --- a/ndc_lib/Cargo.toml +++ b/ndc_lib/Cargo.toml @@ -9,7 +9,6 @@ ahash = { workspace = true, optional = true } anyhow.workspace = true derive_more.workspace = true derive_builder.workspace = true -either.workspace = true factorial.workspace = true itertools.workspace = true miette.workspace = true diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index c5222875..27fe527a 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -1,8 +1,7 @@ -use crate::ast::operator::{BinaryOperator, LogicalOperator}; +use crate::ast::operator::LogicalOperator; use crate::ast::parser::Error as ParseError; use crate::interpreter::evaluate::EvaluationError; use crate::lexer::Span; -use either::Either; use num::BigInt; use num::complex::Complex64; use std::fmt; diff --git a/ndc_lib/src/ast/parser.rs b/ndc_lib/src/ast/parser.rs index c0e1b1fe..551e8ff2 100644 --- a/ndc_lib/src/ast/parser.rs +++ b/ndc_lib/src/ast/parser.rs @@ -1,7 +1,6 @@ use std::fmt::Write; use std::rc::Rc; -use either::Either; use miette::Diagnostic; use crate::ast::Expression; diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 7d14afab..d2cdd681 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use std::cell::RefCell; use std::cmp::Ordering; use std::fmt; -use std::ops::{Add, BitAnd, BitOr, BitXor, Div, IndexMut, Mul, Rem, Sub}; +use std::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Sub}; use std::rc::Rc; use crate::ast::{ @@ -12,7 +12,7 @@ use crate::ast::{ use crate::hash_map; use crate::hash_map::HashMap; use crate::interpreter::environment::{Environment, EnvironmentRef}; -use crate::interpreter::evaluate::index::{EvaluatedIndex, get_at_index}; +use crate::interpreter::evaluate::index::get_at_index; use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction}; use crate::interpreter::int::Int; use crate::interpreter::iterator::mut_value_to_iterator; @@ -603,55 +603,6 @@ pub(crate) fn evaluate_expression( Ok(literal) } -fn take_lvalue( - lvalue: &Lvalue, - lvalue_span: Span, - environment: &mut EnvironmentRef, -) -> EvaluationResult { - match lvalue { - Lvalue::Variable { identifier } => { - let value = environment - .borrow() - .take(identifier) - .ok_or_else(|| EvaluationError::undefined_variable(identifier, lvalue_span))?; - - Ok(value) - } - Lvalue::Index { value, index } => { - let v = evaluate_expression(value, environment)?; - let i = evaluate_as_index(index, environment)?; - let result = index_into_value(v, i, lvalue_span)?; - Ok(result) - } - Lvalue::Sequence(_) => todo!("sequencing not implemented"), - } -} - -fn index_into_value(value: Value, index: EvaluatedIndex, lvalue_span: Span) -> EvaluationResult { - match value { - Value::Sequence(Sequence::List(list)) => { - let size = list.try_borrow().into_evaluation_result(lvalue_span)?.len(); - let index = index.try_into_offset(size, lvalue_span)?; - - let mut list = list.try_borrow_mut().into_evaluation_result(lvalue_span)?; - match index { - Offset::Element(index) => { - let list_item = std::mem::replace(list.index_mut(index), Value::unit()); - Ok(list_item) - } - Offset::Range(_, _) => todo!("range expression not implemented here"), - } - } - Value::Sequence(Sequence::Map(map, def)) => todo!(), - Value::Sequence(Sequence::String(string)) => todo!(), - _ => Err(EvaluationError::syntax_error( - format!("cannot OpAssign an index into a {}", value.value_type()), - lvalue_span, - ) - .into()), - } -} - fn produce_default_value( default: &Value, environment: &EnvironmentRef, @@ -1156,6 +1107,7 @@ fn call_function_by_name( ) -> EvaluationResult { let values = environment.borrow().get_all_by_name(name); let result = try_call_function_from_values(&values, evaluated_args, environment, span); + if let Err(FunctionCarrier::FunctionNotFound) = result { let arguments = evaluated_args.iter().map(Value::value_type).join(", "); return Err(FunctionCarrier::EvaluationError(EvaluationError::new( diff --git a/ndc_lib/src/interpreter/function.rs b/ndc_lib/src/interpreter/function.rs index 29c7a2d5..86e21288 100644 --- a/ndc_lib/src/interpreter/function.rs +++ b/ndc_lib/src/interpreter/function.rs @@ -1,4 +1,4 @@ -use crate::ast::ExpressionLocation; +use crate::ast::{BinaryOperator, ExpressionLocation}; use crate::hash_map::{DefaultHasher, HashMap}; use crate::interpreter::environment::{Environment, EnvironmentRef}; use crate::interpreter::evaluate::{ @@ -103,9 +103,6 @@ impl FunctionBody { function, } } -} - -impl FunctionBody { fn type_signature(&self) -> TypeSignature { match self { Self::Closure { @@ -215,6 +212,30 @@ pub enum TypeSignature { Exact(Vec), } +impl TypeSignature { + /// Matches a list of `ValueTypes` to a type signature. It can return `None` if there is no match or + /// `Some(num)` where num is the sum of the distances of the types. The type `Int`, is distance 1 + /// away from `Number`, and `Number` is 1 distance from `Any`, then `Int` is distance 2 from `Any`. + fn calc_type_score(&self, types: &[ValueType]) -> Option { + match self { + Self::Variadic => Some(0), + Self::Exact(signature) => { + if types.len() == signature.len() { + let mut acc = 0; + for (a, b) in types.iter().zip(signature.iter()) { + let dist = b.type_name.distance(a)?; + acc += dist; + } + + return Some(acc); + } + + None + } + } + } +} + #[derive(Clone)] pub struct OverloadedFunction { implementations: HashMap, @@ -242,23 +263,70 @@ impl OverloadedFunction { pub fn call(&self, args: &mut [Value], env: &EnvironmentRef) -> EvaluationResult { let types: Vec = args.iter().map(ValueType::from).collect(); - let mut best = None; + // TODO: handle vectorization here??! + + let mut best_function_match = None; let mut best_distance = u32::MAX; for (signature, function) in &self.implementations { - let Some(cur) = match_types_to_signature(&types, signature) else { + let Some(cur) = signature.calc_type_score(&types) else { continue; }; if cur < best_distance { best_distance = cur; - best = Some(function); + best_function_match = Some(function); } } - if let Some(function) = best { - function.body().call(args, env) - } else { - Err(FunctionCarrier::FunctionNotFound) + best_function_match + .ok_or(FunctionCarrier::FunctionNotFound) + .and_then(|function| function.body().call(args, env)) + .or_else(|err| { + if let FunctionCarrier::FunctionNotFound = err { + self.call_vectorized(args, env) + } else { + Err(err) + } + }) // TODO: we dont' want to just always call this but let's figure out when later + } + + fn call_vectorized(&self, args: &mut [Value], env: &EnvironmentRef) -> EvaluationResult { + let [left, right] = args else { + // Vectorized application only works in cases where there are two tuple arguments + return Err(FunctionCarrier::FunctionNotFound); + }; + + if !left.supports_vectorization_with(right) { + return Err(FunctionCarrier::FunctionNotFound); } + + let (left, right) = match (left, right) { + // Both are tuples + (Value::Sequence(Sequence::Tuple(left)), Value::Sequence(Sequence::Tuple(right))) => { + (left, right.as_slice()) + } + // Left is a number and right is a tuple + (left @ Value::Number(_), Value::Sequence(Sequence::Tuple(right))) => ( + &mut Rc::new(vec![left.clone(); right.len()]), + right.as_slice(), + ), + // Left is a tuple and right is a number + (Value::Sequence(Sequence::Tuple(left)), right @ Value::Number(_)) => { + (left, std::slice::from_ref(right)) + } + _ => { + return Err(FunctionCarrier::FunctionNotFound); + } + }; + + let left_mut: &mut Vec = Rc::make_mut(left); + + // Zip the mutable vector with the immutable right side and perform the operations on all elements + for (l, r) in left_mut.iter_mut().zip(right.iter().cycle()) { + // TODO: Fuck this cloning business + *l = self.call(&mut [l.clone(), r.clone()], env)?; + } + + Ok(Value::Sequence(Sequence::Tuple(left.clone()))) // TODO PLS NO CLONE } } @@ -277,28 +345,6 @@ impl From for OverloadedFunction { } } -/// Matches a list of `ValueTypes` to a type signature. It can return `None` if there is no match or -/// `Some(num)` where num is the sum of the distances of the types. The type `Int`, is distance 1 -/// away from `Number`, and `Number` is 1 distance from `Any`, then `Int` is distance 2 from `Any`. -fn match_types_to_signature(types: &[ValueType], signature: &TypeSignature) -> Option { - match signature { - TypeSignature::Variadic => Some(0), - TypeSignature::Exact(signature) => { - if types.len() == signature.len() { - let mut acc = 0; - for (a, b) in types.iter().zip(signature.iter()) { - let dist = b.type_name.distance(a)?; - acc += dist; - } - - return Some(acc); - } - - None - } - } -} - #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct Parameter { pub name: String, diff --git a/ndc_lib/src/stdlib/math.rs b/ndc_lib/src/stdlib/math.rs index fcf55e6c..27b1df54 100644 --- a/ndc_lib/src/stdlib/math.rs +++ b/ndc_lib/src/stdlib/math.rs @@ -37,7 +37,7 @@ where T: std::borrow::Borrow, { fn try_product(&mut self) -> Result { - self.try_fold(Number::from(0), |acc, cur| match cur.borrow() { + self.try_fold(Number::from(1), |acc, cur| match cur.borrow() { Value::Number(n) => acc.mul(n), value => Err(BinaryOperatorError::new(format!( "cannot multiply {} and number", diff --git a/ndc_lib/src/stdlib/string.rs b/ndc_lib/src/stdlib/string.rs index 65fc1ca6..6a2f9563 100644 --- a/ndc_lib/src/stdlib/string.rs +++ b/ndc_lib/src/stdlib/string.rs @@ -6,7 +6,6 @@ use crate::interpreter::value::Value; use anyhow::{Context, anyhow}; use std::fmt::Write; -use std::rc::Rc; pub fn join_to_string(list: &mut Sequence, sep: &str) -> anyhow::Result { let mut iter = mut_seq_to_iterator(list); diff --git a/ndc_macros/src/convert.rs b/ndc_macros/src/convert.rs index 9d1c40b2..672d0f35 100644 --- a/ndc_macros/src/convert.rs +++ b/ndc_macros/src/convert.rs @@ -1,6 +1,6 @@ use crate::r#match::{is_ref_mut, is_string, path_ends_with}; -use proc_macro2::{Span, TokenStream}; -use quote::{ToTokens, quote}; +use proc_macro2::TokenStream; +use quote::quote; #[derive(Debug, Clone)] pub struct Argument { diff --git a/ndc_macros/src/function.rs b/ndc_macros/src/function.rs index 69a5db65..8b741118 100644 --- a/ndc_macros/src/function.rs +++ b/ndc_macros/src/function.rs @@ -1,7 +1,7 @@ use crate::convert::{Argument, TypeConverter, build}; use crate::r#match::{ is_ref, is_ref_mut, is_ref_mut_of_slice_of_value, is_ref_of_bigint, is_ref_of_slice_of_value, - is_str_ref, is_string, path_ends_with, + is_str_ref, path_ends_with, }; use itertools::Itertools; use proc_macro2::TokenStream; diff --git a/tests/programs/013_vector_math/002_vector_error.ndct b/tests/programs/013_vector_math/002_vector_error.ndct index 23cbc0ea..c6c6adce 100644 --- a/tests/programs/013_vector_math/002_vector_error.ndct +++ b/tests/programs/013_vector_math/002_vector_error.ndct @@ -1,4 +1,4 @@ --PROGRAM-- (1,2) + (5,3,2) --EXPECT-ERROR-- -+ is not defined for \ No newline at end of file +no function called '+' found matches the arguments diff --git a/tests/programs/013_vector_math/003_vector_error2.ndct b/tests/programs/013_vector_math/003_vector_error2.ndct index f2f80c09..ad3fa2de 100644 --- a/tests/programs/013_vector_math/003_vector_error2.ndct +++ b/tests/programs/013_vector_math/003_vector_error2.ndct @@ -2,4 +2,4 @@ // This looks valid, but isn't (1,1,(1,)) + (1,1,(1,)) --EXPECT-ERROR-- -+ is not defined for \ No newline at end of file +no function called '+' found matches the arguments diff --git a/tests/programs/999_cursed/comical_factorial.ndct b/tests/programs/999_cursed/comical_factorial.ndct index 626f44b9..3d479d9f 100644 --- a/tests/programs/999_cursed/comical_factorial.ndct +++ b/tests/programs/999_cursed/comical_factorial.ndct @@ -1,8 +1,10 @@ --PROGRAM-- let v, n = 100, 100; -while { n -= 1 } > 0 { +while n > 1 { + n -= 1; v *= n; } -print(v); +assert_eq(v, 100.factorial); +print("ok"); --EXPECT-- -93326215443944152681699238856266700490715968264381621468592963895217599993229915608941463976156518286253697920827223758251185210916864000000000000000000000000 \ No newline at end of file +ok \ No newline at end of file From 77b8a6f461b2fb64bd1182dd76a20df9f7766b70 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 18 Nov 2025 14:47:51 +0100 Subject: [PATCH 15/18] remove dead code --- ndc_lib/src/interpreter/evaluate.rs | 262 ---------------------------- 1 file changed, 262 deletions(-) diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index d2cdd681..d63e7d63 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -691,268 +691,6 @@ fn declare_or_assign_variable( Ok(Value::unit()) } -#[allow(clippy::too_many_lines)] -fn apply_operator( - left: Value, - operator: BinaryOperator, - right: Value, -) -> Result { - let (left_type, right_type) = (left.value_type(), right.value_type()); - let create_type_error = - || BinaryOperatorError::undefined_operation(operator, left_type, right_type); - - if left.supports_vectorization_with(&right) && operator.supports_vectorization() { - return apply_operation_vectorized(left, &right, operator); - } - - let val: Value = match operator { - BinaryOperator::Equality => left.eq(&right).into(), - BinaryOperator::Inequality => left.ne(&right).into(), - BinaryOperator::Greater => { - (left.partial_cmp(&right).ok_or_else(create_type_error)? == Ordering::Greater).into() - } - BinaryOperator::GreaterEquals => { - (left.partial_cmp(&right).ok_or_else(create_type_error)? != Ordering::Less).into() - } - BinaryOperator::Less => { - (left.partial_cmp(&right).ok_or_else(create_type_error)? == Ordering::Less).into() - } - BinaryOperator::LessEquals => { - (left.partial_cmp(&right).ok_or_else(create_type_error)? != Ordering::Greater).into() - } - BinaryOperator::Spaceship => { - match left.partial_cmp(&right).ok_or_else(create_type_error)? { - Ordering::Less => Value::from(-1), - Ordering::Equal => Value::from(0), - Ordering::Greater => Value::from(1), - } - } - BinaryOperator::InverseSpaceship => { - match left.partial_cmp(&right).ok_or_else(create_type_error)? { - Ordering::Less => Value::from(1), - Ordering::Equal => Value::from(0), - Ordering::Greater => Value::from(-1), - } - } - BinaryOperator::Plus => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.add(b)?), - _ => return Err(create_type_error()), - }, - BinaryOperator::Minus => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.sub(b)?), - _ => return Err(create_type_error()), - }, - BinaryOperator::Multiply => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.mul(b)?), - - _ => return Err(create_type_error()), - }, - BinaryOperator::Divide => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.div(b)?), - _ => return Err(create_type_error()), - }, - BinaryOperator::FloorDivide => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.floor_div(b)?), - - _ => return Err(create_type_error()), - }, - BinaryOperator::CModulo => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.rem(b)?), - - _ => return Err(create_type_error()), - }, - BinaryOperator::EuclideanModulo => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.checked_rem_euclid(b)?), - - _ => return Err(create_type_error()), - }, - BinaryOperator::Exponent => match (left, right) { - (Value::Number(a), Value::Number(b)) => Value::Number(a.pow(b)?), - - ( - left @ Value::Sequence(Sequence::Tuple(_)), - right @ Value::Sequence(Sequence::Tuple(_)), - ) => apply_operation_vectorized(left, &right, BinaryOperator::Exponent)?, - _ => return Err(create_type_error()), - }, - BinaryOperator::And => match (left, right) { - (Value::Bool(a), Value::Bool(b)) => a.bitand(b).into(), - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::from(Number::from(a & b)) - } - // Note that when using & on two dictionaries with a default values the default value from left is kept - ( - Value::Sequence(Sequence::Map(left, default)), - Value::Sequence(Sequence::Map(right, _)), - ) => Value::Sequence(Sequence::Map( - Rc::new(RefCell::new(hash_map::intersection( - &*left.borrow(), - &*right.borrow(), - ))), - default, - )), - _ => return Err(create_type_error()), - }, - BinaryOperator::Or => match (left, right) { - (Value::Bool(a), Value::Bool(b)) => a.bitor(b).into(), - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::from(Number::from(a | b)) - } - ( - Value::Sequence(Sequence::Map(left, default)), - Value::Sequence(Sequence::Map(right, _)), - ) => Value::Sequence(Sequence::Map( - Rc::new(RefCell::new(hash_map::union( - &*left.borrow(), - &*right.borrow(), - ))), - default, - )), - _ => return Err(create_type_error()), - }, - BinaryOperator::Xor => match (left, right) { - (Value::Bool(a), Value::Bool(b)) => a.bitxor(b).into(), - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::from(Number::from(a ^ b)) - } - ( - Value::Sequence(Sequence::Map(left, default)), - Value::Sequence(Sequence::Map(right, _)), - ) => Value::Sequence(Sequence::Map( - Rc::new(RefCell::new(hash_map::symmetric_difference( - &*left.borrow(), - &*right.borrow(), - ))), - default, - )), - _ => return Err(create_type_error()), - }, - BinaryOperator::ShiftRight => match (left, right) { - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => Value::Number( - Number::Int(a.checked_shr(b).ok_or(BinaryOperatorError::new(format!( - // TODO: check this before merge - "operator {operator} failed because one of its operands is invalid" - )))?), - ), - _ => return Err(create_type_error()), - }, - BinaryOperator::ShiftLeft => match (left, right) { - (Value::Number(Number::Int(a)), Value::Number(Number::Int(b))) => { - Value::Number(Number::Int(a.checked_shl(b).ok_or( - BinaryOperatorError::new(format!( - // TODO: check this before merge - "operator {operator} failed because one of its operands is invalid" - )), - )?)) - } - _ => return Err(create_type_error()), - }, - BinaryOperator::In => match (left, right) { - (needle, Value::Sequence(haystack)) => Value::Bool(haystack.contains(&needle)), - _ => return Err(create_type_error()), - }, - BinaryOperator::Concat => match (left, right) { - (Value::Sequence(Sequence::String(left)), Value::Sequence(Sequence::String(right))) => { - let mut new_string = left.borrow().clone(); - new_string.push_str(&right.borrow()); - Value::from(new_string) - } - (Value::Sequence(Sequence::List(left)), Value::Sequence(Sequence::List(right))) => { - // TODO: NOTE: this first branch is an experiment, I'm not sure if it'll always work correctly - // If we can take ownership of right which is possible if it's used as a literal expression - // we borrow right as mutable and copy the elements over to the new list which I guess is maybe faster? - // Case where this might work is the following: `[1] ++ [2]` - // Case where this might not work is: `l := [1]; [1] ++ l` - match Rc::try_unwrap(right) { - Ok(right) => { - // TODO: if there is no benefit to this branch we might as wel remove it - let mut new_list = left.borrow().clone(); - new_list.append(&mut right.borrow_mut()); - Value::list(new_list) - } - Err(right) => Value::list( - left.borrow() - .iter() - .chain(right.borrow().iter()) - .cloned() - .collect::>(), - ), - } - } - ( - Value::Sequence(Sequence::Tuple(mut left)), - Value::Sequence(Sequence::Tuple(mut right)), - ) => { - let left_mut = Rc::make_mut(&mut left); - let right_mut = Rc::make_mut(&mut right); - left_mut.append(right_mut); - - Value::Sequence(Sequence::Tuple(left)) - } - _ => return Err(create_type_error()), - }, - BinaryOperator::StringConcat => Value::from(format!("{left}{right}")), - }; - - Ok(val) -} - -fn apply_operation_vectorized( - left: Value, - right: &Value, - operator: BinaryOperator, -) -> Result { - let (left_type, right_type) = (left.value_type(), right.value_type()); - - let (mut left, right) = match (left, right) { - (Value::Sequence(Sequence::Tuple(left)), Value::Sequence(Sequence::Tuple(right))) => { - (left, right.as_slice()) - } - (left @ Value::Number(_), Value::Sequence(Sequence::Tuple(right))) => { - (Rc::new(vec![left; right.len()]), right.as_slice()) - } - (Value::Sequence(Sequence::Tuple(left)), right @ Value::Number(_)) => { - (left, std::slice::from_ref(right)) - } - _ => { - return Err(BinaryOperatorError::undefined_operation( - operator, left_type, right_type, - )); - } - }; - - let left_mut: &mut Vec = Rc::make_mut(&mut left); - - // Zip the mutable vector with the immutable right side and perform the operations on all elements - for (l, r) in left_mut.iter_mut().zip(right.iter().cycle()) { - if let (Value::Number(l), Value::Number(r)) = (l, r) { - // TODO: can we use AddAssign etc? - *l = match operator { - BinaryOperator::Plus => Number::add(l.clone(), r)?, - BinaryOperator::Minus => Number::sub(l.clone(), r)?, - BinaryOperator::Multiply => Number::mul(l.clone(), r)?, - BinaryOperator::Divide => Number::div(l.clone(), r)?, - BinaryOperator::FloorDivide => Number::floor_div(l.clone(), r.clone())?, // Get rid of r.clone? - BinaryOperator::CModulo => Number::rem(l.clone(), r)?, - BinaryOperator::EuclideanModulo => { - Number::checked_rem_euclid(l.clone(), r.clone())? // Get rid of r.clone? - } - BinaryOperator::Exponent => Number::pow(l.clone(), r.clone())?, // Get rid of r.clone? - _ => { - return Err(BinaryOperatorError::undefined_operation( - operator, left_type, right_type, - )); - } - }; - } else { - // We could also consider removing this case, but it's probably better to expose this - unreachable!("this case should already be covered by the type checker above") - } - } - - Ok(Value::Sequence(Sequence::Tuple(left))) -} - #[derive(thiserror::Error, miette::Diagnostic, Debug)] #[error("{text}")] pub struct EvaluationError { From fca051e3824581a261372bc75d2995ec6dfaab6a Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 18 Nov 2025 14:49:04 +0100 Subject: [PATCH 16/18] remove unused imports --- ndc_lib/src/interpreter/evaluate.rs | 9 ++------- ndc_lib/src/interpreter/function.rs | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index d63e7d63..683d572c 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -1,22 +1,17 @@ use index::{Offset, evaluate_as_index, set_at_index}; use itertools::Itertools; use std::cell::RefCell; -use std::cmp::Ordering; use std::fmt; -use std::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Sub}; use std::rc::Rc; -use crate::ast::{ - BinaryOperator, Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue, -}; -use crate::hash_map; +use crate::ast::{Expression, ExpressionLocation, ForBody, ForIteration, LogicalOperator, Lvalue}; use crate::hash_map::HashMap; use crate::interpreter::environment::{Environment, EnvironmentRef}; use crate::interpreter::evaluate::index::get_at_index; use crate::interpreter::function::{Function, FunctionBody, FunctionCarrier, OverloadedFunction}; use crate::interpreter::int::Int; use crate::interpreter::iterator::mut_value_to_iterator; -use crate::interpreter::num::{BinaryOperatorError, Number}; +use crate::interpreter::num::Number; use crate::interpreter::sequence::Sequence; use crate::interpreter::value::{Value, ValueType}; use crate::lexer::Span; diff --git a/ndc_lib/src/interpreter/function.rs b/ndc_lib/src/interpreter/function.rs index 86e21288..01a18334 100644 --- a/ndc_lib/src/interpreter/function.rs +++ b/ndc_lib/src/interpreter/function.rs @@ -1,4 +1,4 @@ -use crate::ast::{BinaryOperator, ExpressionLocation}; +use crate::ast::ExpressionLocation; use crate::hash_map::{DefaultHasher, HashMap}; use crate::interpreter::environment::{Environment, EnvironmentRef}; use crate::interpreter::evaluate::{ From 0729421a3c183c71c989a685217fb0e10af27123 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 19 Nov 2025 14:39:47 +0100 Subject: [PATCH 17/18] Make the operation in an OpAssignment expression always String --- ndc_lib/src/ast/expression.rs | 3 +-- ndc_lib/src/ast/parser.rs | 10 ++-------- ndc_lib/src/interpreter/evaluate.rs | 6 +----- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/ndc_lib/src/ast/expression.rs b/ndc_lib/src/ast/expression.rs index 27fe527a..0c0fcbe8 100644 --- a/ndc_lib/src/ast/expression.rs +++ b/ndc_lib/src/ast/expression.rs @@ -42,8 +42,7 @@ pub enum Expression { OpAssignment { l_value: Lvalue, r_value: Box, - // TODO: Should always be an identifier? - operation: Box, + operation: String, }, FunctionDeclaration { name: Option>, diff --git a/ndc_lib/src/ast/parser.rs b/ndc_lib/src/ast/parser.rs index 551e8ff2..94138b3c 100644 --- a/ndc_lib/src/ast/parser.rs +++ b/ndc_lib/src/ast/parser.rs @@ -367,13 +367,7 @@ impl Parser { Ok(assignment_expression.to_location(start.merge(end))) } Some(Token::OpAssign(inner)) => { - // let thing = match &inner.token { - // Token::Identifier(identifier) => Either::Right(identifier.to_string()), - // _ => Either::Left((*inner.clone()).try_into()?), - // }; - - let operation = - Expression::Identifier(inner.token.to_string()).to_location(inner.span); + let operation_identifier = inner.token.to_string(); self.advance(); let expression = self.tuple_expression(Self::single_expression, false)?; @@ -382,7 +376,7 @@ impl Parser { l_value: Lvalue::try_from(maybe_lvalue) .expect("guaranteed to produce an lvalue"), r_value: Box::new(expression), - operation: Box::new(operation), + operation: operation_identifier, }; Ok(op_assign.to_location(start.merge(end))) diff --git a/ndc_lib/src/interpreter/evaluate.rs b/ndc_lib/src/interpreter/evaluate.rs index 683d572c..f94788aa 100644 --- a/ndc_lib/src/interpreter/evaluate.rs +++ b/ndc_lib/src/interpreter/evaluate.rs @@ -68,12 +68,8 @@ pub(crate) fn evaluate_expression( Expression::OpAssignment { l_value, r_value, - operation, + operation: operation_ident, } => { - // Identifier may be an operator such as "++" - let Expression::Identifier(operation_ident) = &operation.expression else { - todo!("OpAssignment operation must be Identifier??"); - }; match l_value { Lvalue::Variable { identifier } => { let rhs = evaluate_expression(r_value, environment)?; From 6193730f854a8868cc4be5e545c697be8a0d7d90 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Wed, 19 Nov 2025 16:31:35 +0100 Subject: [PATCH 18/18] Minor cleanup --- ndc_lib/src/interpreter/function.rs | 42 +++++++---------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/ndc_lib/src/interpreter/function.rs b/ndc_lib/src/interpreter/function.rs index 01a18334..90db1718 100644 --- a/ndc_lib/src/interpreter/function.rs +++ b/ndc_lib/src/interpreter/function.rs @@ -160,12 +160,10 @@ impl FunctionBody { .into()), }, Self::NumericBinaryOp { body } => match args { - [Value::Number(left), Value::Number(right)] => { - // TODO: we could use references here? - Ok(Value::Number(body(left.clone(), right.clone()).map_err( - |err| FunctionCarrier::IntoEvaluationError(Box::new(err)), - )?)) - } + [Value::Number(left), Value::Number(right)] => Ok(Value::Number( + body(left.clone(), right.clone()) + .map_err(|err| FunctionCarrier::IntoEvaluationError(Box::new(err)))?, + )), [Value::Number(_), right] => Err(FunctionCallError::ArgumentTypeError { expected: ParamType::Number, actual: right.value_type(), @@ -263,10 +261,9 @@ impl OverloadedFunction { pub fn call(&self, args: &mut [Value], env: &EnvironmentRef) -> EvaluationResult { let types: Vec = args.iter().map(ValueType::from).collect(); - // TODO: handle vectorization here??! - let mut best_function_match = None; let mut best_distance = u32::MAX; + for (signature, function) in &self.implementations { let Some(cur) = signature.calc_type_score(&types) else { continue; @@ -280,13 +277,14 @@ impl OverloadedFunction { best_function_match .ok_or(FunctionCarrier::FunctionNotFound) .and_then(|function| function.body().call(args, env)) + // For now if we can't find a specific function we just try to vectorize as a fallback .or_else(|err| { if let FunctionCarrier::FunctionNotFound = err { self.call_vectorized(args, env) } else { Err(err) } - }) // TODO: we dont' want to just always call this but let's figure out when later + }) } fn call_vectorized(&self, args: &mut [Value], env: &EnvironmentRef) -> EvaluationResult { @@ -321,12 +319,12 @@ impl OverloadedFunction { let left_mut: &mut Vec = Rc::make_mut(left); // Zip the mutable vector with the immutable right side and perform the operations on all elements + // TODO: maybe one day figure out how to get rid of all these clones for (l, r) in left_mut.iter_mut().zip(right.iter().cycle()) { - // TODO: Fuck this cloning business *l = self.call(&mut [l.clone(), r.clone()], env)?; } - Ok(Value::Sequence(Sequence::Tuple(left.clone()))) // TODO PLS NO CLONE + Ok(Value::Sequence(Sequence::Tuple(left.clone()))) } } @@ -472,28 +470,8 @@ impl From<&Value> for ParamType { } impl fmt::Display for ParamType { - // TODO replace with strum? fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Any => write!(f, "Any"), - Self::Bool => write!(f, "Bool"), - Self::Function => write!(f, "Function"), - Self::Option => write!(f, "Option"), - Self::Number => write!(f, "Number"), - Self::Float => write!(f, "Float"), - Self::Int => write!(f, "Int"), - Self::Rational => write!(f, "Rational"), - Self::Complex => write!(f, "Complex"), - Self::Sequence => write!(f, "Sequence"), - Self::List => write!(f, "List"), - Self::String => write!(f, "String"), - Self::Tuple => write!(f, "Tuple"), - Self::Map => write!(f, "Map"), - Self::Iterator => write!(f, "Iterator"), - Self::MinHeap => write!(f, "MinHeap"), - Self::MaxHeap => write!(f, "MaxHeap"), - Self::Deque => write!(f, "Deque"), - } + f.write_str(self.as_str()) } }