diff --git a/src/protocol/ast.rs b/src/protocol/ast.rs index 1c85a558741dd335dee6f3e89d60059b29e3ae9f..fb2a3c6cd3524b61af5ba30a3e9b10d9b3aa6950 100644 --- a/src/protocol/ast.rs +++ b/src/protocol/ast.rs @@ -1409,6 +1409,13 @@ impl ExpressionParent { _ => false, } } + + pub fn as_expression(&self) -> ExpressionId { + match self { + ExpressionParent::Expression(id, _) => *id, + _ => panic!("called as_expression() on {:?}", self), + } + } } #[derive(Debug, Clone)] diff --git a/src/protocol/parser/pass_typing.rs b/src/protocol/parser/pass_typing.rs index 26a9543b7881db7df3230332507a112d7d9f3ac0..c490a2a446035402fd5845e6b00d04daeba2e2c7 100644 --- a/src/protocol/parser/pass_typing.rs +++ b/src/protocol/parser/pass_typing.rs @@ -66,8 +66,6 @@ use super::visitor::{ const VOID_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Void ]; const MESSAGE_TEMPLATE: [InferenceTypePart; 2] = [ InferenceTypePart::Message, InferenceTypePart::UInt8 ]; const BOOL_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Bool ]; -const BOOLLIKE_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::BoolLike ]; -const BINDING_BOOL_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::BindingBool ]; const CHARACTER_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::Character ]; const STRING_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::String ]; const NUMBERLIKE_TEMPLATE: [InferenceTypePart; 1] = [ InferenceTypePart::NumberLike ]; @@ -92,7 +90,6 @@ pub(crate) enum InferenceTypePart { // Partially known type, may be inferred to to be the appropriate related // type. // IndexLike, // index into array/slice - BoolLike, // boolean or binding boolean NumberLike, // any kind of integer/float IntegerLike, // any kind of integer ArrayLike, // array or slice. Note that this must have a subtype @@ -100,7 +97,6 @@ pub(crate) enum InferenceTypePart { // Special types that cannot be instantiated by the user Void, // For builtin functions that do not return anything // Concrete types without subtypes - BindingBool, // boolean result from a binding expression Bool, UInt8, UInt16, @@ -135,7 +131,7 @@ impl InferenceTypePart { fn is_concrete(&self) -> bool { use InferenceTypePart as ITP; match self { - ITP::Unknown | ITP::BoolLike | ITP::NumberLike | + ITP::Unknown | ITP::NumberLike | ITP::IntegerLike | ITP::ArrayLike | ITP::PortLike => false, _ => true } @@ -184,9 +180,7 @@ impl InferenceTypePart { (*self == ITP::IntegerLike && arg.is_concrete_integer()) || (*self == ITP::NumberLike && (arg.is_concrete_number() || *arg == ITP::IntegerLike)) || (*self == ITP::ArrayLike && arg.is_concrete_msg_array_or_slice()) || - (*self == ITP::PortLike && arg.is_concrete_port()) || - (*self == ITP::BoolLike && (*arg == ITP::Bool || *arg == ITP::BindingBool)) || - (*self == ITP::Bool && *arg == ITP::BindingBool) + (*self == ITP::PortLike && arg.is_concrete_port()) } /// Checks if a part is more specific @@ -198,7 +192,7 @@ impl InferenceTypePart { use InferenceTypePart as ITP; match &self { ITP::Unknown | ITP::NumberLike | ITP::IntegerLike | - ITP::Void | ITP::BoolLike | ITP::Bool | ITP::BindingBool | + ITP::Void | ITP::Bool | ITP::UInt8 | ITP::UInt16 | ITP::UInt32 | ITP::UInt64 | ITP::SInt8 | ITP::SInt16 | ITP::SInt32 | ITP::SInt64 | ITP::Character | ITP::String => { @@ -491,8 +485,7 @@ impl InferenceType { /// is false, then as long as the `to_infer` and `template` types are /// compatible the inference will succeed. If `forced_template` is true, /// then `to_infer` MUST be less specific than `template` (e.g. - /// `IntegerLike` is less specific than `UInt32`. Likewise `Bool` is less - /// specific than `BindingBool`) + /// `IntegerLike` is less specific than `UInt32`) fn infer_subtree_for_single_type( to_infer: &mut InferenceType, mut to_infer_idx: usize, template: &[InferenceTypePart], mut template_idx: usize, @@ -613,7 +606,7 @@ impl InferenceType { idx += 1; continue; }, - ITP::Unknown | ITP::BoolLike | ITP::NumberLike | + ITP::Unknown | ITP::NumberLike | ITP::IntegerLike | ITP::ArrayLike | ITP::PortLike => { // Should not happen if type inferencing works correctly: we // should have returned a programmer-readable error or have @@ -622,7 +615,6 @@ impl InferenceType { }, ITP::Void => CTP::Void, ITP::Message => CTP::Message, - ITP::BindingBool => CTP::Bool, ITP::Bool => CTP::Bool, ITP::UInt8 => CTP::UInt8, ITP::UInt16 => CTP::UInt16, @@ -661,7 +653,6 @@ impl InferenceType { idx = Self::write_display_name(buffer, heap, parts, idx + 1); }, ITP::Unknown => buffer.push_str("?"), - ITP::BoolLike => buffer.push_str("boollike"), ITP::NumberLike => buffer.push_str("numberlike"), ITP::IntegerLike => buffer.push_str("integerlike"), ITP::ArrayLike => { @@ -674,7 +665,6 @@ impl InferenceType { buffer.push('>'); } ITP::Void => buffer.push_str("void"), - ITP::BindingBool => buffer.push_str("binding result"), ITP::Bool => buffer.push_str(KW_TYPE_BOOL_STR), ITP::UInt8 => buffer.push_str(KW_TYPE_UINT8_STR), ITP::UInt16 => buffer.push_str(KW_TYPE_UINT16_STR), @@ -1671,9 +1661,9 @@ impl PassTyping { let bound_from_id = binding_expr.bound_from; let bound_to_id = binding_expr.bound_to; - // Output of a binding expression is a special kind of boolean that can - // only be used in binary-and expressions - let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BINDING_BOOL_TEMPLATE)?; + // Output is always a boolean. The two arguments should be of equal + // type. + let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; let (progress_from, progress_to) = self.apply_equal2_constraint(ctx, upcast_id, bound_from_id, 0, bound_to_id, 0)?; if progress_expr { self.queue_expr_parent(ctx, upcast_id); } @@ -1696,6 +1686,10 @@ impl PassTyping { debug_log!(" - Arg2 type: {}", self.temp_get_display_name(ctx, arg2_expr_id)); debug_log!(" - Expr type: {}", self.temp_get_display_name(ctx, upcast_id)); + // I keep confusing myself: this applies equality of types between the + // condition branches' types, and the result from the conditional + // expression, because the result from the conditional is one of the + // branches. let (progress_expr, progress_arg1, progress_arg2) = self.apply_equal3_constraint( ctx, upcast_id, arg1_expr_id, arg2_expr_id, 0 )?; @@ -1742,16 +1736,12 @@ impl PassTyping { (progress_expr || subtype_expr, progress_arg1 || subtype_arg1, progress_arg2 || subtype_arg2) }, BO::LogicalAnd => { - // Logical AND may operate both on normal booleans and on - // booleans that are the result of a binding expression. So we - // force the expression to bool-like, then apply an equal_3 - // constraint. Any BindingBool will promote all the other Bool - // types. - let base_expr = self.apply_template_constraint(ctx, upcast_id, &BOOLLIKE_TEMPLATE)?; - let (progress_expr, progress_arg1, progress_arg2) = - self.apply_equal3_constraint(ctx, upcast_id, arg1_id, arg2_id, 0)?; + // Forced boolean on all + let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; + let progress_arg1 = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; + let progress_arg2 = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; - (base_expr || progress_expr, progress_arg1, progress_arg2) + (progress_expr, progress_arg1, progress_arg2) }, BO::LogicalOr => { // Forced boolean on all @@ -1771,7 +1761,7 @@ impl PassTyping { }, BO::Equality | BO::Inequality => { // Equal2 on args, forced boolean output - let progress_expr = self.apply_template_constraint(ctx, upcast_id, &BOOLLIKE_TEMPLATE)?; + let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; let (progress_arg1, progress_arg2) = self.apply_equal2_constraint(ctx, upcast_id, arg1_id, 0, arg2_id, 0)?; @@ -1779,7 +1769,7 @@ impl PassTyping { }, BO::LessThan | BO::GreaterThan | BO::LessThanEqual | BO::GreaterThanEqual => { // Equal2 on args with numberlike type, forced boolean output - let progress_expr = self.apply_template_constraint(ctx, upcast_id, &BOOLLIKE_TEMPLATE)?; + let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; let progress_arg_base = self.apply_template_constraint(ctx, arg1_id, &NUMBERLIKE_TEMPLATE)?; let (progress_arg1, progress_arg2) = self.apply_equal2_constraint(ctx, upcast_id, arg1_id, 0, arg2_id, 0)?; @@ -1838,7 +1828,7 @@ impl PassTyping { (progress_base || progress_expr, progress_base || progress_arg) }, UO::LogicalNot => { - // Both booleans + // Both bools let progress_expr = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; let progress_arg = self.apply_forced_constraint(ctx, upcast_id, &BOOL_TEMPLATE)?; (progress_expr, progress_arg) diff --git a/src/protocol/parser/pass_validation_linking.rs b/src/protocol/parser/pass_validation_linking.rs index cfd28c4f3ab906c4d244ccb119ba7adc77f74ec5..99a7ea702e46371d4a3e8ce1834deeb2f53bb9c4 100644 --- a/src/protocol/parser/pass_validation_linking.rs +++ b/src/protocol/parser/pass_validation_linking.rs @@ -406,6 +406,16 @@ impl Visitor2 for PassValidationLinking { let assignment_expr = &mut ctx.heap[id]; + // Although we call assignment an expression to simplify the compiler's + // code (mainly typechecking), we disallow nested use in expressions + match self.expr_parent { + ExpressionParent::ExpressionStmt(_) => {}, + _ => return Err(ParseError::new_error_str_at_span( + &ctx.module.source, assignment_expr.span, + "assignments may only appear at the statement level" + )), + } + let left_expr_id = assignment_expr.left; let right_expr_id = assignment_expr.right; let old_expr_parent = self.expr_parent; @@ -425,7 +435,6 @@ impl Visitor2 for PassValidationLinking { fn visit_binding_expr(&mut self, ctx: &mut Ctx, id: BindingExpressionId) -> VisitorResult { let upcast_id = id.upcast(); - let binding_expr = &mut ctx.heap[id]; // Check for valid context of binding expression if let Some(span) = self.must_be_assignable { @@ -435,6 +444,7 @@ impl Visitor2 for PassValidationLinking { } if self.in_test_expr.is_invalid() { + let binding_expr = &ctx.heap[id]; return Err(ParseError::new_error_str_at_span( &ctx.module.source, binding_expr.span, "binding expressions can only be used inside the testing expression of 'if' and 'while' statements" @@ -453,6 +463,51 @@ impl Visitor2 for PassValidationLinking { )); } + let mut seeking_parent = self.expr_parent; + loop { + // Perform upward search to make sure only LogicalAnd is applied to + // the binding expression + let valid = match seeking_parent { + ExpressionParent::If(_) | ExpressionParent::While(_) => { + // Every parent expression (if any) were LogicalAnd. + break; + } + ExpressionParent::Expression(parent_id, _) => { + let parent_expr = &ctx.heap[parent_id]; + match parent_expr { + Expression::Binary(parent_expr) => { + // Set new parent to continue the search. Otherwise + // halt and provide an error using the current + // parent. + if parent_expr.operation == BinaryOperator::LogicalAnd { + seeking_parent = parent_expr.parent; + true + } else { + false + } + }, + _ => false, + } + }, + _ => unreachable!(), // nested under if/while, so always expressions as parents + }; + + if !valid { + let binding_expr = &ctx.heap[id]; + let parent_expr = &ctx.heap[seeking_parent.as_expression()]; + return Err(ParseError::new_error_str_at_span( + &ctx.module.source, binding_expr.span, + "only the logical-and operator (&&) may be applied to binding expressions" + ).with_info_str_at_span( + &ctx.module.source, parent_expr.span(), + "this was the disallowed operation applied to the result from a binding expression" + )); + } + } + + // Perform all of the index/parent assignment magic + let binding_expr = &mut ctx.heap[id]; + let old_expr_parent = self.expr_parent; binding_expr.parent = old_expr_parent; binding_expr.unique_id_in_definition = self.next_expr_index; diff --git a/src/protocol/tests/parser_binding.rs b/src/protocol/tests/parser_binding.rs index 6fcab16a2354fa7b50c2021042976446f72051ff..4aa276e9371a3b41772ecf8e7feb2b99b8c4b935 100644 --- a/src/protocol/tests/parser_binding.rs +++ b/src/protocol/tests/parser_binding.rs @@ -109,9 +109,11 @@ fn test_boolean_ops_on_binding() { return 0; } ").error(|e| { e - .assert_num(1) - .assert_occurs_at(0, "let") - .assert_msg_has(0, "expected a 'bool'"); + .assert_num(2) + .assert_occurs_at(0, "let a") + .assert_msg_has(0, "only the logical-and operator") + .assert_occurs_at(1, "||") + .assert_msg_has(1, "disallowed operation"); }); Tester::new_single_source_expect_err("apply || after binding", " @@ -123,9 +125,11 @@ fn test_boolean_ops_on_binding() { return 0; } ").error(|e| { e - .assert_num(1) - .assert_occurs_at(0, "let") - .assert_msg_has(0, "expected a 'bool'"); + .assert_num(2) + .assert_occurs_at(0, "let b") + .assert_msg_has(0, "only the logical-and operator") + .assert_occurs_at(1, "||") + .assert_msg_has(1, "disallowed operation"); }); Tester::new_single_source_expect_err("apply || before and after binding", " @@ -137,7 +141,10 @@ fn test_boolean_ops_on_binding() { return 0; } ").error(|e| { e - .assert_num(1) - .assert_msg_has(0, "expected a 'bool'"); + .assert_num(2) + .assert_occurs_at(0, "let a") + .assert_msg_has(0, "only the logical-and operator") + .assert_occurs_at(1, "|| (let a") + .assert_msg_has(1, "disallowed operation"); }); } \ No newline at end of file