diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 48994a9e877..585c726d8cc 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -276,7 +276,6 @@ pub(crate) fn check_trait_impl_method_matches_declaration( noir_function: &NoirFunction, ) -> Vec { let meta = interner.function_meta(&function); - let modifiers = interner.function_modifiers(&function); let method_name = interner.function_name(&function); let mut errors = Vec::new(); @@ -315,14 +314,6 @@ pub(crate) fn check_trait_impl_method_matches_declaration( // issue an error for it here. if let Some(trait_fn_id) = trait_info.method_ids.get(method_name) { let trait_fn_meta = interner.function_meta(trait_fn_id); - let trait_fn_modifiers = interner.function_modifiers(trait_fn_id); - - if modifiers.is_unconstrained != trait_fn_modifiers.is_unconstrained { - let expected = trait_fn_modifiers.is_unconstrained; - let location = meta.name.location; - let item = method_name.to_string(); - errors.push(TypeCheckError::UnconstrainedMismatch { item, expected, location }); - } if trait_fn_meta.direct_generics.len() != meta.direct_generics.len() { let expected = trait_fn_meta.direct_generics.len(); @@ -374,15 +365,21 @@ fn check_function_type_matches_expected_type( errors: &mut Vec, ) { let mut bindings = TypeBindings::new(); - // Shouldn't need to unify envs, they should always be equal since they're both free functions if let ( - Type::Function(params_a, ret_a, _env_a, _unconstrained_a), - Type::Function(params_b, ret_b, _env_b, _unconstrained_b), + Type::Function(params_a, ret_a, env_a, unconstrained_a), + Type::Function(params_b, ret_b, env_b, unconstrained_b), ) = (expected, actual) { - // TODO: we don't yet allow marking a trait function or a trait impl function as unconstrained, - // so both values will always be false here. Once we support that, we should check that both - // match (adding a test for it). + // Shouldn't need to unify envs, they should always be equal since they're both free functions + debug_assert_eq!(env_a, env_b, "envs should match as they're both free functions"); + + if unconstrained_a != unconstrained_b { + errors.push(TypeCheckError::UnconstrainedMismatch { + item: method_name.to_string(), + expected: *unconstrained_a, + location, + }); + } if params_a.len() == params_b.len() { for (i, (a, b)) in params_a.iter().zip(params_b.iter()).enumerate() { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index b0d66722dc6..d644518bdcb 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1576,16 +1576,24 @@ impl NodeInterner { let mut check_trait_generics = |impl_generics: &[Type], impl_associated_types: &[NamedType]| { - trait_generics.iter().zip(impl_generics).all(|(trait_generic, impl_generic)| { - let impl_generic = impl_generic.force_substitute(&instantiation_bindings); - trait_generic.try_unify(&impl_generic, &mut fresh_bindings).is_ok() - }) && trait_associated_types.iter().zip(impl_associated_types).all( + let generics_unify = trait_generics.iter().zip(impl_generics).all( |(trait_generic, impl_generic)| { + let impl_generic = + impl_generic.force_substitute(&instantiation_bindings); + trait_generic.try_unify(&impl_generic, &mut fresh_bindings).is_ok() + }, + ); + + let associated_types_unify = trait_associated_types + .iter() + .zip(impl_associated_types) + .all(|(trait_generic, impl_generic)| { let impl_generic2 = impl_generic.typ.force_substitute(&instantiation_bindings); trait_generic.typ.try_unify(&impl_generic2, &mut fresh_bindings).is_ok() - }, - ) + }); + + generics_unify && associated_types_unify }; let trait_generics = match impl_kind { diff --git a/compiler/noirc_frontend/src/tests/traits.rs b/compiler/noirc_frontend/src/tests/traits.rs index 4e4a1a2a6f2..c2d658b72c7 100644 --- a/compiler/noirc_frontend/src/tests/traits.rs +++ b/compiler/noirc_frontend/src/tests/traits.rs @@ -1494,3 +1494,39 @@ fn returns_self_in_trait_method_3() { "; assert_no_errors!(src); } + +#[named] +#[test] +fn errors_if_constrained_trait_definition_has_unconstrained_impl() { + let src = r#" + pub trait Foo { + fn foo() -> Field; + } + + impl Foo for Field { + unconstrained fn foo() -> Field { + ^^^ foo is not expected to be unconstrained + 42 + } + } + "#; + check_errors!(src); +} + +#[named] +#[test] +fn errors_if_unconstrained_trait_definition_has_constrained_impl() { + let src = r#" + pub trait Foo { + unconstrained fn foo() -> Field; + } + + impl Foo for Field { + fn foo() -> Field { + ^^^ foo is expected to be unconstrained + 42 + } + } + "#; + check_errors!(src); +} diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/Nargo.toml b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/Nargo.toml new file mode 100644 index 00000000000..73828e3fe73 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl" + type = "bin" + authors = [""] + + [dependencies] diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src/main.nr b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src/main.nr new file mode 100644 index 00000000000..64d9d8ace90 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src/main.nr @@ -0,0 +1,11 @@ + + pub trait Foo { + fn foo() -> Field; + } + + impl Foo for Field { + unconstrained fn foo() -> Field { + 42 + } + } + \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src_hash.txt b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src_hash.txt new file mode 100644 index 00000000000..46be0f3a590 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/src_hash.txt @@ -0,0 +1 @@ +11746277096026948021 \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/stderr.txt b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/stderr.txt new file mode 100644 index 00000000000..c7ed66b6a8a --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_constrained_trait_definition_has_unconstrained_impl/stderr.txt @@ -0,0 +1,8 @@ +error: foo is not expected to be unconstrained + ┌─ src/main.nr:7:26 + │ +7 │ unconstrained fn foo() -> Field { + │ --- + │ + +Aborting due to 1 previous error diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/Nargo.toml b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/Nargo.toml new file mode 100644 index 00000000000..fad94b6e7a3 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl" + type = "bin" + authors = [""] + + [dependencies] diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src/main.nr b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src/main.nr new file mode 100644 index 00000000000..2d7495cd02e --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src/main.nr @@ -0,0 +1,11 @@ + + pub trait Foo { + unconstrained fn foo() -> Field; + } + + impl Foo for Field { + fn foo() -> Field { + 42 + } + } + \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src_hash.txt b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src_hash.txt new file mode 100644 index 00000000000..ff458187950 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/src_hash.txt @@ -0,0 +1 @@ +9121808244621508618 \ No newline at end of file diff --git a/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/stderr.txt b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/stderr.txt new file mode 100644 index 00000000000..f1618dd4a57 --- /dev/null +++ b/test_programs/compile_failure/noirc_frontend_tests_traits_errors_if_unconstrained_trait_definition_has_constrained_impl/stderr.txt @@ -0,0 +1,8 @@ +error: foo is expected to be unconstrained + ┌─ src/main.nr:7:12 + │ +7 │ fn foo() -> Field { + │ --- + │ + +Aborting due to 1 previous error diff --git a/test_programs/execution_panic/regression_8210/temp/Nargo.toml b/test_programs/execution_panic/regression_8210/temp/Nargo.toml new file mode 100644 index 00000000000..56061329e3f --- /dev/null +++ b/test_programs/execution_panic/regression_8210/temp/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "temp" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_panic/regression_8210/temp/Prover.toml b/test_programs/execution_panic/regression_8210/temp/Prover.toml new file mode 100644 index 00000000000..9e6b8773a83 --- /dev/null +++ b/test_programs/execution_panic/regression_8210/temp/Prover.toml @@ -0,0 +1 @@ +x = "-1" diff --git a/test_programs/execution_panic/regression_8210/temp/src/main.nr b/test_programs/execution_panic/regression_8210/temp/src/main.nr new file mode 100644 index 00000000000..3c223ee728b --- /dev/null +++ b/test_programs/execution_panic/regression_8210/temp/src/main.nr @@ -0,0 +1,3 @@ +fn main(x: i8) { + assert_eq((x as i16) as i8, -1); +}