Skip to content

Commit

Permalink
feat: don't eagerly error on cast expressions (#5635)
Browse files Browse the repository at this point in the history
# 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 <jake@aztecprotocol.com>
  • Loading branch information
asterite and jfecher authored Jul 31, 2024
1 parent efef6b4 commit 0ca5d9d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 8 additions & 3 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 => {
Expand Down
37 changes: 37 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, U>(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<T, U>(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 { .. })
));
}

0 comments on commit 0ca5d9d

Please sign in to comment.