From fbe6f02fd1b4c8cf3722906303b89e1ea7dc9752 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Tue, 20 Aug 2024 14:19:27 +0200 Subject: [PATCH 1/3] Allow assignment as last expression This is debatable, but I would argue that it's been sufficiently annoying for people and such a low-hanging fruit that we ought to do something about it. The strategy here is simple: when we find a sequence of expression that ends with an assignment (let or expect), we can simply desugar it into two expressions: the assignment followed by either `Void` or a boolean. The latter is used when the assignment pattern is itself a boolean; the next boolean becomes the expected value. The former, `Void`, is used for everything else. So said differently, any assignment implicitly _returns Void_, except for boolean which return the actual patterned bool.
expressiondesugar into
```aiken fn expect_bool(data: Data) -> Void { expect _: Bool = data } ``` ```aiken fn expect_bool(data: Data) -> Void { expect _: Bool = data Void } ```
```aiken fn weird_maths() -> Bool { expect 1 == 2 } ``` ```aiken fn weird_maths() -> Bool { expect True = 1 == 2 True } ```
--- crates/aiken-lang/src/ast.rs | 20 ++++ crates/aiken-lang/src/expr.rs | 43 +++++++- crates/aiken-lang/src/tests/check.rs | 155 +++++++++++++-------------- crates/aiken-lang/src/tipo/error.rs | 4 +- crates/aiken-lang/src/tipo/expr.rs | 121 +++++++++++++++++---- 5 files changed, 240 insertions(+), 103 deletions(-) diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index f3b0782d..2a14012f 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -1423,6 +1423,26 @@ impl UntypedPattern { is_record: false, } } + + /// Returns Some() if the pattern is a [`Boolean`] literal, + /// holding the target value. None if it isn't a bool pattern. + pub fn get_bool(&self) -> Option { + match self { + Self::Constructor { + module: None, + name, + constructor: (), + .. + } if name == "True" => Some(true), + Self::Constructor { + module: None, + name, + constructor: (), + .. + } if name == "False" => Some(false), + _ => None, + } + } } impl TypedPattern { diff --git a/crates/aiken-lang/src/expr.rs b/crates/aiken-lang/src/expr.rs index 6e131a5b..2749d9bf 100644 --- a/crates/aiken-lang/src/expr.rs +++ b/crates/aiken-lang/src/expr.rs @@ -1,4 +1,5 @@ -use crate::{ +use crate::tipo::ValueConstructorVariant; +pub(crate) use crate::{ ast::{ self, Annotation, ArgBy, ArgName, AssignmentPattern, BinOp, Bls12_381Point, ByteArrayFormatPreference, CallArg, Curve, DataType, DataTypeKey, DefinitionLocation, @@ -7,7 +8,7 @@ use crate::{ TypedDataType, TypedIfBranch, TypedRecordUpdateArg, UnOp, UntypedArg, UntypedAssignmentKind, UntypedClause, UntypedIfBranch, UntypedRecordUpdateArg, }, - builtins::void, + builtins::{bool, void}, parser::token::Base, tipo::{ check_replaceable_opaque_type, convert_opaque_type, lookup_data_type_by_tipo, @@ -472,6 +473,44 @@ impl TypedExpr { .or(Some(Located::Expression(self))), } } + + pub fn void(location: Span) -> Self { + TypedExpr::Var { + name: "Void".to_string(), + constructor: ValueConstructor { + public: true, + variant: ValueConstructorVariant::Record { + name: "Void".to_string(), + arity: 0, + field_map: None, + location: Span::empty(), + module: String::new(), + constructors_count: 1, + }, + tipo: void(), + }, + location, + } + } + + pub fn bool(value: bool, location: Span) -> Self { + TypedExpr::Var { + name: "Bool".to_string(), + constructor: ValueConstructor { + public: true, + variant: ValueConstructorVariant::Record { + name: if value { "True" } else { "False" }.to_string(), + arity: 0, + field_map: None, + location: Span::empty(), + module: String::new(), + constructors_count: 2, + }, + tipo: bool(), + }, + location, + } + } } // Represent how a function was written so that we can format it back. diff --git a/crates/aiken-lang/src/tests/check.rs b/crates/aiken-lang/src/tests/check.rs index 906240e7..69d18311 100644 --- a/crates/aiken-lang/src/tests/check.rs +++ b/crates/aiken-lang/src/tests/check.rs @@ -1009,85 +1009,6 @@ fn anonymous_function_dupicate_args() { )) } -#[test] -fn assignement_last_expr_when() { - let source_code = r#" - pub fn foo() { - let bar = None - - when bar is { - Some(_) -> { - let wow = 1 - } - None -> { - 2 - } - } - } - "#; - - assert!(matches!( - check(parse(source_code)), - Err((_, Error::LastExpressionIsAssignment { .. })) - )) -} - -#[test] -fn assignement_last_expr_if_first_branch() { - let source_code = r#" - pub fn foo() { - if True { - let thing = 1 - } else { - 1 - } - } - "#; - - assert!(matches!( - check(parse(source_code)), - Err((_, Error::LastExpressionIsAssignment { .. })) - )) -} - -#[test] -fn assignement_last_expr_if_branches() { - let source_code = r#" - pub fn foo() { - if True { - 2 - } else if False { - let thing = 1 - } else { - 1 - } - } - "#; - - assert!(matches!( - check(parse(source_code)), - Err((_, Error::LastExpressionIsAssignment { .. })) - )) -} - -#[test] -fn assignement_last_expr_if_final_else() { - let source_code = r#" - pub fn foo() { - if True { - 1 - } else { - let thing = 1 - } - } - "#; - - assert!(matches!( - check(parse(source_code)), - Err((_, Error::LastExpressionIsAssignment { .. })) - )) -} - #[test] fn if_scoping() { let source_code = r#" @@ -2956,3 +2877,79 @@ fn pattern_bytearray_not_unify_subject() { Err((_, Error::CouldNotUnify { .. })) )) } + +#[test] +fn recover_no_assignment_sequence() { + let source_code = r#" + pub fn main() { + let result = 42 + expect result + 1 == 43 + } + "#; + + assert!(check(parse(source_code)).is_ok()); +} + +#[test] +fn recover_no_assignment_fn_body() { + let source_code = r#" + pub fn is_bool(foo: Data) -> Void { + expect _: Bool = foo + } + "#; + + assert!(check(parse(source_code)).is_ok()); +} + +#[test] +fn recover_no_assignment_when_clause() { + let source_code = r#" + pub fn main(foo) { + when foo is { + [] -> let bar = foo + [x, ..] -> expect _: Int = x + } + } + "#; + + let (warnings, _) = check(parse(source_code)).unwrap(); + + assert!(matches!( + &warnings[..], + [Warning::UnusedVariable { name, .. }] if name == "bar", + )) +} + +#[test] +fn recover_no_assignment_fn_if_then_else() { + let source_code = r#" + pub fn foo(weird_maths) -> Bool { + if weird_maths { + expect 1 == 2 + } else { + expect 1 + 1 == 2 + } + } + "#; + + assert!(check(parse(source_code)).is_ok()); +} + +#[test] +fn recover_no_assignment_logical_chain_op() { + let source_code = r#" + pub fn foo() -> Bool { + and { + expect 1 + 1 == 2, + True, + 2 > 0, + or { + expect True, + False, + } + } + } + "#; + + assert!(check(parse(source_code)).is_ok()); +} diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 91dbb988..0fe9293d 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -2,7 +2,7 @@ use super::Type; use crate::{ ast::{Annotation, BinOp, CallArg, LogicalOpChainKind, Span, UntypedFunction, UntypedPattern}, error::ExtraData, - expr::{self, UntypedExpr}, + expr::{self, AssignmentPattern, UntypedExpr}, format::Formatter, levenshtein, pretty::Documentable, @@ -15,6 +15,7 @@ use owo_colors::{ Stream::{Stderr, Stdout}, }; use std::{collections::HashMap, fmt::Display, rc::Rc}; +use vec1::Vec1; #[derive(Debug, Clone, thiserror::Error)] #[error( @@ -470,6 +471,7 @@ If you really meant to return that last expression, try to replace it with the f #[label("let-binding as last expression")] location: Span, expr: expr::UntypedExpr, + patterns: Vec1, }, #[error( diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index cfa63a1b..7c0041fc 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -1397,9 +1397,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let (then, typed_patterns) = self.in_new_scope(|scope| { let typed_patterns = scope.infer_clause_pattern(patterns, subject, &location)?; - assert_no_assignment(&then)?; - - let then = scope.infer(then)?; + let then = if let Some(filler) = + recover_from_no_assignment(assert_no_assignment(&then), then.location())? + { + TypedExpr::Sequence { + location, + expressions: vec![scope.infer(then)?, filler], + } + } else { + scope.infer(then)? + }; Ok::<_, Error>((then, typed_patterns)) })?; @@ -1521,8 +1528,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> { typed_branches.push(typed_branch); } - assert_no_assignment(&final_else)?; - let typed_final_else = self.infer(final_else)?; + let typed_final_else = if let Some(filler) = + recover_from_no_assignment(assert_no_assignment(&final_else), final_else.location())? + { + TypedExpr::Sequence { + location: final_else.location(), + expressions: vec![self.infer(final_else)?, filler], + } + } else { + self.infer(final_else)? + }; self.unify( first_body_type.clone(), @@ -1569,8 +1584,18 @@ impl<'a, 'b> ExprTyper<'a, 'b> { location: branch.condition.location().union(location), }) } - assert_no_assignment(&branch.body)?; - let body = typer.infer(branch.body.clone())?; + + let body = if let Some(filler) = recover_from_no_assignment( + assert_no_assignment(&branch.body), + branch.body.location(), + )? { + TypedExpr::Sequence { + location: branch.body.location(), + expressions: vec![typer.infer(branch.body.clone())?, filler], + } + } else { + typer.infer(branch.body.clone())? + }; Ok((*value, body, Some((pattern, tipo)))) })?, @@ -1584,8 +1609,17 @@ impl<'a, 'b> ExprTyper<'a, 'b> { false, )?; - assert_no_assignment(&branch.body)?; - let body = self.infer(branch.body.clone())?; + let body = if let Some(filler) = recover_from_no_assignment( + assert_no_assignment(&branch.body), + branch.body.location(), + )? { + TypedExpr::Sequence { + location: branch.body.location(), + expressions: vec![self.infer(branch.body.clone())?, filler], + } + } else { + self.infer(branch.body.clone())? + }; (condition, body, None) } @@ -1631,7 +1665,9 @@ impl<'a, 'b> ExprTyper<'a, 'b> { body: UntypedExpr, return_type: Option>, ) -> Result<(Vec, TypedExpr, Rc), Error> { - assert_no_assignment(&body)?; + let location = body.location(); + + let no_assignment = assert_no_assignment(&body); let (body_rigid_names, body_infer) = self.in_new_scope(|body_typer| { let mut argument_names = HashMap::with_capacity(args.len()); @@ -1668,7 +1704,17 @@ impl<'a, 'b> ExprTyper<'a, 'b> { Ok((body_typer.hydrator.rigid_names(), body_typer.infer(body))) })?; - let body = body_infer.map_err(|e| e.with_unify_error_rigid_names(&body_rigid_names))?; + let inferred_body = + body_infer.map_err(|e| e.with_unify_error_rigid_names(&body_rigid_names)); + + let body = if let Some(filler) = recover_from_no_assignment(no_assignment, location)? { + TypedExpr::Sequence { + location, + expressions: vec![inferred_body?, filler], + } + } else { + inferred_body? + }; // Check that any return type is accurate. let return_type = match return_type { @@ -1757,9 +1803,17 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let mut typed_expressions = vec![]; for expression in expressions { - assert_no_assignment(&expression)?; - - let typed_expression = self.infer(expression)?; + let typed_expression = if let Some(filler) = recover_from_no_assignment( + assert_no_assignment(&expression), + expression.location(), + )? { + TypedExpr::Sequence { + location: expression.location(), + expressions: vec![self.infer(expression)?, filler], + } + } else { + self.infer(expression)? + }; self.unify( bool(), @@ -2046,21 +2100,29 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let typed_expression = scope.infer(expression)?; - expressions.push(match i.cmp(&(count - 1)) { + match i.cmp(&(count - 1)) { // When the expression is the last in a sequence, we enforce it is NOT // an assignment (kind of treat assignments like statements). Ordering::Equal => { - no_assignment?; - typed_expression + if let Some(filler) = + recover_from_no_assignment(no_assignment, typed_expression.location())? + { + expressions.push(typed_expression); + expressions.push(filler); + } else { + expressions.push(typed_expression); + } } // This isn't the final expression in the sequence, so it *must* // be a let-binding; we do not allow anything else. - Ordering::Less => assert_assignment(typed_expression)?, + Ordering::Less => { + expressions.push(assert_assignment(typed_expression)?); + } // Can't actually happen - Ordering::Greater => typed_expression, - }) + Ordering::Greater => unreachable!(), + } } Ok(expressions) @@ -2479,11 +2541,28 @@ impl<'a, 'b> ExprTyper<'a, 'b> { } } +fn recover_from_no_assignment( + result: Result<(), Error>, + span: Span, +) -> Result, Error> { + if let Err(Error::LastExpressionIsAssignment { patterns, .. }) = result { + match patterns.first().pattern.get_bool() { + Some(expected) if patterns.len() == 1 => Ok(Some(TypedExpr::bool(expected, span))), + _ => Ok(Some(TypedExpr::void(span))), + } + } else { + result.map(|()| None) + } +} + fn assert_no_assignment(expr: &UntypedExpr) -> Result<(), Error> { match expr { - UntypedExpr::Assignment { value, .. } => Err(Error::LastExpressionIsAssignment { + UntypedExpr::Assignment { + value, patterns, .. + } => Err(Error::LastExpressionIsAssignment { location: expr.location(), expr: *value.clone(), + patterns: patterns.clone(), }), UntypedExpr::Trace { then, .. } => assert_no_assignment(then), UntypedExpr::Fn { .. } From 9aa9070f562c8f9f946c9f981ae2f4b9ccee56fb Mon Sep 17 00:00:00 2001 From: KtorZ Date: Wed, 21 Aug 2024 14:31:57 +0200 Subject: [PATCH 2/3] Revise desugaring following feedback - We now consistently desugar an expect in the last position as `Void`. Regardless of the pattern. Desugaring to a boolean value is deemed too confusing. - This commit also removes the desugaring for let-binding. It's only ever allowed for _expect_ which then behaves like a side effect. - We also now allow tests to return either `Bool` or `Void`. A test that returns `Void` is treated the same as a test returning `True`. --- crates/aiken-lang/src/ast.rs | 20 --- crates/aiken-lang/src/expr.rs | 21 +-- .../snapshots/def_invalid_property_test.snap | 9 +- .../snapshots/def_property_test.snap | 9 +- .../def_property_test_annotated_fuzzer.snap | 9 +- .../parser/definition/snapshots/def_test.snap | 9 +- .../definition/snapshots/def_test_fail.snap | 9 +- .../aiken-lang/src/parser/definition/test.rs | 2 +- crates/aiken-lang/src/tests/check.rs | 160 +++++++++++++++--- crates/aiken-lang/src/tipo/error.rs | 18 +- crates/aiken-lang/src/tipo/expr.rs | 35 ++-- crates/aiken-lang/src/tipo/infer.rs | 17 +- crates/uplc/src/machine/eval_result.rs | 6 +- 13 files changed, 196 insertions(+), 128 deletions(-) diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index 2a14012f..f3b0782d 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -1423,26 +1423,6 @@ impl UntypedPattern { is_record: false, } } - - /// Returns Some() if the pattern is a [`Boolean`] literal, - /// holding the target value. None if it isn't a bool pattern. - pub fn get_bool(&self) -> Option { - match self { - Self::Constructor { - module: None, - name, - constructor: (), - .. - } if name == "True" => Some(true), - Self::Constructor { - module: None, - name, - constructor: (), - .. - } if name == "False" => Some(false), - _ => None, - } - } } impl TypedPattern { diff --git a/crates/aiken-lang/src/expr.rs b/crates/aiken-lang/src/expr.rs index 2749d9bf..2af362ea 100644 --- a/crates/aiken-lang/src/expr.rs +++ b/crates/aiken-lang/src/expr.rs @@ -8,7 +8,7 @@ pub(crate) use crate::{ TypedDataType, TypedIfBranch, TypedRecordUpdateArg, UnOp, UntypedArg, UntypedAssignmentKind, UntypedClause, UntypedIfBranch, UntypedRecordUpdateArg, }, - builtins::{bool, void}, + builtins::void, parser::token::Base, tipo::{ check_replaceable_opaque_type, convert_opaque_type, lookup_data_type_by_tipo, @@ -492,25 +492,6 @@ impl TypedExpr { location, } } - - pub fn bool(value: bool, location: Span) -> Self { - TypedExpr::Var { - name: "Bool".to_string(), - constructor: ValueConstructor { - public: true, - variant: ValueConstructorVariant::Record { - name: if value { "True" } else { "False" }.to_string(), - arity: 0, - field_map: None, - location: Span::empty(), - module: String::new(), - constructors_count: 2, - }, - tipo: bool(), - }, - location, - } - } } // Represent how a function was written so that we can format it back. diff --git a/crates/aiken-lang/src/parser/definition/snapshots/def_invalid_property_test.snap b/crates/aiken-lang/src/parser/definition/snapshots/def_invalid_property_test.snap index b7b65666..8d46100a 100644 --- a/crates/aiken-lang/src/parser/definition/snapshots/def_invalid_property_test.snap +++ b/crates/aiken-lang/src/parser/definition/snapshots/def_invalid_property_test.snap @@ -52,14 +52,7 @@ Test( location: 0..26, name: "foo", public: false, - return_annotation: Some( - Constructor { - location: 0..39, - module: None, - name: "Bool", - arguments: [], - }, - ), + return_annotation: None, return_type: (), end_position: 38, on_test_failure: FailImmediately, diff --git a/crates/aiken-lang/src/parser/definition/snapshots/def_property_test.snap b/crates/aiken-lang/src/parser/definition/snapshots/def_property_test.snap index f2ba0716..3a806be9 100644 --- a/crates/aiken-lang/src/parser/definition/snapshots/def_property_test.snap +++ b/crates/aiken-lang/src/parser/definition/snapshots/def_property_test.snap @@ -37,14 +37,7 @@ Test( location: 0..28, name: "foo", public: false, - return_annotation: Some( - Constructor { - location: 0..41, - module: None, - name: "Bool", - arguments: [], - }, - ), + return_annotation: None, return_type: (), end_position: 40, on_test_failure: FailImmediately, diff --git a/crates/aiken-lang/src/parser/definition/snapshots/def_property_test_annotated_fuzzer.snap b/crates/aiken-lang/src/parser/definition/snapshots/def_property_test_annotated_fuzzer.snap index 721edd75..31b1a283 100644 --- a/crates/aiken-lang/src/parser/definition/snapshots/def_property_test_annotated_fuzzer.snap +++ b/crates/aiken-lang/src/parser/definition/snapshots/def_property_test_annotated_fuzzer.snap @@ -44,14 +44,7 @@ Test( location: 0..26, name: "foo", public: false, - return_annotation: Some( - Constructor { - location: 0..39, - module: None, - name: "Bool", - arguments: [], - }, - ), + return_annotation: None, return_type: (), end_position: 38, on_test_failure: FailImmediately, diff --git a/crates/aiken-lang/src/parser/definition/snapshots/def_test.snap b/crates/aiken-lang/src/parser/definition/snapshots/def_test.snap index daddcb5f..42561931 100644 --- a/crates/aiken-lang/src/parser/definition/snapshots/def_test.snap +++ b/crates/aiken-lang/src/parser/definition/snapshots/def_test.snap @@ -13,14 +13,7 @@ Test( location: 0..10, name: "foo", public: false, - return_annotation: Some( - Constructor { - location: 0..23, - module: None, - name: "Bool", - arguments: [], - }, - ), + return_annotation: None, return_type: (), end_position: 22, on_test_failure: FailImmediately, diff --git a/crates/aiken-lang/src/parser/definition/snapshots/def_test_fail.snap b/crates/aiken-lang/src/parser/definition/snapshots/def_test_fail.snap index b8916bcf..4b0d4cb4 100644 --- a/crates/aiken-lang/src/parser/definition/snapshots/def_test_fail.snap +++ b/crates/aiken-lang/src/parser/definition/snapshots/def_test_fail.snap @@ -44,14 +44,7 @@ Test( location: 0..26, name: "invalid_inputs", public: false, - return_annotation: Some( - Constructor { - location: 0..61, - module: None, - name: "Bool", - arguments: [], - }, - ), + return_annotation: None, return_type: (), end_position: 60, on_test_failure: SucceedEventually, diff --git a/crates/aiken-lang/src/parser/definition/test.rs b/crates/aiken-lang/src/parser/definition/test.rs index 0931c30d..6e4cada8 100644 --- a/crates/aiken-lang/src/parser/definition/test.rs +++ b/crates/aiken-lang/src/parser/definition/test.rs @@ -45,7 +45,7 @@ pub fn parser() -> impl Parser { + let wow = 1 + } + None -> { + 2 + } + } + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::LastExpressionIsAssignment { .. })) + )) +} + +#[test] +fn assignement_last_expr_if_first_branch() { + let source_code = r#" + pub fn foo() { + if True { + let thing = 1 + } else { + 1 + } + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::LastExpressionIsAssignment { .. })) + )) +} + +#[test] +fn assignement_last_expr_if_branches() { + let source_code = r#" + pub fn foo() { + if True { + 2 + } else if False { + let thing = 1 + } else { + 1 + } + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::LastExpressionIsAssignment { .. })) + )) +} + +#[test] +fn assignement_last_expr_if_final_else() { + let source_code = r#" + pub fn foo() { + if True { + 1 + } else { + let thing = 1 + } + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::LastExpressionIsAssignment { .. })) + )) +} + +#[test] +fn assignment_last_expr_logical_chain() { + let source_code = r#" + pub fn foo() -> Bool { + and { + expect 1 + 1 == 2, + True, + 2 > 0, + or { + expect True, + False, + } + } + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::LastExpressionIsAssignment { .. })) + )) +} + #[test] fn if_scoping() { let source_code = r#" @@ -1668,8 +1769,7 @@ fn pipe_wrong_arity_fully_saturated_return_fn() { fn fuzzer_ok_basic() { let source_code = r#" fn int() -> Fuzzer { todo } - - test prop(n via int()) { todo } + test prop(n via int()) { True } "#; assert!(check(parse(source_code)).is_ok()); @@ -1679,8 +1779,7 @@ fn fuzzer_ok_basic() { fn fuzzer_ok_explicit() { let source_code = r#" fn int(prng: PRNG) -> Option<(PRNG, Int)> { todo } - - test prop(n via int) { todo } + test prop(n via int) { Void } "#; assert!(check(parse(source_code)).is_ok()); @@ -1692,7 +1791,7 @@ fn fuzzer_ok_list() { fn int() -> Fuzzer { todo } fn list(a: Fuzzer) -> Fuzzer> { todo } - test prop(xs via list(int())) { todo } + test prop(xs via list(int())) { True } "#; assert!(check(parse(source_code)).is_ok()); @@ -2906,24 +3005,19 @@ fn recover_no_assignment_when_clause() { let source_code = r#" pub fn main(foo) { when foo is { - [] -> let bar = foo + [] -> Void [x, ..] -> expect _: Int = x } } "#; - let (warnings, _) = check(parse(source_code)).unwrap(); - - assert!(matches!( - &warnings[..], - [Warning::UnusedVariable { name, .. }] if name == "bar", - )) + assert!(check(parse(source_code)).is_ok()); } #[test] fn recover_no_assignment_fn_if_then_else() { let source_code = r#" - pub fn foo(weird_maths) -> Bool { + pub fn foo(weird_maths) -> Void { if weird_maths { expect 1 == 2 } else { @@ -2936,20 +3030,38 @@ fn recover_no_assignment_fn_if_then_else() { } #[test] -fn recover_no_assignment_logical_chain_op() { +fn test_return_explicit_void() { let source_code = r#" - pub fn foo() -> Bool { - and { - expect 1 + 1 == 2, - True, - 2 > 0, - or { - expect True, - False, - } - } + test foo() { + Void } "#; assert!(check(parse(source_code)).is_ok()); } + +#[test] +fn test_return_implicit_void() { + let source_code = r#" + test foo() { + let data: Data = 42 + expect _: Int = data + } + "#; + + assert!(check(parse(source_code)).is_ok()); +} + +#[test] +fn test_return_illegal() { + let source_code = r#" + test foo() { + 42 + } + "#; + + assert!(matches!( + check(parse(source_code)), + Err((_, Error::IllegalTestType { .. })) + )) +} diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 0fe9293d..8f4fa449 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -2,7 +2,7 @@ use super::Type; use crate::{ ast::{Annotation, BinOp, CallArg, LogicalOpChainKind, Span, UntypedFunction, UntypedPattern}, error::ExtraData, - expr::{self, AssignmentPattern, UntypedExpr}, + expr::{self, AssignmentPattern, UntypedAssignmentKind, UntypedExpr}, format::Formatter, levenshtein, pretty::Documentable, @@ -472,6 +472,7 @@ If you really meant to return that last expression, try to replace it with the f location: Span, expr: expr::UntypedExpr, patterns: Vec1, + kind: UntypedAssignmentKind, }, #[error( @@ -1025,7 +1026,7 @@ The best thing to do from here is to remove it."#))] }, #[error("I caught a test with too many arguments.\n")] - #[diagnostic(code("illegal::test_arity"))] + #[diagnostic(code("illegal::test::arity"))] #[diagnostic(help( "Tests are allowed to have 0 or 1 argument, but no more. Here I've found a test definition with {count} arguments. If you need to provide multiple values to a test, use a Record or a Tuple.", ))] @@ -1035,6 +1036,18 @@ The best thing to do from here is to remove it."#))] location: Span, }, + #[error("I caught a test with an illegal return type.\n")] + #[diagnostic(code("illegal::test::return"))] + #[diagnostic(help( + "Tests must return either {Bool} or {Void}. Note that `expect` assignment are implicitly typed {Void} (and thus, may be the last expression of a test).", + Bool = "Bool".if_supports_color(Stderr, |s| s.cyan()), + Void = "Void".if_supports_color(Stderr, |s| s.cyan()), + ))] + IllegalTestType { + #[label("expected Bool or Void")] + location: Span, + }, + #[error("I choked on a generic type left in an outward-facing interface.\n")] #[diagnostic(code("illegal::generic_in_abi"))] #[diagnostic(help( @@ -1104,6 +1117,7 @@ impl ExtraData for Error { | Error::UpdateMultiConstructorType { .. } | Error::ValidatorImported { .. } | Error::IncorrectTestArity { .. } + | Error::IllegalTestType { .. } | Error::GenericLeftAtBoundary { .. } | Error::UnexpectedMultiPatternAssignment { .. } | Error::ExpectOnOpaqueType { .. } diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index 7c0041fc..1df80e83 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -1803,17 +1803,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> { let mut typed_expressions = vec![]; for expression in expressions { - let typed_expression = if let Some(filler) = recover_from_no_assignment( - assert_no_assignment(&expression), - expression.location(), - )? { - TypedExpr::Sequence { - location: expression.location(), - expressions: vec![self.infer(expression)?, filler], - } - } else { - self.infer(expression)? - }; + assert_no_assignment(&expression)?; + let typed_expression = self.infer(expression)?; self.unify( bool(), @@ -2545,24 +2536,32 @@ fn recover_from_no_assignment( result: Result<(), Error>, span: Span, ) -> Result, Error> { - if let Err(Error::LastExpressionIsAssignment { patterns, .. }) = result { - match patterns.first().pattern.get_bool() { - Some(expected) if patterns.len() == 1 => Ok(Some(TypedExpr::bool(expected, span))), - _ => Ok(Some(TypedExpr::void(span))), + if let Err(Error::LastExpressionIsAssignment { + ref patterns, + ref kind, + .. + }) = result + { + if matches!(kind, AssignmentKind::Expect { ..} if patterns.len() == 1) { + return Ok(Some(TypedExpr::void(span))); } - } else { - result.map(|()| None) } + + result.map(|()| None) } fn assert_no_assignment(expr: &UntypedExpr) -> Result<(), Error> { match expr { UntypedExpr::Assignment { - value, patterns, .. + value, + patterns, + kind, + .. } => Err(Error::LastExpressionIsAssignment { location: expr.location(), expr: *value.clone(), patterns: patterns.clone(), + kind: *kind, }), UntypedExpr::Trace { then, .. } => assert_no_assignment(then), UntypedExpr::Fn { .. } diff --git a/crates/aiken-lang/src/tipo/infer.rs b/crates/aiken-lang/src/tipo/infer.rs index 8f9a37a6..a3e0ef6b 100644 --- a/crates/aiken-lang/src/tipo/infer.rs +++ b/crates/aiken-lang/src/tipo/infer.rs @@ -392,12 +392,25 @@ fn infer_definition( let typed_f = infer_function(&f.into(), module_name, hydrators, environment, tracing)?; - environment.unify( + let is_bool = environment.unify( typed_f.return_type.clone(), builtins::bool(), typed_f.location, false, - )?; + ); + + let is_void = environment.unify( + typed_f.return_type.clone(), + builtins::void(), + typed_f.location, + false, + ); + + if is_bool.or(is_void).is_err() { + return Err(Error::IllegalTestType { + location: typed_f.location, + }); + } Ok(Definition::Test(Function { doc: typed_f.doc, diff --git a/crates/uplc/src/machine/eval_result.rs b/crates/uplc/src/machine/eval_result.rs index 572d2276..364d4869 100644 --- a/crates/uplc/src/machine/eval_result.rs +++ b/crates/uplc/src/machine/eval_result.rs @@ -39,7 +39,11 @@ impl EvalResult { } else { self.result.is_err() || matches!(self.result, Ok(Term::Error)) - || !matches!(self.result, Ok(Term::Constant(ref con)) if matches!(con.as_ref(), Constant::Bool(true))) + || !matches!( + self.result, + Ok(Term::Constant(ref con)) + if matches!(con.as_ref(), Constant::Bool(true)) || matches!(con.as_ref(), Constant::Unit) + ) } } From 70e760cbaa6887c66b734a827b06664a482766be Mon Sep 17 00:00:00 2001 From: KtorZ Date: Wed, 21 Aug 2024 14:36:33 +0200 Subject: [PATCH 3/3] Fill-in CHANGELOG regarding last expect and tests now accepting Void. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6bdf491..19318061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,10 @@ - **aiken-lang**: remove warning on discarded expect, allowing to keep 'side-effects' when necessary. See [#967](https://github.com/aiken-lang/aiken/pull/967). @KtorZ +- **aiken-lang**: allow expect as last (or only) expression in function body, when clauses and if branches. Such expressions unify with `Void`. See [#1000](https://github.com/aiken-lang/aiken/pull/1000). @KtorZ + +- **aiken-lang**: allow tests to return `Void`. Tests that return `Void` are treated the same as tests that return `True`. See [#1000](https://github.com/aiken-lang/aiken/pull/1000). @KtorZ + - **aiken-lang**: rework traces to be (1) variadic, (2) generic in its arguments and (3) structured. @KtorZ In more details: