Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: Ban nested slices #4018

Merged
merged 10 commits into from
Jan 16, 2024
Merged
5 changes: 4 additions & 1 deletion compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ pub enum RuntimeError {
UnknownLoopBound { call_stack: CallStack },
#[error("Argument is not constant")]
AssertConstantFailed { call_stack: CallStack },
#[error("Nested slices are not supported")]
NestedSlice { call_stack: CallStack },
}

// We avoid showing the actual lhs and rhs since most of the time they are just 0
Expand Down Expand Up @@ -129,7 +131,8 @@ impl RuntimeError {
| RuntimeError::UnknownLoopBound { call_stack }
| RuntimeError::AssertConstantFailed { call_stack }
| RuntimeError::IntegerOutOfBounds { call_stack, .. }
| RuntimeError::UnsupportedIntegerSize { call_stack, .. } => call_stack,
| RuntimeError::UnsupportedIntegerSize { call_stack, .. }
| RuntimeError::NestedSlice { call_stack, .. } => call_stack,
}
}
}
Expand Down
20 changes: 17 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,14 @@ impl<'a> FunctionContext<'a> {

let typ = Self::convert_type(&array.typ).flatten();
Ok(match array.typ {
ast::Type::Array(_, _) => self.codegen_array(elements, typ[0].clone()),
ast::Type::Array(_, _) => {
self.codegen_array_checked(elements, typ[0].clone())?
}
ast::Type::Slice(_) => {
let slice_length =
self.builder.field_constant(array.contents.len() as u128);

let slice_contents = self.codegen_array(elements, typ[1].clone());
let slice_contents =
self.codegen_array_checked(elements, typ[1].clone())?;
Tree::Branch(vec![slice_length.into(), slice_contents])
}
_ => unreachable!(
Expand Down Expand Up @@ -231,6 +233,18 @@ impl<'a> FunctionContext<'a> {
self.codegen_array(elements, typ)
}

// Codegen an array but make sure that we do not have a nested slice
fn codegen_array_checked(
&mut self,
elements: Vec<Values>,
typ: Type,
) -> Result<Values, RuntimeError> {
if typ.is_nested_slice() {
return Err(RuntimeError::NestedSlice { call_stack: self.builder.get_call_stack() });
}
Ok(self.codegen_array(elements, typ))
}

/// Codegen an array by allocating enough space for each element and inserting separate
/// store instructions until each element is stored. The store instructions will be separated
/// by add instructions to calculate the new offset address to store to next.
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ pub enum ResolverError {
NonCrateFunctionCalled { name: String, span: Span },
#[error("Only sized types may be used in the entry point to a program")]
InvalidTypeForEntryPoint { span: Span },
#[error("Nested slices are not supported")]
NestedSlices { span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -304,6 +306,11 @@ impl From<ResolverError> for Diagnostic {
ResolverError::InvalidTypeForEntryPoint { span } => Diagnostic::simple_error(
"Only sized types may be used in the entry point to a program".to_string(),
"Slices, references, or any type containing them may not be used in main or a contract function".to_string(), span),
ResolverError::NestedSlices { span } => Diagnostic::simple_error(
"Nested slices are not supported".into(),
"Try to use a constant sized array instead".into(),
span,
),
}
}
}
7 changes: 6 additions & 1 deletion compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,12 @@ impl<'a> Resolver<'a> {

/// Translates an UnresolvedType to a Type
pub fn resolve_type(&mut self, typ: UnresolvedType) -> Type {
self.resolve_type_inner(typ, &mut vec![])
let span = typ.span;
let resolved_type = self.resolve_type_inner(typ, &mut vec![]);
if resolved_type.is_nested_slice() {
self.errors.push(ResolverError::NestedSlices { span: span.unwrap() });
}
resolved_type
}

pub fn resolve_type_aliases(
Expand Down
27 changes: 27 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ pub(crate) fn resolve_structs(
crate_id: CrateId,
) -> Vec<(CompilationError, FileId)> {
let mut errors: Vec<(CompilationError, FileId)> = vec![];
// This is necessary to avoid cloning the entire struct map
// when adding checks after each struct field is resolved.
let struct_ids = structs.keys().copied().collect::<Vec<_>>();

// Resolve each field in each struct.
// Each struct should already be present in the NodeInterner after def collection.
for (type_id, typ) in structs {
Expand All @@ -35,6 +39,28 @@ pub(crate) fn resolve_structs(
struct_def.generics = generics;
});
}

// Check whether the struct fields have nested slices
// We need to check after all structs are resolved to
// make sure every struct's fields is accurately set.
for id in struct_ids {
let struct_type = context.def_interner.get_struct(id);
// Only handle structs without generics as any generics args will be checked
// after monomorphization when performing SSA codegen
if struct_type.borrow().generics.is_empty() {
let fields = struct_type.borrow().get_fields(&[]);
for field in fields.iter() {
if field.1.is_nested_slice() {
errors.push((
ResolverError::NestedSlices { span: struct_type.borrow().location.span }
.into(),
struct_type.borrow().location.file,
));
}
}
}
}

errors
}

Expand All @@ -49,5 +75,6 @@ fn resolve_struct_fields(
let (generics, fields, errors) =
Resolver::new(&mut context.def_interner, &path_resolver, &context.def_maps, file_id)
.resolve_struct_fields(unresolved.struct_def);

(generics, fields, errors)
}
37 changes: 37 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,43 @@ impl Type {
| Type::Error => unreachable!("This type cannot exist as a parameter to main"),
}
}

pub(crate) fn is_nested_slice(&self) -> bool {
match self {
Type::Array(size, elem) => {
if let Type::NotConstant = size.as_ref() {
elem.as_ref().contains_slice()
} else {
false
}
}
_ => false,
}
}

fn contains_slice(&self) -> bool {
match self {
Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant),
Type::Struct(struct_typ, generics) => {
let fields = struct_typ.borrow().get_fields(generics);
for field in fields.iter() {
if field.1.contains_slice() {
return true;
}
}
false
}
Type::Tuple(types) => {
for typ in types.iter() {
if typ.contains_slice() {
return true;
}
}
false
}
_ => false,
}
}
}

/// A list of TypeVariableIds to bind to a type. Storing the
Expand Down
4 changes: 4 additions & 0 deletions docs/docs/noir/concepts/data_types/arrays.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ You can define multidimensional arrays:
let array : [[Field; 2]; 2];
let element = array[0][0];
```
However, multidimensional slices are not supported. For example, the following code will error at compile time:
```rust
let slice : [[Field]] = [];
```

## Types

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "nested_slice_declared_type"
type = "bin"
authors = [""]
compiler_version = ">=0.22.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn main(x: Field, y: pub Field) {
assert(x != y);

let slice: [[Field]] = [];
assert(slice.len() != 10);
}
7 changes: 7 additions & 0 deletions test_programs/compile_failure/nested_slice_literal/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "nested_slice_literal"
type = "bin"
authors = [""]
compiler_version = ">=0.22.0"

[dependencies]
23 changes: 23 additions & 0 deletions test_programs/compile_failure/nested_slice_literal/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
struct FooParent<T> {
parent_arr: [Field; 3],
foos: [Foo<T>],
}

struct Bar {
inner: [Field; 3],
}

struct Foo<T> {
a: Field,
b: T,
bar: Bar,
}

fn main(x: Field, y: pub Field) {
assert(x != y);

let foo = Foo { a: 7, b: [8, 9, 22].as_slice(), bar: Bar { inner: [106, 107, 108] } };
let mut slice = [foo, foo];
slice = slice.push_back(foo);
assert(slice.len() == 3);
}
7 changes: 7 additions & 0 deletions test_programs/compile_failure/nested_slice_struct/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "nested_slice_struct"
type = "bin"
authors = [""]
compiler_version = ">=0.22.0"

[dependencies]
18 changes: 18 additions & 0 deletions test_programs/compile_failure/nested_slice_struct/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
struct FooParent {
parent_arr: [Field; 3],
foos: [Foo],
}

struct Bar {
inner: [Field; 3],
}

struct Foo {
a: Field,
b: [Field],
bar: Bar,
}

fn main(x: Field, y: pub Field) {
assert(x != y);
}

This file was deleted.

This file was deleted.

6 changes: 0 additions & 6 deletions test_programs/execution_success/slice_struct_field/Nargo.toml

This file was deleted.

This file was deleted.

Loading
Loading