From 0ca5d9d7b389d24cf48fa62a29cb437b1855438a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 31 Jul 2024 12:38:53 -0300 Subject: [PATCH] feat: don't eagerly error on cast expressions (#5635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description ## Problem Resolves #4994 ## Summary Before this PR, if you had something like: ```rust x as Field ``` and x's type was an unbound type variable, the compiler would immediately produce an error. In this PR that check is delayed. In order to do that, the cast can only succeed if `x` is an integer, a field... or a bool. We don't have a polymorphic type that's one of those, so this PR introduces that. (note: I think what I'm doing here is correct, but let me know if it's not!) Then, because we now have three different polymorphic types, I put them all into an enum, mainly because some of this polymorphic handling was sometimes duplicated, or almost the same with slight changes. ## Additional Context I'm not sure I covered all the scenarios where `IntegerOrFieldOrBool` should be handled. I'm not even sure I handled correctly the cases I needed to handle. This code now works fine: ```rust fn main() { let a: [u8; 10] = [1; 10]; let b: [Field; 10] = a.map(|x| x as Field); } ``` Also this one: ```rust fn main() { let a: [u8; 10] = [true; 10]; let b: [Field; 10] = a.map(|x| x as Field); } ``` However, this code correctly fails to compile, but the error is a bit strange: ```rust fn main() { let a = ["a"; 10]; let b: [Field; 10] = a.map(|x| x as Field); } ``` The error is: ``` error: Expected type fn(str<1>) -> _ with env _, found type fn(Integer | Field | bool) -> Field ┌─ src/main.nr:3:32 │ 3 │ let b: [Field; 10] = a.map(|x| x as Field); │ -------------- ``` but maybe that's expected? (I also don't know what type we could show here instead of `Integer | Field | bool`) ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: jfecher --- .../src/elaborator/expressions.rs | 2 +- .../noirc_frontend/src/elaborator/types.rs | 11 ++++-- compiler/noirc_frontend/src/tests.rs | 37 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 295297cc738..6e2756f0301 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -553,7 +553,7 @@ impl<'context> Elaborator<'context> { fn elaborate_cast(&mut self, cast: CastExpression, span: Span) -> (HirExpression, Type) { let (lhs, lhs_type) = self.elaborate_expression(cast.lhs); let r#type = self.resolve_type(cast.r#type); - let result = self.check_cast(lhs_type, &r#type, span); + let result = self.check_cast(&lhs_type, &r#type, span); let expr = HirExpression::Cast(HirCastExpression { lhs, r#type }); (expr, result) } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index c134820811e..0973e592c1e 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -779,7 +779,7 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn check_cast(&mut self, from: Type, to: &Type, span: Span) -> Type { + pub(super) fn check_cast(&mut self, from: &Type, to: &Type, span: Span) -> Type { match from.follow_bindings() { Type::Integer(..) | Type::FieldElement @@ -788,8 +788,13 @@ impl<'context> Elaborator<'context> { | Type::Bool => (), Type::TypeVariable(_, _) => { - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); - return Type::Error; + // NOTE: in reality the expected type can also include bool, but for the compiler's simplicity + // we only allow integer types. If a bool is in `from` it will need an explicit type annotation. + let expected = Type::polymorphic_integer_or_field(self.interner); + self.unify(from, &expected, || TypeCheckError::InvalidCast { + from: from.clone(), + span, + }); } Type::Error => return Type::Error, from => { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 91644ff4470..8ce430b6e48 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2809,3 +2809,40 @@ fn uses_self_type_in_trait_where_clause() { assert_eq!(method_name, "trait_func"); } + +#[test] +fn do_not_eagerly_error_on_cast_on_type_variable() { + let src = r#" + pub fn foo(x: T, f: fn(T) -> U) -> U { + f(x) + } + + fn main() { + let x: u8 = 1; + let _: Field = foo(x, |x| x as Field); + } + "#; + assert_no_errors(src); +} + +#[test] +fn error_on_cast_over_type_variable() { + let src = r#" + pub fn foo(x: T, f: fn(T) -> U) -> U { + f(x) + } + + fn main() { + let x = "a"; + let _: Field = foo(x, |x| x as Field); + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) + )); +}