Skip to content

Commit

Permalink
feat!: separating out array and slice types in the AST (#4504)
Browse files Browse the repository at this point in the history
# Description

Replacement for #4232. Holding off
on closing it to possibly port fixes from it, but wanted to start fresh
because this approach is quite different.

## Problem\*

Resolves #4220

## Summary\*

- Removes `NotConstant`
- Separate syntax for slice literals: `&[..]`
- Separate AST/type constructors for slices

Notes:
- I removed several broken links that were blocking updating the docs:
+ A few were to a missing `reference` docs folder, likely from an
in-progress branch
+ One was to some `base_order_curve`, which has been moved to reference
the `BigInt` struct

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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: Jake Fecher <[email protected]>
Co-authored-by: jfecher <[email protected]>
  • Loading branch information
3 people authored and TomAFrench committed Apr 3, 2024
1 parent 90b2888 commit 931cf2e
Show file tree
Hide file tree
Showing 64 changed files with 763 additions and 304 deletions.
14 changes: 10 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,17 +199,23 @@ impl<'a> FunctionContext<'a> {
ast::Type::Array(_, _) => {
self.codegen_array_checked(elements, typ[0].clone())?
}
_ => unreachable!("ICE: unexpected array literal type, got {}", array.typ),
})
}
ast::Literal::Slice(array) => {
let elements =
try_vecmap(&array.contents, |element| self.codegen_expression(element))?;

let typ = Self::convert_type(&array.typ).flatten();
Ok(match array.typ {
ast::Type::Slice(_) => {
let slice_length =
self.builder.length_constant(array.contents.len() as u128);
let slice_contents =
self.codegen_array_checked(elements, typ[1].clone())?;
Tree::Branch(vec![slice_length.into(), slice_contents])
}
_ => unreachable!(
"ICE: array literal type must be an array or a slice, but got {}",
array.typ
),
_ => unreachable!("ICE: unexpected slice literal type, got {}", array.typ),
})
}
ast::Literal::Integer(value, typ, location) => {
Expand Down
19 changes: 19 additions & 0 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ impl ExpressionKind {
}))
}

pub fn slice(contents: Vec<Expression>) -> ExpressionKind {
ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Standard(contents)))
}

pub fn repeated_slice(repeated_element: Expression, length: Expression) -> ExpressionKind {
ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Repeated {
repeated_element: Box::new(repeated_element),
length: Box::new(length),
}))
}

