Skip to content

Commit

Permalink
fix: Allow type aliases in main (#4505)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
jfecher authored Mar 7, 2024
1 parent 3163d81 commit 8a5359c
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 97 deletions.
5 changes: 0 additions & 5 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -309,9 +307,6 @@ impl From<ResolverError> 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(),
Expand Down
88 changes: 1 addition & 87 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -284,6 +286,9 @@ impl From<TypeCheckError> 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),
}
}
}
20 changes: 19 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -74,6 +74,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
// This is locally obvious, but it must be bound here so that the
// Definition object of the parameter in the NodeInterner is given the correct type.
for param in parameters {
check_if_type_is_valid_for_program_input(&type_checker, func_id, &param, &mut errors);
type_checker.bind_pattern(&param.0, param.1);
}

Expand Down Expand Up @@ -143,6 +144,22 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
errors
}

/// 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 check_if_type_is_valid_for_program_input(
type_checker: &TypeChecker<'_>,
func_id: FuncId,
param: &Param,
errors: &mut Vec<TypeCheckError>,
) {
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) {
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ pub struct FuncMeta {

/// The trait impl this function belongs to, if any
pub trait_impl: Option<TraitImplId>,

/// 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 {
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit 8a5359c

Please sign in to comment.