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

feat: better error message when trying to invoke struct function field #6661

Merged
merged 3 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@
// TODO(https://github.com/noir-lang/noir/issues/6238):
// support non-u32 generics here
if !kind.unifies(&Kind::u32()) {
let error = TypeCheckError::EvaluatedGlobalIsntU32 {

Check warning on line 421 in compiler/noirc_frontend/src/elaborator/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
expected_kind: Kind::u32().to_string(),
expr_kind: kind.to_string(),
expr_span: path.span(),
Expand Down Expand Up @@ -1321,11 +1321,23 @@
{
Some(method_id) => Some(HirMethodReference::FuncId(method_id)),
None => {
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
let has_field_with_function_type =
typ.borrow().get_fields_as_written().into_iter().any(|field| {
field.name.0.contents == method_name && field.typ.is_function()
});
if has_field_with_function_type {
self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
} else {
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
}
None
}
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
},
// TODO(https://github.com/noir-lang/noir/issues/6238): implement handling for larger types
#[error("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)")]
EvaluatedGlobalIsntU32 { expected_kind: String, expr_kind: String, expr_span: Span },

Check warning on line 71 in compiler/noirc_frontend/src/hir/type_check/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
#[error("Expected {expected:?} found {found:?}")]
ArityMisMatch { expected: usize, found: usize, span: Span },
#[error("Return type in a function cannot be public")]
Expand Down Expand Up @@ -99,6 +99,8 @@
CannotMutateImmutableVariable { name: String, span: Span },
#[error("No method named '{method_name}' found for type '{object_type}'")]
UnresolvedMethodCall { method_name: String, object_type: Type, span: Span },
#[error("Cannot invoke function field '{method_name}' on type '{object_type}' as a method")]
CannotInvokeStructFieldFunctionType { method_name: String, object_type: Type, span: Span },
#[error("Integers must have the same signedness LHS is {sign_x:?}, RHS is {sign_y:?}")]
IntegerSignedness { sign_x: Signedness, sign_y: Signedness, span: Span },
#[error("Integers must have the same bit width LHS is {bit_width_x}, RHS is {bit_width_y}")]
Expand Down Expand Up @@ -273,7 +275,7 @@
}
// TODO(https://github.com/noir-lang/noir/issues/6238): implement
// handling for larger types
TypeCheckError::EvaluatedGlobalIsntU32 { expected_kind, expr_kind, expr_span } => {

Check warning on line 278 in compiler/noirc_frontend/src/hir/type_check/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
Diagnostic::simple_warning(
format!("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)"),
String::new(),
Expand Down Expand Up @@ -511,6 +513,13 @@
TypeCheckError::CyclicType { typ: _, span } => {
Diagnostic::simple_error(error.to_string(), "Cyclic types have unlimited size and are prohibited in Noir".into(), *span)
}
TypeCheckError::CannotInvokeStructFieldFunctionType { method_name, object_type, span } => {
Diagnostic::simple_error(
format!("Cannot invoke function field '{method_name}' on type '{object_type}' as a method"),
format!("to call the function stored in '{method_name}', surround the field access with parentheses: '(', ')'"),
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
*span,
)
},
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,14 @@
}
}

pub fn is_function(&self) -> bool {
match self.follow_bindings() {
asterite marked this conversation as resolved.
Show resolved Hide resolved
Type::Function(..) => true,
Type::Alias(alias_type, _) => alias_type.borrow().typ.is_function(),
_ => false,
}
}

/// True if this type can be used as a parameter to `main` or a contract function.
/// This is only false for unsized types like slices or slices that do not make sense
/// as a program input such as named generics or mutable references.
Expand Down Expand Up @@ -2138,7 +2146,7 @@
}

let recur_on_binding = |id, replacement: &Type| {
// Prevent recuring forever if there's a `T := T` binding

Check warning on line 2149 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (recuring)
if replacement.type_variable_id() == Some(id) {
replacement.clone()
} else {
Expand Down Expand Up @@ -2223,7 +2231,7 @@
Type::Tuple(fields)
}
Type::Forall(typevars, typ) => {
// Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall

Check warning on line 2234 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (typevarsfined)
// is usually impossible and indicative of an error in the type checker somewhere.
for var in typevars {
assert!(!type_bindings.contains_key(&var.id()));
Expand Down Expand Up @@ -2389,7 +2397,7 @@
}
}

/// Follow bindings if this is a type variable or generic to the first non-typevariable

Check warning on line 2400 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (typevariable)
/// type. Unlike `follow_bindings`, this won't recursively follow any bindings on any
/// fields or arguments of this type.
pub fn follow_bindings_shallow(&self) -> Cow<Type> {
Expand All @@ -2410,7 +2418,7 @@

/// Replace any `Type::NamedGeneric` in this type with a `Type::TypeVariable`
/// using to the same inner `TypeVariable`. This is used during monomorphization
/// to bind to named generics since they are unbindable during type checking.

Check warning on line 2421 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (unbindable)
pub fn replace_named_generics_with_type_variables(&mut self) {
match self {
Type::FieldElement
Expand Down Expand Up @@ -2871,7 +2879,7 @@
len.hash(state);
env.hash(state);
}
Type::Tuple(elems) => elems.hash(state),

Check warning on line 2882 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elems)

Check warning on line 2882 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elems)
Type::Struct(def, args) => {
def.hash(state);
args.hash(state);
Expand Down
29 changes: 29 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2002,7 +2002,7 @@
}

// TODO(https://github.com/noir-lang/noir/issues/6238):
// The EvaluatedGlobalIsntU32 warning is a stopgap

Check warning on line 2005 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
// (originally from https://github.com/noir-lang/noir/issues/6125)
#[test]
fn numeric_generic_field_larger_than_u32() {
Expand Down Expand Up @@ -3752,6 +3752,35 @@
assert_no_errors(src);
}

#[test]
fn errors_with_better_message_when_trying_to_invoke_struct_field_that_is_a_function() {
let src = r#"
pub struct Foo {
wrapped: fn(Field) -> bool,
}

impl Foo {
fn call(self) -> bool {
self.wrapped(1)
}
}

fn main() {}
"#;
let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::TypeError(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name,
..
}) = &errors[0].0
else {
panic!("Expected a 'CannotInvokeStructFieldFunctionType' error, got {:?}", errors[0].0);
};

assert_eq!(method_name, "wrapped");
}

fn test_disallows_attribute_on_impl_method(
attr: &str,
check_error: impl FnOnce(&CompilationError),
Expand Down
Loading