pub fn integer(contents: FieldElement) -> ExpressionKind {
ExpressionKind::Literal(Literal::Integer(contents, false))
}
Expand Down Expand Up @@ -319,6 +330,7 @@ impl UnaryOp {
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum Literal {
Array(ArrayLiteral),
Slice(ArrayLiteral),
Bool(bool),
Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative
Str(String),
Expand Down Expand Up @@ -498,6 +510,13 @@ impl Display for Literal {
Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => {
write!(f, "[{repeated_element}; {length}]")
}
Literal::Slice(ArrayLiteral::Standard(elements)) => {
let contents = vecmap(elements, ToString::to_string);
write!(f, "&[{}]", contents.join(", "))
}
Literal::Slice(ArrayLiteral::Repeated { repeated_element, length }) => {
write!(f, "&[{repeated_element}; {length}]")
}
Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }),
Literal::Integer(integer, sign) => {
if *sign {
Expand Down
9 changes: 4 additions & 5 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ impl core::fmt::Display for IntegerBitSize {
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedTypeData {
FieldElement,
Array(Option<UnresolvedTypeExpression>, Box<UnresolvedType>), // [4]Witness = Array(4, Witness)
Array(UnresolvedTypeExpression, Box<UnresolvedType>), // [Field; 4] = Array(4, Field)
Slice(Box<UnresolvedType>),
Integer(Signedness, IntegerBitSize), // u32 = Integer(unsigned, ThirtyTwo)
Bool,
Expression(UnresolvedTypeExpression),
Expand Down Expand Up @@ -151,10 +152,8 @@ impl std::fmt::Display for UnresolvedTypeData {
use UnresolvedTypeData::*;
match self {
FieldElement => write!(f, "Field"),
Array(len, typ) => match len {
None => write!(f, "[{typ}]"),
Some(len) => write!(f, "[{typ}; {len}]"),
},
Array(len, typ) => write!(f, "[{typ}; {len}]"),
Slice(typ) => write!(f, "[{typ}]"),
Integer(sign, num_bits) => match sign {
Signedness::Signed => write!(f, "i{num_bits}"),
Signedness::Unsigned => write!(f, "u{num_bits}"),
Expand Down
57 changes: 35 additions & 22 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,13 +481,13 @@ impl<'a> Resolver<'a> {
FieldElement => Type::FieldElement,
Array(size, elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
let size = if size.is_none() {
Type::NotConstant
} else {
self.resolve_array_size(size, new_variables)
};
let size = self.resolve_array_size(Some(size), new_variables);
Type::Array(Box::new(size), elem)
}
Slice(elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
Type::Slice(elem)
}
Expression(expr) => self.convert_expression_type(expr),
Integer(sign, bits) => Type::Integer(sign, bits),
Bool => Type::Bool,
Expand Down Expand Up @@ -1084,7 +1084,6 @@ impl<'a> Resolver<'a> {
| Type::TypeVariable(_, _)
| Type::Constant(_)
| Type::NamedGeneric(_, _)
| Type::NotConstant
| Type::TraitAsType(..)
| Type::Forall(_, _) => (),

Expand All @@ -1095,6 +1094,10 @@ impl<'a> Resolver<'a> {
Self::find_numeric_generics_in_type(element_type, found);
}

Type::Slice(element_type) => {
Self::find_numeric_generics_in_type(element_type, found);
}

Type::Tuple(fields) => {
for field in fields {
Self::find_numeric_generics_in_type(field, found);
Expand Down Expand Up @@ -1402,27 +1405,37 @@ impl<'a> Resolver<'a> {
}
}

fn resolve_array_literal(&mut self, array_literal: ArrayLiteral) -> HirArrayLiteral {
match array_literal {
ArrayLiteral::Standard(elements) => {
let elements = vecmap(elements, |elem| self.resolve_expression(elem));
HirArrayLiteral::Standard(elements)
}
ArrayLiteral::Repeated { repeated_element, length } => {
let span = length.span;
let length =
UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(|error| {
self.errors.push(ResolverError::ParserError(Box::new(error)));
UnresolvedTypeExpression::Constant(0, span)
});

let length = self.convert_expression_type(length);
let repeated_element = self.resolve_expression(*repeated_element);

HirArrayLiteral::Repeated { repeated_element, length }
}
}
}

pub fn resolve_expression(&mut self, expr: Expression) -> ExprId {
let hir_expr = match expr.kind {
ExpressionKind::Literal(literal) => HirExpression::Literal(match literal {
Literal::Bool(b) => HirLiteral::Bool(b),
Literal::Array(ArrayLiteral::Standard(elements)) => {
let elements = vecmap(elements, |elem| self.resolve_expression(elem));
HirLiteral::Array(HirArrayLiteral::Standard(elements))
Literal::Array(array_literal) => {
HirLiteral::Array(self.resolve_array_literal(array_literal))
}
Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => {
let span = length.span;
let length = UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(
|error| {
self.errors.push(ResolverError::ParserError(Box::new(error)));
UnresolvedTypeExpression::Constant(0, span)
},
);

let length = self.convert_expression_type(length);
let repeated_element = self.resolve_expression(*repeated_element);

HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length })
Literal::Slice(array_literal) => {
HirLiteral::Slice(self.resolve_array_literal(array_literal))
}
Literal::Integer(integer, sign) => HirLiteral::Integer(integer, sign),
Literal::Str(str) => HirLiteral::Str(str),
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ pub enum TypeCheckError {
UnconstrainedReferenceToConstrained { span: Span },
#[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")]
UnconstrainedSliceReturnToConstrained { span: Span },
#[error("Slices must have constant length")]
NonConstantSliceLength { span: Span },
#[error("Only sized types may be used in the entry point to a program")]
InvalidTypeForEntryPoint { span: Span },
#[error("Mismatched number of parameters in trait implementation")]
Expand Down Expand Up @@ -233,6 +235,7 @@ impl From<TypeCheckError> for Diagnostic {
| TypeCheckError::OverflowingAssignment { span, .. }
| TypeCheckError::FieldModulo { span }
| TypeCheckError::ConstrainedReferenceToUnconstrained { span }
| TypeCheckError::NonConstantSliceLength { span }
| TypeCheckError::UnconstrainedReferenceToConstrained { span }
| TypeCheckError::UnconstrainedSliceReturnToConstrained { span } => {
Diagnostic::simple_error(error.to_string(), String::new(), span)
Expand Down
130 changes: 76 additions & 54 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,49 @@ impl<'interner> TypeChecker<'interner> {
false
}

fn check_hir_array_literal(
&mut self,
hir_array_literal: HirArrayLiteral,
) -> (Result<u64, Box<Type>>, Box<Type>) {
match hir_array_literal {
HirArrayLiteral::Standard(arr) => {
let elem_types = vecmap(&arr, |arg| self.check_expression(arg));

let first_elem_type = elem_types
.first()
.cloned()
.unwrap_or_else(|| self.interner.next_type_variable());

// Check if the array is homogeneous
for (index, elem_type) in elem_types.iter().enumerate().skip(1) {
let location = self.interner.expr_location(&arr[index]);

elem_type.unify(&first_elem_type, &mut self.errors, || {
TypeCheckError::NonHomogeneousArray {
first_span: self.interner.expr_location(&arr[0]).span,
first_type: first_elem_type.to_string(),
first_index: index,
second_span: location.span,
second_type: elem_type.to_string(),
second_index: index + 1,
}
.add_context("elements in an array must have the same type")
});
}

(Ok(arr.len() as u64), Box::new(first_elem_type.clone()))
}
HirArrayLiteral::Repeated { repeated_element, length } => {
let elem_type = self.check_expression(&repeated_element);
let length = match length {
Type::Constant(length) => Ok(length),
other => Err(Box::new(other)),
};
(length, Box::new(elem_type))
}
}
}

/// Infers a type for a given expression, and return this type.
/// As a side-effect, this function will also remember this type in the NodeInterner
/// for the given expr_id key.
Expand All @@ -59,64 +102,42 @@ impl<'interner> TypeChecker<'interner> {
pub(crate) fn check_expression(&mut self, expr_id: &ExprId) -> Type {
let typ = match self.interner.expression(expr_id) {
HirExpression::Ident(ident) => self.check_ident(ident, expr_id),
HirExpression::Literal(literal) => {
match literal {
HirLiteral::Array(HirArrayLiteral::Standard(arr)) => {
let elem_types = vecmap(&arr, |arg| self.check_expression(arg));

let first_elem_type = elem_types
.first()
.cloned()
.unwrap_or_else(|| self.interner.next_type_variable());

let arr_type = Type::Array(
Box::new(Type::constant_variable(arr.len() as u64, self.interner)),
Box::new(first_elem_type.clone()),
);

// Check if the array is homogeneous
for (index, elem_type) in elem_types.iter().enumerate().skip(1) {
let location = self.interner.expr_location(&arr[index]);

elem_type.unify(&first_elem_type, &mut self.errors, || {
TypeCheckError::NonHomogeneousArray {
first_span: self.interner.expr_location(&arr[0]).span,
first_type: first_elem_type.to_string(),
first_index: index,
second_span: location.span,
second_type: elem_type.to_string(),
second_index: index + 1,
}
.add_context("elements in an array must have the same type")
HirExpression::Literal(literal) => match literal {
HirLiteral::Array(hir_array_literal) => {
let (length, elem_type) = self.check_hir_array_literal(hir_array_literal);
Type::Array(
length.map_or_else(
|typ| typ,
|constant| Box::new(Type::constant_variable(constant, self.interner)),
),
elem_type,
)
}
HirLiteral::Slice(hir_array_literal) => {
let (length_type, elem_type) = self.check_hir_array_literal(hir_array_literal);
match length_type {
Ok(_length) => Type::Slice(elem_type),
Err(_non_constant) => {
self.errors.push(TypeCheckError::NonConstantSliceLength {
span: self.interner.expr_span(expr_id),
});
Type::Error
}

arr_type
}
HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) => {
let elem_type = self.check_expression(&repeated_element);
let length = match length {
Type::Constant(length) => {
Type::constant_variable(length, self.interner)
}
other => other,
};
Type::Array(Box::new(length), Box::new(elem_type))
}
HirLiteral::Bool(_) => Type::Bool,
HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner),
HirLiteral::Str(string) => {
let len = Type::Constant(string.len() as u64);
Type::String(Box::new(len))
}
HirLiteral::FmtStr(string, idents) => {
let len = Type::Constant(string.len() as u64);
let types = vecmap(&idents, |elem| self.check_expression(elem));
Type::FmtString(Box::new(len), Box::new(Type::Tuple(types)))
}
HirLiteral::Unit => Type::Unit,
}
}
HirLiteral::Bool(_) => Type::Bool,
HirLiteral::Integer(_, _) => Type::polymorphic_integer_or_field(self.interner),
HirLiteral::Str(string) => {
let len = Type::Constant(string.len() as u64);
Type::String(Box::new(len))
}
HirLiteral::FmtStr(string, idents) => {
let len = Type::Constant(string.len() as u64);
let types = vecmap(&idents, |elem| self.check_expression(elem));
Type::FmtString(Box::new(len), Box::new(Type::Tuple(types)))
}
HirLiteral::Unit => Type::Unit,
},
HirExpression::Infix(infix_expr) => {
// The type of the infix expression must be looked up from a type table
let lhs_type = self.check_expression(&infix_expr.lhs);
Expand Down Expand Up @@ -557,6 +578,7 @@ impl<'interner> TypeChecker<'interner> {
// XXX: We can check the array bounds here also, but it may be better to constant fold first
// and have ConstId instead of ExprId for constants
Type::Array(_, base_type) => *base_type,
Type::Slice(base_type) => *base_type,
Type::Error => Type::Error,
typ => {
let span = self.interner.expr_span(&new_lhs);
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir/type_check/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl<'interner> TypeChecker<'interner> {

let typ = match lvalue_type.follow_bindings() {
Type::Array(_, elem_type) => *elem_type,
Type::Slice(elem_type) => *elem_type,
Type::Error => Type::Error,
other => {
// TODO: Need a better span here
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ impl HirBinaryOp {
#[derive(Debug, Clone)]
pub enum HirLiteral {
Array(HirArrayLiteral),
Slice(HirArrayLiteral),
Bool(bool),
Integer(FieldElement, bool), //true for negative integer and false for positive
Str(String),
Expand Down
Loading

0 comments on commit 931cf2e

Please sign in to comment.