From 3c7663cd3c1363340fe4b548671a5b4f4f242364 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 11 Feb 2023 16:20:28 +0100 Subject: [PATCH 1/5] Basic exhaustivness check on list patterns Before that commit, the type-checker would allow unsafe list patterns such as: ``` let [x] = xs when xs is { [x] -> ... [x, ..] -> ... } ``` This is quite unsafe and can lead to confusing situations. Now at least the compiler warns about this. It isn't perfect though, especially in the presence of clause guards. But that's a start. --- crates/aiken-lang/src/tipo/environment.rs | 73 +++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/crates/aiken-lang/src/tipo/environment.rs b/crates/aiken-lang/src/tipo/environment.rs index 8b2f265a..6bcea04f 100644 --- a/crates/aiken-lang/src/tipo/environment.rs +++ b/crates/aiken-lang/src/tipo/environment.rs @@ -1386,6 +1386,10 @@ impl<'a> Environment<'a> { Some(module.clone()) }; + if type_name == "List" && module.is_empty() { + return self.check_list_pattern_exhaustiveness(patterns); + } + if let Ok(constructors) = self.get_constructors_for_type(&m, type_name, location) { let mut unmatched_constructors: HashSet = constructors.iter().cloned().collect(); @@ -1426,6 +1430,75 @@ impl<'a> Environment<'a> { } } + pub fn check_list_pattern_exhaustiveness( + &mut self, + patterns: Vec>>, + ) -> Result<(), Vec> { + let mut cover_empty = false; + let mut cover_tail = false; + + let patterns = patterns.iter().map(|p| match p { + Pattern::Assign { pattern, .. } => pattern, + _ => p, + }); + + // TODO: We could also warn on redundant patterns. As soon as we've matched the entire + // list, any new pattern is redundant. For example: + // + // when xs is { + // [] => ... + // [x, ..] => ... + // [y] => ... + // } + // + // That last pattern is actually redundant / unreachable. + for p in patterns { + match p { + Pattern::Var { .. } => { + cover_empty = true; + cover_tail = true; + } + Pattern::Discard { .. } => { + cover_empty = true; + cover_tail = true; + } + Pattern::List { elements, tail, .. } => { + if elements.is_empty() { + cover_empty = true; + } + match tail { + None => {} + Some(p) => match **p { + Pattern::Discard { .. } => { + cover_tail = true; + } + Pattern::Var { .. } => { + cover_tail = true; + } + _ => { + unreachable!() + } + }, + } + } + _ => {} + } + } + + if cover_empty && cover_tail { + Ok(()) + } else { + let mut missing = vec![]; + if !cover_empty { + missing.push("[]".to_owned()); + } + if !cover_tail { + missing.push("[_, ..]".to_owned()); + } + Err(missing) + } + } + /// Lookup constructors for type in the current scope. /// pub fn get_constructors_for_type( From 56e90fba21a26b8fd1677322b9325756485ef544 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 11 Feb 2023 16:24:56 +0100 Subject: [PATCH 2/5] Add missing newlines to 'join' in error messages. --- crates/aiken-lang/src/tipo/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 28f61c45..5fa694a7 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -424,7 +424,7 @@ In this particular instance, the following cases are unmatched: .iter() .map(|s| format!("─▶ {s}")) .collect::>() - .join("") + .join("\n") ))] NotExhaustivePatternMatch { #[label("{}", if *is_let { "use when/is" } else { "non-exhaustive" })] @@ -1247,5 +1247,5 @@ fn format_suggestion(sample: &UntypedExpr) -> String { } }) .collect::>() - .join("") + .join("\n") } From 6649821200bfb03e11109346b5a31e6828f47e21 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 11 Feb 2023 16:54:49 +0100 Subject: [PATCH 3/5] Add type-checker sanity tests for list patterns. --- crates/aiken-lang/src/tests/check.rs | 128 +++++++++++++++++++++++++++ crates/aiken-lang/src/tests/mod.rs | 1 + 2 files changed, 129 insertions(+) create mode 100644 crates/aiken-lang/src/tests/check.rs diff --git a/crates/aiken-lang/src/tests/check.rs b/crates/aiken-lang/src/tests/check.rs new file mode 100644 index 00000000..f547261b --- /dev/null +++ b/crates/aiken-lang/src/tests/check.rs @@ -0,0 +1,128 @@ +use crate::{ + ast::{ModuleKind, TypedModule, UntypedModule}, + builtins, parser, + tipo::error::{Error, Warning}, + IdGenerator, +}; +use std::collections::HashMap; + +fn parse(source_code: &str) -> UntypedModule { + let kind = ModuleKind::Lib; + let (ast, _) = parser::module(source_code, kind).expect("Failed to parse module"); + ast +} + +fn check(ast: UntypedModule) -> Result, Error)> { + let kind = ModuleKind::Lib; + + let id_gen = IdGenerator::new(); + + let mut warnings = vec![]; + + let mut module_types = HashMap::new(); + module_types.insert("aiken".to_string(), builtins::prelude(&id_gen)); + module_types.insert("aiken/builtin".to_string(), builtins::plutus(&id_gen)); + + let result = ast.infer(&id_gen, kind, "test/project", &module_types, &mut warnings); + + result.map_err(|e| (warnings, e)) +} + +#[test] +fn list_pattern_1() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let [x] = xs + x == 1 + } + "#; + assert!(matches!( + check(parse(source_code)), + Err((_, Error::NotExhaustivePatternMatch { .. })) + )) +} + +#[test] +fn list_pattern_2() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let x = when xs is { + [x] -> x + } + x == 1 + } + "#; + assert!(matches!( + check(parse(source_code)), + Err((_, Error::NotExhaustivePatternMatch { .. })) + )) +} + +#[test] +fn list_pattern_3() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let x = when xs is { + [x] -> x + [x, ..] -> x + } + x == 1 + } + "#; + assert!(matches!( + check(parse(source_code)), + Err((_, Error::NotExhaustivePatternMatch { .. })) + )) +} + +#[test] +fn list_pattern_4() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let x = when xs is { + [] -> 1 + [x] -> x + [x, ..] if x > 10 -> x + } + x == 1 + } + "#; + assert!(matches!( + check(parse(source_code)), + Err((_, Error::NotExhaustivePatternMatch { .. })) + )) +} + +#[test] +fn list_pattern_5() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let x = when xs is { + [] -> 1 + [_, ..] -> 1 + } + x == 1 + } + "#; + assert!(check(parse(source_code)).is_ok()) +} + +#[test] +fn list_pattern_6() { + let source_code = r#" + test foo() { + let xs = [1, 2, 3] + let x = when xs is { + [x, ..] -> 1 + _ -> 1 + } + x == 1 + } + "#; + assert!(check(parse(source_code)).is_ok()) +} diff --git a/crates/aiken-lang/src/tests/mod.rs b/crates/aiken-lang/src/tests/mod.rs index a8f5375c..0b33bec1 100644 --- a/crates/aiken-lang/src/tests/mod.rs +++ b/crates/aiken-lang/src/tests/mod.rs @@ -1,3 +1,4 @@ +mod check; mod format; mod lexer; mod parser; From 2e8fd6e1c220ce797d7c896ea38b16133ca39cc4 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 11 Feb 2023 16:57:14 +0100 Subject: [PATCH 4/5] Remove patterns on 'String' There's arguably no use case ever for that in the context of on-chain Plutus. Strings are really just meant to be used for tracing. They aren't meant to be manipulated as heavily as in classic programming languages. --- crates/aiken-lang/src/ast.rs | 6 ------ crates/aiken-lang/src/format.rs | 2 -- crates/aiken-lang/src/parser.rs | 6 ------ crates/aiken-lang/src/tipo/pattern.rs | 8 +------- crates/aiken-lang/src/uplc.rs | 9 +-------- 5 files changed, 2 insertions(+), 29 deletions(-) diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index 098b80f2..b137c7f2 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -717,11 +717,6 @@ pub enum Pattern { value: String, }, - String { - location: Span, - value: String, - }, - /// The creation of a variable. /// e.g. `assert [this_is_a_var, .._] = x` Var { @@ -776,7 +771,6 @@ impl Pattern { | Pattern::Var { location, .. } | Pattern::List { location, .. } | Pattern::Discard { location, .. } - | Pattern::String { location, .. } | Pattern::Tuple { location, .. } // | Pattern::Concatenate { location, .. } | Pattern::Constructor { location, .. } => *location, diff --git a/crates/aiken-lang/src/format.rs b/crates/aiken-lang/src/format.rs index 1fe00e8f..64ae9561 100644 --- a/crates/aiken-lang/src/format.rs +++ b/crates/aiken-lang/src/format.rs @@ -1454,8 +1454,6 @@ impl<'comments> Formatter<'comments> { let doc = match pattern { Pattern::Int { value, .. } => value.to_doc(), - Pattern::String { value, .. } => self.string(value), - Pattern::Var { name, .. } => name.to_doc(), Pattern::Assign { name, pattern, .. } => { diff --git a/crates/aiken-lang/src/parser.rs b/crates/aiken-lang/src/parser.rs index c9944a15..d3df67c3 100644 --- a/crates/aiken-lang/src/parser.rs +++ b/crates/aiken-lang/src/parser.rs @@ -1689,12 +1689,6 @@ pub fn pattern_parser() -> impl Parser value}.map_with_span(|value, span| { - ast::UntypedPattern::String { - location: span, - value, - } - }), select! {Token::Int {value} => value}.map_with_span(|value, span| { ast::UntypedPattern::Int { location: span, diff --git a/crates/aiken-lang/src/tipo/pattern.rs b/crates/aiken-lang/src/tipo/pattern.rs index f77bf88d..3f596dd3 100644 --- a/crates/aiken-lang/src/tipo/pattern.rs +++ b/crates/aiken-lang/src/tipo/pattern.rs @@ -16,7 +16,7 @@ use super::{ }; use crate::{ ast::{CallArg, Pattern, Span, TypedPattern, UntypedMultiPattern, UntypedPattern}, - builtins::{int, list, string, tuple}, + builtins::{int, list, tuple}, }; pub struct PatternTyper<'a, 'b> { @@ -286,12 +286,6 @@ impl<'a, 'b> PatternTyper<'a, 'b> { Ok(Pattern::Int { location, value }) } - Pattern::String { location, value } => { - self.environment.unify(tipo, string(), location, false)?; - - Ok(Pattern::String { location, value }) - } - Pattern::List { location, elements, diff --git a/crates/aiken-lang/src/uplc.rs b/crates/aiken-lang/src/uplc.rs index 9c035e33..1c72c5f2 100644 --- a/crates/aiken-lang/src/uplc.rs +++ b/crates/aiken-lang/src/uplc.rs @@ -871,7 +871,6 @@ impl<'a> CodeGenerator<'a> { pattern_vec.append(values); } - Pattern::String { .. } => todo!("String matching in whens not yet implemented"), Pattern::Var { name, .. } => { pattern_vec.push(Air::Void { scope: scope.clone(), @@ -1060,7 +1059,6 @@ impl<'a> CodeGenerator<'a> { ) { match pattern { Pattern::Int { .. } => unreachable!(), - Pattern::String { .. } => unreachable!(), Pattern::Var { .. } => unreachable!(), Pattern::Assign { .. } => todo!("Nested assign not yet implemented"), Pattern::Discard { .. } => { @@ -1537,7 +1535,6 @@ impl<'a> CodeGenerator<'a> { } Pattern::Assign { .. } => todo!("Nested assign is not yet done"), Pattern::Int { .. } => unimplemented!(), - Pattern::String { .. } => unimplemented!(), } } @@ -1569,7 +1566,7 @@ impl<'a> CodeGenerator<'a> { ) } match pattern { - Pattern::Int { .. } | Pattern::String { .. } => unreachable!(), + Pattern::Int { .. } => unreachable!(), Pattern::Var { name, .. } => { pattern_vec.push(Air::Let { name: name.clone(), @@ -1690,7 +1687,6 @@ impl<'a> CodeGenerator<'a> { ) { match pattern { Pattern::Int { .. } => todo!(), - Pattern::String { .. } => todo!(), Pattern::Var { .. } => todo!(), Pattern::Assign { .. } => todo!(), Pattern::Discard { .. } => todo!(), @@ -1736,7 +1732,6 @@ impl<'a> CodeGenerator<'a> { ); } Pattern::Int { .. } => todo!(), - Pattern::String { .. } => todo!(), Pattern::Assign { .. } => todo!(), Pattern::Discard { .. } => { names.push("_".to_string()); @@ -1904,7 +1899,6 @@ impl<'a> CodeGenerator<'a> { ) { match pattern { Pattern::Int { .. } => unreachable!(), - Pattern::String { .. } => unreachable!(), Pattern::Var { name, .. } => { pattern_vec.append(value_vec); @@ -2651,7 +2645,6 @@ impl<'a> CodeGenerator<'a> { (false, tuple_name) } Pattern::Int { .. } => todo!(), - Pattern::String { .. } => todo!(), Pattern::Assign { .. } => todo!(), }; From b83a247ff7a752d16c9396c66ab772c99def2674 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 11 Feb 2023 17:00:32 +0100 Subject: [PATCH 5/5] Add slightly more informative note for list pattern on int. --- crates/aiken-lang/src/uplc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/aiken-lang/src/uplc.rs b/crates/aiken-lang/src/uplc.rs index 1c72c5f2..0fd2dd02 100644 --- a/crates/aiken-lang/src/uplc.rs +++ b/crates/aiken-lang/src/uplc.rs @@ -1534,7 +1534,7 @@ impl<'a> CodeGenerator<'a> { Some(item_name) } Pattern::Assign { .. } => todo!("Nested assign is not yet done"), - Pattern::Int { .. } => unimplemented!(), + Pattern::Int { .. } => todo!("Nested pattern-match on integers isn't implemented yet. Use when clause-guard as an alternative, or break down the pattern."), } }