Skip to content

Commit

Permalink
feat!: kind size checks (#6137)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

* Need to check that type constants fit into their `Kind`'s
* The sizes of results from `op.function` and `evaluate_to_u32` are
unchecked

## Summary\*

Split out from #6094
- Some parts only work with its additional kind information
- Several follow up issues, including:
  * #6247
  * #6245
  * #6238

## Additional Context

TODO:
- [x] Add this test and/or similar execution tests unless we already
have a similar one (sanity-check that global `Field` arithmetic works
past `u32::MAX`)

```noir
// 2^32 + 1
global A: Field = 4294967297;
global B: Field = 4294967297;
global C: Field = A + B;

fn main() {
    // 2 * (2^32 + 1)
    assert(C == 8589934594);

    let mut leading_zeroes = 0;
    let mut stop = false;
    let bits: [u1; 64] = C.to_be_bits();
    for i in 0..64 {
        if (bits[i] == 0) & !stop {
            leading_zeroes += 1;
        } else {
            stop = true;
        }
    }
    let size = 64 - leading_zeroes;

    // 8589934594 has 34 bits
    assert(size == 34);
}
```

## 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: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
4 people authored Oct 9, 2024
1 parent 7cc7197 commit 6e40f62
Show file tree
Hide file tree
Showing 20 changed files with 767 additions and 217 deletions.
8 changes: 5 additions & 3 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub use visitor::Visitor;
pub use expression::*;
pub use function::*;

use acvm::FieldElement;
pub use docs::*;
use noirc_errors::Span;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -219,7 +220,7 @@ pub struct UnaryRhsMethodCall {
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedTypeExpression {
Variable(Path),
Constant(u32, Span),
Constant(FieldElement, Span),
BinaryOperation(
Box<UnresolvedTypeExpression>,
BinaryTypeOperator,
Expand Down Expand Up @@ -421,12 +422,13 @@ impl UnresolvedTypeExpression {
fn from_expr_helper(expr: Expression) -> Result<UnresolvedTypeExpression, Expression> {
match expr.kind {
ExpressionKind::Literal(Literal::Integer(int, _)) => match int.try_to_u32() {
Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)),
Some(int) => Ok(UnresolvedTypeExpression::Constant(int.into(), expr.span)),
None => Err(expr),
},
ExpressionKind::Variable(path) => Ok(UnresolvedTypeExpression::Variable(path)),
ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => {
let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span));
let lhs =
Box::new(UnresolvedTypeExpression::Constant(FieldElement::zero(), expr.span));
let rhs = Box::new(UnresolvedTypeExpression::from_expr_helper(prefix.rhs)?);
let op = BinaryTypeOperator::Subtraction;
Ok(UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, expr.span))
Expand Down
11 changes: 6 additions & 5 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use acvm::{AcirField, FieldElement};
use iter_extended::vecmap;
use noirc_errors::{Location, Span};
use regex::Regex;
Expand Down Expand Up @@ -161,7 +162,7 @@ impl<'context> Elaborator<'context> {
(Lit(int), self.polymorphic_integer_or_field())
}
Literal::Str(str) | Literal::RawStr(str, _) => {
let len = Type::Constant(str.len() as u32, Kind::u32());
let len = Type::Constant(str.len().into(), Kind::u32());
(Lit(HirLiteral::Str(str)), Type::String(Box::new(len)))
}
Literal::FmtStr(str) => self.elaborate_fmt_string(str, span),
Expand Down Expand Up @@ -203,15 +204,15 @@ impl<'context> Elaborator<'context> {
elem_id
});

let length = Type::Constant(elements.len() as u32, Kind::u32());
let length = Type::Constant(elements.len().into(), Kind::u32());
(HirArrayLiteral::Standard(elements), first_elem_type, length)
}
ArrayLiteral::Repeated { repeated_element, length } => {
let span = length.span;
let length =
UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(|error| {
self.push_err(ResolverError::ParserError(Box::new(error)));
UnresolvedTypeExpression::Constant(0, span)
UnresolvedTypeExpression::Constant(FieldElement::zero(), span)
});

let length = self.convert_expression_type(length, &Kind::u32(), span);
Expand Down Expand Up @@ -267,7 +268,7 @@ impl<'context> Elaborator<'context> {
}
}

let len = Type::Constant(str.len() as u32, Kind::u32());
let len = Type::Constant(str.len().into(), Kind::u32());
let typ = Type::FmtString(Box::new(len), Box::new(Type::Tuple(capture_types)));
(HirExpression::Literal(HirLiteral::FmtStr(str, fmt_str_idents)), typ)
}
Expand Down Expand Up @@ -676,7 +677,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, &lhs_type, &r#type, span);
let expr = HirExpression::Cast(HirCastExpression { lhs, r#type });
(expr, result)
}
Expand Down
56 changes: 50 additions & 6 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,17 @@ impl<'context> Elaborator<'context> {
.map(|let_statement| Kind::Numeric(Box::new(let_statement.r#type)))
.unwrap_or(Kind::u32());

Some(Type::Constant(self.eval_global_as_array_length(id, path), kind))
// TODO(https://github.com/noir-lang/noir/issues/6238):
// support non-u32 generics here
if !kind.unifies(&Kind::u32()) {
let error = TypeCheckError::EvaluatedGlobalIsntU32 {
expected_kind: Kind::u32().to_string(),
expr_kind: kind.to_string(),
expr_span: path.span(),
};
self.push_err(error);
}
Some(Type::Constant(self.eval_global_as_array_length(id, path).into(), kind))
}
_ => None,
}
Expand Down Expand Up @@ -833,26 +843,60 @@ impl<'context> Elaborator<'context> {
}
}

pub(super) fn check_cast(&mut self, from: &Type, to: &Type, span: Span) -> Type {
match from.follow_bindings() {
Type::Integer(..) | Type::FieldElement | Type::Bool => (),
pub(super) fn check_cast(
&mut self,
from_expr_id: &ExprId,
from: &Type,
to: &Type,
span: Span,
) -> Type {
let from_follow_bindings = from.follow_bindings();

Type::TypeVariable(var) if var.is_integer() || var.is_integer_or_field() => (),
let from_value_opt = match self.interner.expression(from_expr_id) {
HirExpression::Literal(HirLiteral::Integer(int, false)) => Some(int),

// TODO(https://github.com/noir-lang/noir/issues/6247):
// handle negative literals
_ => None,
};

let from_is_polymorphic = match from_follow_bindings {
Type::Integer(..) | Type::FieldElement | Type::Bool => false,

Type::TypeVariable(ref var) if var.is_integer() || var.is_integer_or_field() => true,
Type::TypeVariable(_) => {
// 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 = self.polymorphic_integer_or_field();
self.unify(from, &expected, || TypeCheckError::InvalidCast {
from: from.clone(),
span,
reason: "casting from a non-integral type is unsupported".into(),
});
true
}
Type::Error => return Type::Error,
from => {
self.push_err(TypeCheckError::InvalidCast { from, span });
let reason = "casting from this type is unsupported".into();
self.push_err(TypeCheckError::InvalidCast { from, span, reason });
return Type::Error;
}
};

// TODO(https://github.com/noir-lang/noir/issues/6247):
// handle negative literals
// when casting a polymorphic value to a specifically sized type,
// check that it fits or throw a warning
if let (Some(from_value), Some(to_maximum_size)) =
(from_value_opt, to.integral_maximum_size())
{
if from_is_polymorphic && from_value > to_maximum_size {
let from = from.clone();
let to = to.clone();
let reason = format!("casting untyped value ({from_value}) to a type with a maximum size ({to_maximum_size}) that's smaller than it");
// we warn that the 'to' type is too small for the value
self.push_err(TypeCheckError::DownsizingCast { from, to, span, reason });
}
}

match to {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ impl HirArrayLiteral {
let repeated_element = Box::new(repeated_element.to_display_ast(interner));
let length = match length {
Type::Constant(length, _kind) => {
let literal = Literal::Integer((*length as u128).into(), false);
let literal = Literal::Integer(*length, false);
let expr_kind = ExpressionKind::Literal(literal);
Box::new(Expression::new(expr_kind, span))
}
Expand Down
Loading

0 comments on commit 6e40f62

Please sign in to comment.