diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9600fc509cb..1fde7e1ffbc 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1893,6 +1893,7 @@ impl<'context> Elaborator<'context> { datatype.borrow_mut().init_variants(); let module_id = ModuleId { krate: self.crate_id, local_id: typ.module_id }; + self.resolving_ids.insert(*type_id); for (i, variant) in typ.enum_def.variants.iter().enumerate() { let parameters = variant.item.parameters.as_ref(); @@ -1919,7 +1920,10 @@ impl<'context> Elaborator<'context> { let location = variant.item.name.location(); self.interner.add_definition_location(reference_id, location, Some(module_id)); } + + self.resolving_ids.remove(type_id); } + self.generics.clear(); } fn elaborate_global(&mut self, global: UnresolvedGlobal) { diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index 93a12a46591..2f5eaccffe9 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -16,6 +16,7 @@ pub enum MonomorphizationError { ComptimeTypeInRuntimeCode { typ: String, location: Location }, CheckedTransmuteFailed { actual: Type, expected: Type, location: Location }, CheckedCastFailed { actual: Type, expected: Type, location: Location }, + RecursiveType { typ: Type, location: Location }, } impl MonomorphizationError { @@ -28,6 +29,7 @@ impl MonomorphizationError { | MonomorphizationError::ComptimeTypeInRuntimeCode { location, .. } | MonomorphizationError::CheckedTransmuteFailed { location, .. } | MonomorphizationError::CheckedCastFailed { location, .. } + | MonomorphizationError::RecursiveType { location, .. } | MonomorphizationError::NoDefaultType { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.location(), } @@ -67,6 +69,11 @@ impl From for CustomDiagnostic { let secondary = "Comptime type used here".into(); return CustomDiagnostic::simple_error(message, secondary, *location); } + MonomorphizationError::RecursiveType { typ, location } => { + let message = format!("Type `{typ}` is recursive"); + let secondary = "All types in Noir must have a known size at compile-time".into(); + return CustomDiagnostic::simple_error(message, secondary, *location); + } }; let location = error.location(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ba4640fa104..7a8f32b58d4 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -27,7 +27,7 @@ use crate::{ }; use acvm::{FieldElement, acir::AcirField}; use ast::{GlobalId, While}; -use fxhash::FxHashMap as HashMap; +use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use iter_extended::{btree_map, try_vecmap, vecmap}; use noirc_errors::Location; use noirc_printable_type::PrintableType; @@ -48,6 +48,7 @@ mod debug; pub mod debug_types; pub mod errors; pub mod printer; +pub mod tests; struct LambdaContext { env_ident: ast::Ident, @@ -378,7 +379,10 @@ impl<'interner> Monomorphizer<'interner> { other => other, }; - let return_type = Self::convert_type(return_type, meta.location)?; + // If `convert_type` fails here it is most likely because of generics at the + // call site after instantiating this function's type. So show the error there + // instead of at the function definition. + let return_type = Self::convert_type(return_type, location)?; let unconstrained = self.in_unconstrained_function; let attributes = self.interner.function_attributes(&f); @@ -1120,6 +1124,14 @@ impl<'interner> Monomorphizer<'interner> { /// Convert a non-tuple/struct type to a monomorphized type fn convert_type(typ: &HirType, location: Location) -> Result { + Self::convert_type_helper(typ, location, &mut HashSet::default()) + } + + fn convert_type_helper( + typ: &HirType, + location: Location, + seen_types: &mut HashSet, + ) -> Result { let typ = typ.follow_bindings_shallow(); Ok(match typ.as_ref() { HirType::FieldElement => ast::Type::Field, @@ -1155,12 +1167,14 @@ impl<'interner> Monomorphizer<'interner> { }); } }; - let fields = Box::new(Self::convert_type(fields.as_ref(), location)?); + let fields = + Box::new(Self::convert_type_helper(fields.as_ref(), location, seen_types)?); ast::Type::FmtString(size, fields) } HirType::Unit => ast::Type::Unit, HirType::Array(length, element) => { - let element = Box::new(Self::convert_type(element.as_ref(), location)?); + let element = + Box::new(Self::convert_type_helper(element.as_ref(), location, seen_types)?); let length = match length.evaluate_to_u32(location) { Ok(length) => length, Err(err) => { @@ -1175,7 +1189,8 @@ impl<'interner> Monomorphizer<'interner> { ast::Type::Array(length, element) } HirType::Slice(element) => { - let element = Box::new(Self::convert_type(element.as_ref(), location)?); + let element = + Box::new(Self::convert_type_helper(element.as_ref(), location, seen_types)?); ast::Type::Slice(element) } HirType::TraitAsType(..) => { @@ -1183,7 +1198,7 @@ impl<'interner> Monomorphizer<'interner> { } HirType::NamedGeneric(binding, _) => { if let TypeBinding::Bound(binding) = &*binding.borrow() { - return Self::convert_type(binding, location); + return Self::convert_type_helper(binding, location, seen_types); } // Default any remaining unbound type variables. @@ -1195,13 +1210,21 @@ impl<'interner> Monomorphizer<'interner> { HirType::CheckedCast { from, to } => { Self::check_checked_cast(from, to, location)?; - Self::convert_type(to, location)? + Self::convert_type_helper(to, location, seen_types)? } HirType::TypeVariable(binding) => { + let input_type = typ.as_ref().clone(); + if !seen_types.insert(input_type.clone()) { + let typ = input_type; + return Err(MonomorphizationError::RecursiveType { typ, location }); + } + let type_var_kind = match &*binding.borrow() { TypeBinding::Bound(binding) => { - return Self::convert_type(binding, location); + let typ = Self::convert_type_helper(binding, location, seen_types); + seen_types.remove(&input_type); + return typ; } TypeBinding::Unbound(_, type_var_kind) => type_var_kind.clone(), }; @@ -1214,7 +1237,8 @@ impl<'interner> Monomorphizer<'interner> { None => return Err(MonomorphizationError::NoDefaultType { location }), }; - let monomorphized_default = Self::convert_type(&default, location)?; + let monomorphized_default = + Self::convert_type_helper(&default, location, seen_types)?; binding.bind(default); monomorphized_default } @@ -1226,19 +1250,30 @@ impl<'interner> Monomorphizer<'interner> { Self::check_type(arg, location)?; } + let input_type = typ.as_ref().clone(); + if !seen_types.insert(input_type.clone()) { + let typ = input_type; + return Err(MonomorphizationError::RecursiveType { typ, location }); + } + let def = def.borrow(); if let Some(fields) = def.get_fields(args) { - let fields = - try_vecmap(fields, |(_, field)| Self::convert_type(&field, location))?; + let fields = try_vecmap(fields, |(_, field)| { + Self::convert_type_helper(&field, location, seen_types) + })?; + + seen_types.remove(&input_type); ast::Type::Tuple(fields) } else if let Some(variants) = def.get_variants(args) { // Enums are represented as (tag, variant1, variant2, .., variantN) let mut fields = vec![ast::Type::Field]; for (_, variant_fields) in variants { - let variant_fields = - try_vecmap(variant_fields, |typ| Self::convert_type(&typ, location))?; + let variant_fields = try_vecmap(variant_fields, |typ| { + Self::convert_type_helper(&typ, location, seen_types) + })?; fields.push(ast::Type::Tuple(variant_fields)); } + seen_types.remove(&input_type); ast::Type::Tuple(fields) } else { unreachable!("Data type has no body") @@ -1252,18 +1287,20 @@ impl<'interner> Monomorphizer<'interner> { Self::check_type(arg, location)?; } - Self::convert_type(&def.borrow().get_type(args), location)? + Self::convert_type_helper(&def.borrow().get_type(args), location, seen_types)? } HirType::Tuple(fields) => { - let fields = try_vecmap(fields, |x| Self::convert_type(x, location))?; + let fields = + try_vecmap(fields, |x| Self::convert_type_helper(x, location, seen_types))?; ast::Type::Tuple(fields) } HirType::Function(args, ret, env, unconstrained) => { - let args = try_vecmap(args, |x| Self::convert_type(x, location))?; - let ret = Box::new(Self::convert_type(ret, location)?); - let env = Self::convert_type(env, location)?; + let args = + try_vecmap(args, |x| Self::convert_type_helper(x, location, seen_types))?; + let ret = Box::new(Self::convert_type_helper(ret, location, seen_types)?); + let env = Self::convert_type_helper(env, location, seen_types)?; match &env { ast::Type::Unit => { ast::Type::Function(args, ret, Box::new(env), *unconstrained) @@ -1282,7 +1319,7 @@ impl<'interner> Monomorphizer<'interner> { // Lower both mutable & immutable references to the same reference type HirType::Reference(element, _mutable) => { - let element = Self::convert_type(element, location)?; + let element = Self::convert_type_helper(element, location, seen_types)?; ast::Type::MutableReference(Box::new(element)) } diff --git a/compiler/noirc_frontend/src/monomorphization/tests.rs b/compiler/noirc_frontend/src/monomorphization/tests.rs new file mode 100644 index 00000000000..77190b67576 --- /dev/null +++ b/compiler/noirc_frontend/src/monomorphization/tests.rs @@ -0,0 +1,129 @@ +#![cfg(test)] +use crate::tests::get_program; + +use super::{ast::Program, errors::MonomorphizationError, monomorphize}; + +pub fn get_monomorphized(src: &str) -> Result { + let (_parsed_module, mut context, errors) = get_program(src); + assert!( + errors.iter().all(|err| !err.is_error()), + "Expected monomorphized program to have no errors before monomorphization, but found: {errors:?}" + ); + + let main = context + .get_main_function(context.root_crate_id()) + .unwrap_or_else(|| panic!("get_monomorphized: test program contains no 'main' function")); + + monomorphize(main, &mut context.def_interner, false) +} + +fn check_rewrite(src: &str, expected: &str) { + let program = get_monomorphized(src).unwrap(); + assert!(format!("{}", program) == expected); +} + +#[test] +fn bounded_recursive_type_errors() { + // We want to eventually allow bounded recursive types like this, but for now they are + // disallowed because they cause a panic in convert_type during monomorphization. + let src = " + fn main() { + let _tree: Tree>> = Tree::Branch( + Tree::Branch(Tree::Leaf, Tree::Leaf), + Tree::Branch(Tree::Leaf, Tree::Leaf), + ); + } + + enum Tree { + Branch(T, T), + Leaf, + }"; + + let error = get_monomorphized(src).unwrap_err(); + assert!(matches!(error, MonomorphizationError::RecursiveType { .. })); +} + +#[test] +fn recursive_type_with_alias_errors() { + // We want to eventually allow bounded recursive types like this, but for now they are + // disallowed because they cause a panic in convert_type during monomorphization. + // + // In the future we could lower this type to: + // struct OptOptUnit { + // is_some: Field, + // some: OptUnit, + // none: (), + // } + // + // struct OptUnit { + // is_some: Field, + // some: (), + // none: (), + // } + let src = " + fn main() { + let _tree: Opt> = Opt::Some(OptAlias::None); + } + + type OptAlias = Opt; + + enum Opt { + Some(T), + None, + }"; + + let error = get_monomorphized(src).unwrap_err(); + assert!(matches!(error, MonomorphizationError::RecursiveType { .. })); +} + +#[test] +fn mutually_recursive_types_error() { + let src = " + fn main() { + let _zero = Even::Zero; + } + + enum Even { + Zero, + Succ(Odd), + } + + enum Odd { + One, + Succ(Even), + }"; + + let error = get_monomorphized(src).unwrap_err(); + assert!(matches!(error, MonomorphizationError::RecursiveType { .. })); +} + +#[test] +fn simple_closure_with_no_captured_variables() { + let src = r#" + fn main() -> pub Field { + let x = 1; + let closure = || x; + closure() + } + "#; + + let expected_rewrite = r#"fn main$f0() -> Field { + let x$0 = 1; + let closure$3 = { + let closure_variable$2 = { + let env$1 = (x$l0); + (env$l1, lambda$f1) + }; + closure_variable$l2 + }; + { + let tmp$4 = closure$l3; + tmp$l4.1(tmp$l4.0) + } +} +fn lambda$f1(mut env$l1: (Field)) -> Field { + env$l1.0 +} +"#; + check_rewrite(src, expected_rewrite); +} diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 8fd292aab75..f89b0960d3c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -33,9 +33,6 @@ use crate::hir::def_collector::dc_crate::DefCollector; use crate::hir::def_map::{CrateDefMap, LocalModuleId}; use crate::hir_def::expr::HirExpression; use crate::hir_def::stmt::HirStatement; -use crate::monomorphization::ast::Program; -use crate::monomorphization::errors::MonomorphizationError; -use crate::monomorphization::monomorphize; use crate::parser::{ItemKind, ParserErrorReason}; use crate::token::SecondaryAttribute; use crate::{ParsedModule, parse_program}; @@ -1124,52 +1121,6 @@ fn resolve_fmt_strings() { check_errors(src); } -fn monomorphize_program(src: &str) -> Result { - let (_program, mut context, _errors) = get_program(src); - let main_func_id = context.def_interner.find_function("main").unwrap(); - monomorphize(main_func_id, &mut context.def_interner, false) -} - -fn get_monomorphization_error(src: &str) -> Option { - monomorphize_program(src).err() -} - -fn check_rewrite(src: &str, expected: &str) { - let program = monomorphize_program(src).unwrap(); - assert!(format!("{}", program) == expected); -} - -#[test] -fn simple_closure_with_no_captured_variables() { - let src = r#" - fn main() -> pub Field { - let x = 1; - let closure = || x; - closure() - } - "#; - - let expected_rewrite = r#"fn main$f0() -> Field { - let x$0 = 1; - let closure$3 = { - let closure_variable$2 = { - let env$1 = (x$l0); - (env$l1, lambda$f1) - }; - closure_variable$l2 - }; - { - let tmp$4 = closure$l3; - tmp$l4.1(tmp$l4.0) - } -} -fn lambda$f1(mut env$l1: (Field)) -> Field { - env$l1.0 -} -"#; - check_rewrite(src, expected_rewrite); -} - #[test] fn deny_cyclic_globals() { let src = r#" diff --git a/compiler/noirc_frontend/src/tests/arithmetic_generics.rs b/compiler/noirc_frontend/src/tests/arithmetic_generics.rs index bcbdbdd6211..f1ef49669c3 100644 --- a/compiler/noirc_frontend/src/tests/arithmetic_generics.rs +++ b/compiler/noirc_frontend/src/tests/arithmetic_generics.rs @@ -5,7 +5,8 @@ use acvm::{AcirField, FieldElement}; use crate::hir::type_check::TypeCheckError; use crate::hir_def::types::{BinaryTypeOperator, Type}; use crate::monomorphization::errors::MonomorphizationError; -use crate::tests::{assert_no_errors, get_monomorphization_error}; +use crate::monomorphization::tests::get_monomorphized; +use crate::tests::assert_no_errors; #[test] fn arithmetic_generics_canonicalization_deduplication_regression() { @@ -72,13 +73,10 @@ fn arithmetic_generics_checked_cast_zeros() { bar(w) } "#; - assert_no_errors(source); - let monomorphization_error = get_monomorphization_error(source); - assert!(monomorphization_error.is_some()); + let monomorphization_error = get_monomorphized(source).unwrap_err(); // Expect a CheckedCast (0 % 0) failure - let monomorphization_error = monomorphization_error.unwrap(); if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } = monomorphization_error { @@ -117,13 +115,10 @@ fn arithmetic_generics_checked_cast_indirect_zeros() { let _ = bar(w); } "#; - assert_no_errors(source); - let monomorphization_error = get_monomorphization_error(source); - assert!(monomorphization_error.is_some()); + let monomorphization_error = get_monomorphized(source).unwrap_err(); // Expect a CheckedCast (0 % 0) failure - let monomorphization_error = monomorphization_error.unwrap(); if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } = monomorphization_error {