From 8a5359c012579e54c2766de1074482a36ecada32 Mon Sep 17 00:00:00 2001 From: jfecher Date: Thu, 7 Mar 2024 16:01:08 -0600 Subject: [PATCH] fix: Allow type aliases in main (#4505) # Description ## Problem\* Resolves #4500 ## Summary\* This was due to this check https://github.com/noir-lang/noir/blob/master/compiler/noirc_frontend/src/hir_def/types.rs#L706-L709 which was needed since we checked if types were valid for main during name resolution - which is before we know type aliases are not cyclic. I've moved this check to type checking instead. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] 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. --- .../src/hir/resolution/errors.rs | 5 -- .../src/hir/resolution/resolver.rs | 88 +------------------ .../src/hir/type_check/errors.rs | 5 ++ .../noirc_frontend/src/hir/type_check/mod.rs | 20 ++++- .../noirc_frontend/src/hir_def/function.rs | 4 + compiler/noirc_frontend/src/hir_def/types.rs | 8 +- compiler/noirc_frontend/src/tests.rs | 9 ++ 7 files changed, 42 insertions(+), 97 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index d2fe67da38c..1049599f079 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -80,8 +80,6 @@ pub enum ResolverError { PrivateFunctionCalled { name: String, span: Span }, #[error("{name} is not visible from the current crate")] 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 }, #[error("#[recursive] attribute is only allowed on entry points to a program")] @@ -309,9 +307,6 @@ impl From for Diagnostic { ResolverError::NonCrateFunctionCalled { span, name } => Diagnostic::simple_warning( format!("{name} is not visible from the current crate"), format!("{name} is only visible within its crate"), span), - 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(), diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 7789c06ca69..322891f0ae9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -911,10 +911,6 @@ impl<'a> Resolver<'a> { }); } - if self.is_entry_point_function(func) { - self.verify_type_valid_for_program_input(&typ); - } - let pattern = self.resolve_pattern(pattern, DefinitionKind::Local(None)); let typ = self.resolve_type_inner(typ, &mut generics); @@ -991,6 +987,7 @@ impl<'a> Resolver<'a> { return_distinctness: func.def.return_distinctness, has_body: !func.def.body.is_empty(), trait_constraints: self.resolve_trait_constraints(&func.def.where_clause), + is_entry_point: self.is_entry_point_function(func), } } @@ -2003,89 +2000,6 @@ impl<'a> Resolver<'a> { } HirLiteral::FmtStr(str, fmt_str_idents) } - - /// Only sized types are valid to be used as main's parameters or the parameters to a contract - /// function. If the given type is not sized (e.g. contains a slice or NamedGeneric type), an - /// error is issued. - fn verify_type_valid_for_program_input(&mut self, typ: &UnresolvedType) { - match &typ.typ { - UnresolvedTypeData::FieldElement - | UnresolvedTypeData::Integer(_, _) - | UnresolvedTypeData::Bool - | UnresolvedTypeData::Unit - | UnresolvedTypeData::Error => (), - - UnresolvedTypeData::MutableReference(_) - | UnresolvedTypeData::Function(_, _, _) - | UnresolvedTypeData::FormatString(_, _) - | UnresolvedTypeData::TraitAsType(..) - | UnresolvedTypeData::Unspecified => { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - - UnresolvedTypeData::Array(length, element) => { - if let Some(length) = length { - self.verify_type_expression_valid_for_program_input(length); - } else { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - self.verify_type_valid_for_program_input(element); - } - UnresolvedTypeData::Expression(expression) => { - self.verify_type_expression_valid_for_program_input(expression); - } - UnresolvedTypeData::String(length) => { - if let Some(length) = length { - self.verify_type_expression_valid_for_program_input(length); - } else { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - } - UnresolvedTypeData::Named(path, generics, _) => { - // Since the type is named, we need to resolve it to see what it actually refers to - // in order to check whether it is valid. Since resolving it may lead to a - // resolution error, we have to truncate our error count to the previous count just - // in case. This is to ensure resolution errors are not issued twice when this type - // is later resolved properly. - let error_count = self.errors.len(); - let resolved = self.resolve_named_type(path.clone(), generics.clone(), &mut vec![]); - self.errors.truncate(error_count); - - if !resolved.is_valid_for_program_input() { - let span = typ.span.expect("Function parameters should always have spans"); - self.push_err(ResolverError::InvalidTypeForEntryPoint { span }); - } - } - UnresolvedTypeData::Tuple(elements) => { - for element in elements { - self.verify_type_valid_for_program_input(element); - } - } - UnresolvedTypeData::Parenthesized(typ) => self.verify_type_valid_for_program_input(typ), - } - } - - fn verify_type_expression_valid_for_program_input(&mut self, expr: &UnresolvedTypeExpression) { - match expr { - UnresolvedTypeExpression::Constant(_, _) => (), - UnresolvedTypeExpression::Variable(path) => { - let error_count = self.errors.len(); - let resolved = self.resolve_named_type(path.clone(), vec![], &mut vec![]); - self.errors.truncate(error_count); - - if !resolved.is_valid_for_program_input() { - self.push_err(ResolverError::InvalidTypeForEntryPoint { span: path.span() }); - } - } - UnresolvedTypeExpression::BinaryOperation(lhs, _, rhs, _) => { - self.verify_type_expression_valid_for_program_input(lhs); - self.verify_type_expression_valid_for_program_input(rhs); - } - } - } } /// Gives an error if a user tries to create a mutable reference diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 96d30100d8b..cba2400441f 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -122,6 +122,8 @@ pub enum TypeCheckError { ConstrainedReferenceToUnconstrained { span: Span }, #[error("Slices cannot be returned from an unconstrained runtime to a constrained runtime")] UnconstrainedSliceReturnToConstrained { span: Span }, + #[error("Only sized types may be used in the entry point to a program")] + InvalidTypeForEntryPoint { span: Span }, } impl TypeCheckError { @@ -284,6 +286,9 @@ impl From for Diagnostic { let msg = format!("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope"); Diagnostic::simple_warning(msg, "Unnecessary trait constraint in where clause".into(), span) } + TypeCheckError::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), } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 21d1c75a0f2..aab793ec867 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -14,7 +14,7 @@ mod stmt; pub use errors::TypeCheckError; use crate::{ - hir_def::{expr::HirExpression, stmt::HirStatement, traits::TraitConstraint}, + hir_def::{expr::HirExpression, function::Param, stmt::HirStatement, traits::TraitConstraint}, node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, Type, }; @@ -74,6 +74,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec, + func_id: FuncId, + param: &Param, + errors: &mut Vec, +) { + let meta = type_checker.interner.function_meta(&func_id); + if meta.is_entry_point && !param.1.is_valid_for_program_input() { + let span = param.0.span(); + errors.push(TypeCheckError::InvalidTypeForEntryPoint { span }); + } +} + fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_errors::Span, bool) { let (expr_span, empty_function) = if let HirExpression::Block(block) = interner.expression(function_body_id) { @@ -329,6 +346,7 @@ mod test { trait_impl: None, return_type: FunctionReturnType::Default(Span::default()), trait_constraints: Vec::new(), + is_entry_point: true, }; interner.push_fn_meta(func_meta, func_id); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index d3ab2a9393b..82bbe1aa5b6 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -112,6 +112,10 @@ pub struct FuncMeta { /// The trait impl this function belongs to, if any pub trait_impl: Option, + + /// True if this function is an entry point to the program. + /// For non-contracts, this means the function is `main`. + pub is_entry_point: bool, } impl FuncMeta { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 13fa41733cf..5ab036eef5b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -703,10 +703,10 @@ impl Type { | Type::TraitAsType(..) | Type::NotConstant => false, - // This function is called during name resolution before we've verified aliases - // are not cyclic. As a result, it wouldn't be safe to check this alias' definition - // to see if the aliased type is valid. - Type::Alias(..) => false, + Type::Alias(alias, generics) => { + let alias = alias.borrow(); + alias.get_type(generics).is_valid_for_program_input() + } Type::Array(length, element) => { length.is_valid_for_program_input() && element.is_valid_for_program_input() diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 0c8c677d9af..9be6252b10a 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1206,4 +1206,13 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { "#; assert_eq!(get_program_errors(src).len(), 1); } + + #[test] + fn type_aliases_in_entry_point() { + let src = r#" + type Foo = u8; + fn main(_x: Foo) {} + "#; + assert_eq!(get_program_errors(src).len(), 0); + } }