diff --git a/src/protocol/ast.rs b/src/protocol/ast.rs index 71ea38e2e1768e374b9d1fb2b5726386ca1482ab..1fb57928f4d4c536feecb74413f991c0b204f675 100644 --- a/src/protocol/ast.rs +++ b/src/protocol/ast.rs @@ -637,6 +637,8 @@ pub struct FieldSymbolic { // Phase 1: Parser pub(crate) identifier: Identifier, // Phase 3: Typing + // TODO: @Monomorph These fields cannot be trusted because the last time it + // was typed it may refer to a different monomorph. pub(crate) definition: Option, pub(crate) field_idx: usize, } diff --git a/src/protocol/eval/executor.rs b/src/protocol/eval/executor.rs index 9834caa60c2f4e8b6362040612f79769e78e4dee..239d890bff7cb47c2f6000277c2b39f825e91fb5 100644 --- a/src/protocol/eval/executor.rs +++ b/src/protocol/eval/executor.rs @@ -229,15 +229,14 @@ impl Prompt { let expr = &heap[expr_id]; match expr { Expression::Assignment(expr) => { - // TODO: Stuff goes wrong here, either make these - // utilities do the alloc/dealloc, or let it all be - // done here. let to = cur_frame.expr_values.pop_back().unwrap().as_ref(); let rhs = cur_frame.expr_values.pop_back().unwrap(); - // let rhs_heap_pos = rhs.get_heap_pos(); + + // Note: although not pretty, the assignment operator takes ownership + // of the right-hand side value when possible. So we do not drop the + // rhs's optionally owned heap data. + let rhs = self.store.read_take_ownership(rhs); apply_assignment_operator(&mut self.store, to, expr.operation, rhs); - cur_frame.expr_values.push_back(self.store.read_copy(to)); - // self.store.drop_value(rhs_heap_pos); }, Expression::Binding(_expr) => { todo!("Binding expression"); @@ -280,23 +279,29 @@ impl Prompt { index.as_unsigned_integer() as u32 }; - // TODO: This is probably wrong, we're dropping the - // heap while refering to an element... let subject = cur_frame.expr_values.pop_back().unwrap(); - let subject_heap_pos = subject.get_heap_pos(); - let heap_pos = match subject { + let (deallocate_heap_pos, value_to_push) = match subject { Value::Ref(value_ref) => { - println!("DEBUG: Called with {:?}", subject); - let result = self.store.read_ref(value_ref); - println!("DEBUG: And got {:?}", result); - result.as_array() + // Our expression stack value is a reference to something that + // exists in the normal stack/heap. We don't want to deallocate + // this thing. Rather we want to return a reference to it. + let subject = self.store.read_ref(value_ref); + let subject_heap_pos = subject.as_array(); + + (None, Value::Ref(ValueId::Heap(subject_heap_pos, index))) + }, + _ => { + // Our value lives on the expression stack, hence we need to + // clone whatever we're referring to. Then drop the subject. + let subject_heap_pos = subject.as_array(); + let subject_indexed = Value::Ref(ValueId::Heap(subject_heap_pos, index)); + (Some(subject_heap_pos), self.store.clone_value(subject_indexed)) }, - val => val.as_array(), }; - cur_frame.expr_values.push_back(Value::Ref(ValueId::Heap(heap_pos, index))); - self.store.drop_value(subject_heap_pos); + cur_frame.expr_values.push_back(value_to_push); + self.store.drop_value(deallocate_heap_pos); }, Expression::Slicing(expr) => { // TODO: Out of bounds checking @@ -304,13 +309,25 @@ impl Prompt { }, Expression::Select(expr) => { let subject= cur_frame.expr_values.pop_back().unwrap(); - let heap_pos = match &subject { - Value::Ref(value_ref) => self.store.read_ref(*value_ref).as_struct(), - subject => subject.as_struct(), + let field_idx = expr.field.as_symbolic().field_idx as u32; + // Note: same as above: clone if value lives on expr stack, simply + // refer to it if it already lives on the stack/heap. + let (deallocate_heap_pos, value_to_push) = match subject { + Value::Ref(value_ref) => { + let subject = self.store.read_ref(value_ref); + let subject_heap_pos = subject.as_struct(); + + (None, Value::Ref(ValueId::Heap(subject_heap_pos, field_idx))) + }, + _ => { + let subject_heap_pos = subject.as_struct(); + let subject_indexed = Value::Ref(ValueId::Heap(subject_heap_pos, field_idx)); + (Some(subject_heap_pos), self.store.clone_value(subject_indexed)) + }, }; - cur_frame.expr_values.push_back(Value::Ref(ValueId::Heap(heap_pos, expr.field.as_symbolic().field_idx as u32))); - self.store.drop_value(subject.get_heap_pos()); + cur_frame.expr_values.push_back(value_to_push); + self.store.drop_value(deallocate_heap_pos); }, Expression::Literal(expr) => { let value = match &expr.value { @@ -524,12 +541,10 @@ impl Prompt { // we may be returning a reference to something on our stack, // so we need to read that value and clone it. let return_value = cur_frame.expr_values.pop_back().unwrap(); - println!("DEBUG: Pre-ret val {:?}", &return_value); let return_value = match return_value { Value::Ref(value_id) => self.store.read_copy(value_id), _ => return_value, }; - println!("DEBUG: Pos-ret val {:?}", &return_value); // Pre-emptively pop our stack frame self.frames.pop(); @@ -552,8 +567,9 @@ impl Prompt { let cur_frame = self.frames.last_mut().unwrap(); cur_frame.expr_values.push_back(return_value); - // Immediately return, we don't care about the current frame - // anymore and there is nothing left to evaluate + // We just returned to the previous frame, which might be in + // the middle of evaluating expressions for a particular + // statement. So we don't want to enter the code below. return Ok(EvalContinuation::Stepping); }, Statement::Goto(stmt) => { @@ -600,6 +616,14 @@ impl Prompt { }, }; + assert!( + cur_frame.expr_values.is_empty(), + "This is a debugging assertion that will fail if you perform expressions without \ + assigning to anything. This should be completely valid, and this assertions should be \ + replaced by something that clears the expression values if needed, but I'll keep this \ + in for now for debugging purposes." + ); + // If the next statement requires evaluating expressions then we push // these onto the expression stack. This way we will evaluate this // stack in the next loop, then evaluate the statement using the result diff --git a/src/protocol/eval/store.rs b/src/protocol/eval/store.rs index b2c17b8273c6af9d5dda4368f3b074d8cce7f392..468f98827091074a43fb43658ec71837aa7f68c9 100644 --- a/src/protocol/eval/store.rs +++ b/src/protocol/eval/store.rs @@ -148,10 +148,9 @@ impl Store { /// either a direct value, or might contain an index to a heap value), but /// should be treated by the programmer as a reference (i.e. don't call /// `drop_value(thing)` after calling `clone_value(thing.clone())`. - fn clone_value(&mut self, value: Value) -> Value { + pub(crate) fn clone_value(&mut self, value: Value) -> Value { // Quickly check if the value is not on the heap let source_heap_pos = value.get_heap_pos(); - println!("DEBUG: Cloning {:?}", value); if source_heap_pos.is_none() { // We can do a trivial copy, unless we're dealing with a value // reference diff --git a/src/protocol/eval/value.rs b/src/protocol/eval/value.rs index 27c8a7129c6bd33ba070a856018bb4490ec71a01..74e48556f09b6af6c33ab29a2c00744d3c218493 100644 --- a/src/protocol/eval/value.rs +++ b/src/protocol/eval/value.rs @@ -275,6 +275,7 @@ pub(crate) fn apply_assignment_operator(store: &mut Store, lhs: ValueId, op: Ass } } + // let rhs = store.maybe_read_ref(&rhs).clone(); // we don't own this thing, so don't drop it let lhs = store.read_mut_ref(lhs); let mut to_dealloc = None; @@ -392,10 +393,20 @@ pub(crate) fn apply_binary_operator(store: &mut Store, lhs: &Value, op: BinaryOp _ => unreachable!("apply_binary_operator {:?} on lhs {:?} and rhs {:?}", op, lhs, rhs) } + let lhs_heap_pos = lhs_heap_pos as usize; + let rhs_heap_pos = rhs_heap_pos as usize; + // TODO: I hate this, but fine... let mut concatenated = Vec::new(); - concatenated.extend_from_slice(&store.heap_regions[lhs_heap_pos as usize].values); - concatenated.extend_from_slice(&store.heap_regions[rhs_heap_pos as usize].values); + let lhs_len = store.heap_regions[lhs_heap_pos].values.len(); + let rhs_len = store.heap_regions[rhs_heap_pos].values.len(); + concatenated.reserve(lhs_len + rhs_len); + for idx in 0..lhs_len { + concatenated.push(store.clone_value(store.heap_regions[lhs_heap_pos].values[idx].clone())); + } + for idx in 0..rhs_len { + concatenated.push(store.clone_value(store.heap_regions[rhs_heap_pos].values[idx].clone())); + } store.heap_regions[target_heap_pos as usize].values = concatenated; diff --git a/src/protocol/parser/pass_typing.rs b/src/protocol/parser/pass_typing.rs index 09d5717e678e4a079590eb3adb24b3dfe272c1de..94f8f0026575ac6f8c04687afe91180408558ad9 100644 --- a/src/protocol/parser/pass_typing.rs +++ b/src/protocol/parser/pass_typing.rs @@ -70,6 +70,7 @@ use super::visitor::{ }; use std::collections::hash_map::Entry; +const VOID_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Void ]; const MESSAGE_TEMPLATE: [InferenceTypePart; 2] = [ InferenceTypePart::Message, InferenceTypePart::UInt8 ]; const BOOL_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Bool ]; const CHARACTER_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Character ]; @@ -1210,6 +1211,13 @@ impl Visitor2 for PassTyping { } fn visit_select_expr(&mut self, ctx: &mut Ctx, id: SelectExpressionId) -> VisitorResult { + // TODO: @Monomorph, this is a temporary hack, see other comments + let expr = &mut ctx.heap[id]; + if let Field::Symbolic(field) = &mut expr.field { + field.definition = None; + field.field_idx = 0; + } + let upcast_id = id.upcast(); self.insert_initial_expr_inference_type(ctx, upcast_id)?; @@ -1347,6 +1355,9 @@ impl PassTyping { // the AST and have now performed typechecking for a different // monomorph. In that case we just need to perform typechecking, no need // to annotate the AST again. + // TODO: @Monomorph, this is completely wrong. It seemed okay, but it + // isn't. Each monomorph might result in completely different internal + // types. let definition_id = match &self.definition_type { DefinitionType::Component(id) => id.upcast(), DefinitionType::Function(id) => id.upcast(), @@ -1517,8 +1528,11 @@ impl PassTyping { fn progress_assignment_expr(&mut self, ctx: &mut Ctx, id: AssignmentExpressionId) -> Result<(), ParseError> { use AssignmentOperator as AO; - // TODO: Assignable check let upcast_id = id.upcast(); + + // Assignment does not return anything (it operates like a statement) + let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &VOID_TEMPLATE)?; + let expr = &ctx.heap[id]; let arg1_expr_id = expr.left; let arg2_expr_id = expr.right; @@ -1529,28 +1543,30 @@ impl PassTyping { debug_log!(" - Arg2 type: {}", self.expr_types.get(&arg2_expr_id).unwrap().display_name(&ctx.heap)); debug_log!(" - Expr type: {}", self.expr_types.get(&upcast_id).unwrap().display_name(&ctx.heap)); - let progress_base = match expr.operation { + // Apply forced constraint to LHS value + let progress_forced = match expr.operation { AO::Set => false, AO::Multiplied | AO::Divided | AO::Added | AO::Subtracted => - self.apply_forced_constraint(ctx, upcast_id, &NUMBERLIKE_TEMPLATE)?, + self.apply_forced_constraint(ctx, arg1_expr_id, &NUMBERLIKE_TEMPLATE)?, AO::Remained | AO::ShiftedLeft | AO::ShiftedRight | AO::BitwiseAnded | AO::BitwiseXored | AO::BitwiseOred => - self.apply_forced_constraint(ctx, upcast_id, &INTEGERLIKE_TEMPLATE)?, + self.apply_forced_constraint(ctx, arg1_expr_id, &INTEGERLIKE_TEMPLATE)?, }; - let (progress_expr, progress_arg1, progress_arg2) = self.apply_equal3_constraint( - ctx, upcast_id, arg1_expr_id, arg2_expr_id, 0 + let (progress_arg1, progress_arg2) = self.apply_equal2_constraint( + ctx, upcast_id, arg1_expr_id, 0, arg2_expr_id, 0 )?; + debug_assert!(if progress_forced { progress_arg2 } else { true }); debug_log!(" * After:"); - debug_log!(" - Arg1 type [{}]: {}", progress_arg1, self.expr_types.get(&arg1_expr_id).unwrap().display_name(&ctx.heap)); + debug_log!(" - Arg1 type [{}]: {}", progress_forced || progress_arg1, self.expr_types.get(&arg1_expr_id).unwrap().display_name(&ctx.heap)); debug_log!(" - Arg2 type [{}]: {}", progress_arg2, self.expr_types.get(&arg2_expr_id).unwrap().display_name(&ctx.heap)); - debug_log!(" - Expr type [{}]: {}", progress_base || progress_expr, self.expr_types.get(&upcast_id).unwrap().display_name(&ctx.heap)); + debug_log!(" - Expr type [{}]: {}", progress_expr, self.expr_types.get(&upcast_id).unwrap().display_name(&ctx.heap)); - if progress_base || progress_expr { self.queue_expr_parent(ctx, upcast_id); } - if progress_arg1 { self.queue_expr(arg1_expr_id); } + if progress_expr { self.queue_expr_parent(ctx, upcast_id); } + if progress_forced || progress_arg1 { self.queue_expr(arg1_expr_id); } if progress_arg2 { self.queue_expr(arg2_expr_id); } Ok(()) @@ -2358,7 +2374,7 @@ impl PassTyping { let var_expr = &ctx.heap[id]; let var_id = var_expr.declaration.unwrap(); - debug_log!("Variable expr '{}': {}", ctx.heap[var_id].identifier().value.as_str(), upcast_id.index); + debug_log!("Variable expr '{}': {}", ctx.heap[var_id].identifier.value.as_str(), upcast_id.index); debug_log!(" * Before:"); debug_log!(" - Var type: {}", self.var_types.get(&var_id).unwrap().var_type.display_name(&ctx.heap)); debug_log!(" - Expr type: {}", self.expr_types.get(&upcast_id).unwrap().display_name(&ctx.heap)); @@ -2619,7 +2635,6 @@ impl PassTyping { // if polymorph_progress.contains(&poly_idx) { // Need to match subtrees let polymorph_type = &polymorph_data.poly_vars[poly_idx]; - debug_log!(" - DEBUG: Applying {} to '{}' from '{}'", polymorph_type.display_name(heap), InferenceType::partial_display_name(heap, &signature_type.parts[start_idx..]), signature_type.display_name(heap)); let modified_at_marker = Self::apply_forced_constraint_types( signature_type, start_idx, &polymorph_type.parts, 0 diff --git a/src/protocol/tests/eval_calls.rs b/src/protocol/tests/eval_calls.rs index 46a5cfe377b50fb58dbd2ea76dbe31e8f631fb4d..c5ee0ea3f76e521b80950084514a5c802b7ba42f 100644 --- a/src/protocol/tests/eval_calls.rs +++ b/src/protocol/tests/eval_calls.rs @@ -1,5 +1,4 @@ use super::*; -use crate::protocol::eval::*; #[test] fn test_function_call() { diff --git a/src/protocol/tests/eval_operators.rs b/src/protocol/tests/eval_operators.rs index 37082ccd680e5cb35f24f01548193745e74f32fb..96393fcf8ad85e10409432e24801b1080918ba02 100644 --- a/src/protocol/tests/eval_operators.rs +++ b/src/protocol/tests/eval_operators.rs @@ -1,5 +1,4 @@ use super::*; -use crate::protocol::eval::*; #[test] fn test_assignment_operators() { @@ -88,54 +87,6 @@ fn test_assignment_operators() { ); } -#[test] -fn test_concatenate_operator() { - Tester::new_single_source_expect_ok( - "concatenate and check pairs", - " - func check_pair(T[] arr, u32 idx) -> bool { - return arr[idx] == arr[idx + 1]; - } - - struct Point2D { - u32 x, - u32 y, - } - - func create_point(u32 x, u32 y) -> Point2D { - return Point2D{ x: x, y: y }; - } - - func create_array() -> Point2D[] { - return { - create_point(1, 2), - create_point(1, 2), - create_point(3, 4), - create_point(3, 4) - }; - } - - func foo() -> bool { - auto lhs = create_array(); - auto rhs = create_array(); - auto total = lhs @ rhs; - auto is_equal = - check_pair(total, 0) && - check_pair(total, 2) && - check_pair(total, 4) && - check_pair(total, 6); - auto is_not_equal = - !check_pair(total, 0) || - !check_pair(total, 2) || - !check_pair(total, 4) || - !check_pair(total, 6); - return is_equal && !is_not_equal; - } - " - ).for_function("foo", |f| { - f.call(Some(Value::Bool(true))); - }); -} #[test] fn test_binary_integer_operators() { fn construct_source(value_type: &str, code: &str) -> String { diff --git a/src/protocol/tests/eval_silly.rs b/src/protocol/tests/eval_silly.rs new file mode 100644 index 0000000000000000000000000000000000000000..ef3688cd7673147e0bc939fc03e6f908ebef4d32 --- /dev/null +++ b/src/protocol/tests/eval_silly.rs @@ -0,0 +1,98 @@ +use super::*; + +#[test] +fn test_concatenate_operator() { + Tester::new_single_source_expect_ok( + "concatenate and check values", + " + // Too see if we accept the polymorphic arg + func check_pair(T[] arr, u32 idx) -> bool { + return arr[idx] == arr[idx + 1]; + } + + // Too see if we can check fields of polymorphs + func check_values(T[] arr, u32 idx, u32 x, u32 y) -> bool { + auto value = arr[idx]; + return value.x == x && value.y == y; + } + + struct Point2D { + u32 x, + u32 y, + } + + // Could do this inline, but we're attempt to stress the system a bit + func create_point(u32 x, u32 y) -> Point2D { + return Point2D{ x: x, y: y }; + } + + // Again, more stressing: returning a heap-allocated thing + func create_array() -> Point2D[] { + return { + create_point(1, 2), + create_point(1, 2), + create_point(3, 4), + create_point(3, 4) + }; + } + + // The silly checkamajig + func foo() -> bool { + auto lhs = create_array(); + auto rhs = create_array(); + auto total = lhs @ rhs; + auto is_equal = + check_pair(total, 0) && + check_pair(total, 2) && + check_pair(total, 4) && + check_pair(total, 6); + auto is_not_equal = + !check_pair(total, 0) || + !check_pair(total, 2) || + !check_pair(total, 4) || + !check_pair(total, 6); + auto has_correct_fields = + check_values(total, 3, 3, 4) && + check_values(total, 4, 1, 2); + return is_equal && !is_not_equal && has_correct_fields; + } + " + ).for_function("foo", |f| { + f.call(Some(Value::Bool(true))); + }); +} + +#[test] +fn test_struct_fields() { + Tester::new_single_source_expect_ok("struct field access", +" +struct Nester { + T v, +} + +func make(T inner) -> Nester { + return Nester{ v: inner }; +} + +func modify(Nester outer, T inner) -> Nester { + outer.v = inner; + return outer; +} + +func foo() -> bool { + // Single depth modification + auto original1 = make(5); + auto modified1 = modify(original1, 2); + auto success1 = original1.v == 5 && modified1.v == 2; + + // Multiple levels of modification + auto original2 = make(make(make(make(true)))); + auto modified2 = modify(original2.v, make(make(false))); // strip one Nester level + auto success2 = original2.v.v.v.v == true && modified2.v.v.v == false; + + return success1 && success2; +} +").for_function("foo", |f| { + f.call(Some(Value::Bool(true))); + }); +} \ No newline at end of file diff --git a/src/protocol/tests/mod.rs b/src/protocol/tests/mod.rs index c326c008d60559ce528ad882e1869a116e00583e..e10f3657e438837822df5f51d791f793ebdf1a96 100644 --- a/src/protocol/tests/mod.rs +++ b/src/protocol/tests/mod.rs @@ -16,5 +16,7 @@ mod parser_monomorphs; mod parser_imports; mod eval_operators; mod eval_calls; +mod eval_silly; -pub(crate) use utils::{Tester}; \ No newline at end of file +pub(crate) use utils::{Tester}; // the testing harness +pub(crate) use crate::protocol::eval::value::*; // to test functions \ No newline at end of file diff --git a/src/protocol/tests/utils.rs b/src/protocol/tests/utils.rs index b510760722f0cb48d5502cd2e5c25691a9a0d367..36a068cde8afa9a987d60c0cc7a1d09542add123 100644 --- a/src/protocol/tests/utils.rs +++ b/src/protocol/tests/utils.rs @@ -638,9 +638,10 @@ impl<'a> VariableTester<'a> { pub(crate) fn assert_concrete_type(self, expected: &str) -> Self { let mut serialized = String::new(); + let lhs = self.ctx.heap[self.assignment.left].as_variable(); serialize_concrete_type( &mut serialized, self.ctx.heap, self.definition_id, - &self.assignment.concrete_type + &lhs.concrete_type ); assert_eq!(