Skip to content

Commit

Permalink
fix: Allow where clause on all functions and improve error message (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored Nov 9, 2023
1 parent 7ae12f8 commit 1647e33
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 32 deletions.
14 changes: 11 additions & 3 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ impl<'interner> TypeChecker<'interner> {
method_name: &str,
expr_id: &ExprId,
) -> Option<HirMethodReference> {
match object_type {
match object_type.follow_bindings() {
Type::Struct(typ, _args) => {
let id = typ.borrow().id;
match self.interner.lookup_method(object_type, id, method_name, false) {
Expand Down Expand Up @@ -914,12 +914,20 @@ impl<'interner> TypeChecker<'interner> {
.interner
.lookup_primitive_trait_method_mut(element.as_ref(), method_name)
.map(HirMethodReference::FuncId)
.or_else(|| self.lookup_method(element, method_name, expr_id)),
.or_else(|| self.lookup_method(&element, method_name, expr_id)),

// If we fail to resolve the object to a struct type, we have no way of type
// checking its arguments as we can't even resolve the name of the function
Type::Error => None,

other => match self.interner.lookup_primitive_method(other, method_name) {
// The type variable must be unbound at this point since follow_bindings was called
Type::TypeVariable(_, TypeVariableKind::Normal) => {
let span = self.interner.expr_span(expr_id);
self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span });
None
}

other => match self.interner.lookup_primitive_method(&other, method_name) {
Some(method_id) => Some(HirMethodReference::FuncId(method_id)),
None => {
self.errors.push(TypeCheckError::UnresolvedMethodCall {
Expand Down
2 changes: 0 additions & 2 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ pub enum ParserErrorReason {
ComptimeDeprecated,
#[error("{0} are experimental and aren't fully supported yet")]
ExperimentalFeature(&'static str),
#[error("Where clauses are allowed only on functions with generic parameters")]
WhereClauseOnNonGenericFunction,
#[error(
"Multiple primary attributes found. Only one function attribute is allowed per function"
)]
Expand Down
33 changes: 6 additions & 27 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,11 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
let ((((attributes, modifiers), name), generics), parameters) = args;

// Validate collected attributes, filtering them into function and secondary variants
let attrs = validate_attributes(attributes, span, emit);
validate_where_clause(&generics, &where_clause, span, emit);
let attributes = validate_attributes(attributes, span, emit);
FunctionDefinition {
span: body_span,
name,
attributes: attrs,
attributes,
is_unconstrained: modifiers.0,
is_open: modifiers.2,
is_internal: modifiers.3,
Expand Down Expand Up @@ -412,7 +411,6 @@ fn trait_definition() -> impl NoirParser<TopLevelStatement> {
.then(trait_body())
.then_ignore(just(Token::RightBrace))
.validate(|(((name, generics), where_clause), items), span, emit| {
validate_where_clause(&generics, &where_clause, span, emit);
emit(ParserError::with_reason(ParserErrorReason::ExperimentalFeature("Traits"), span));
TopLevelStatement::Trait(NoirTrait { name, generics, where_clause, span, items })
})
Expand Down Expand Up @@ -451,12 +449,9 @@ fn trait_function_declaration() -> impl NoirParser<TraitItem> {
.then(function_return_type().map(|(_, typ)| typ))
.then(where_clause())
.then(trait_function_body_or_semicolon)
.validate(
|(((((name, generics), parameters), return_type), where_clause), body), span, emit| {
validate_where_clause(&generics, &where_clause, span, emit);
TraitItem::Function { name, generics, parameters, return_type, where_clause, body }
},
)
.map(|(((((name, generics), parameters), return_type), where_clause), body)| {
TraitItem::Function { name, generics, parameters, return_type, where_clause, body }
})
}

fn validate_attributes(
Expand Down Expand Up @@ -514,17 +509,6 @@ fn validate_struct_attributes(
struct_attributes
}

fn validate_where_clause(
generics: &Vec<Ident>,
where_clause: &Vec<UnresolvedTraitConstraint>,
span: Span,
emit: &mut dyn FnMut(ParserError),
) {
if !where_clause.is_empty() && generics.is_empty() {
emit(ParserError::with_reason(ParserErrorReason::WhereClauseOnNonGenericFunction, span));
}
}

/// Function declaration parameters differ from other parameters in that parameter
/// patterns are not allowed in declarations. All parameters must be identifiers.
fn function_declaration_parameters() -> impl NoirParser<Vec<(Ident, UnresolvedType)>> {
Expand Down Expand Up @@ -2133,7 +2117,6 @@ mod test {
"fn func_name<T>(f: Field, y : pub Field, z : pub [u8;5],) where T: {}",
"fn func_name<T>(f: Field, y : pub Field, z : pub [u8;5],) where SomeTrait {}",
"fn func_name<T>(f: Field, y : pub Field, z : pub [u8;5],) SomeTrait {}",
"fn func_name(f: Field, y : pub Field, z : pub [u8;5],) where T: SomeTrait {}",
// A leading plus is not allowed.
"fn func_name<T>(f: Field, y : T) where T: + SomeTrait {}",
"fn func_name<T>(f: Field, y : T) where T: TraitX + <Y> {}",
Expand Down Expand Up @@ -2164,11 +2147,7 @@ mod test {

parse_all_failing(
trait_definition(),
vec![
"trait MissingBody",
"trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }",
"trait WhereClauseWithoutGenerics where A: SomeTrait { }",
],
vec!["trait MissingBody", "trait WrongDelimiter { fn foo() -> u8, fn bar() -> u8 }"],
);
}

Expand Down

0 comments on commit 1647e33

Please sign in to comment.