diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs index 15a566156f7..6f4ddcef9a3 100644 --- a/compiler/noirc_frontend/src/ast/traits.rs +++ b/compiler/noirc_frontend/src/ast/traits.rs @@ -289,6 +289,15 @@ impl Display for TraitImplItemKind { } } +/// Does both `desugar_generic_trait_bounds` and `reorder_where_clause`. +pub(crate) fn desugar_generic_trait_bounds_and_reorder_where_clause( + generics: &mut Vec, + where_clause: &mut Vec, +) { + desugar_generic_trait_bounds(generics, where_clause); + reorder_where_clause(where_clause); +} + /// Moves trait bounds from generics into where clauses. For example: /// /// ```noir @@ -300,7 +309,7 @@ impl Display for TraitImplItemKind { /// ```noir /// fn foo(x: T) -> T where T: Trait {} /// ``` -pub(crate) fn desugar_generic_trait_bounds( +fn desugar_generic_trait_bounds( generics: &mut Vec, where_clause: &mut Vec, ) { @@ -323,3 +332,29 @@ pub(crate) fn desugar_generic_trait_bounds( } } } + +/// Reorders a where clause in-place so that simpler constraints come before more complex ones. +/// The resulting where clause will have constraints in this order: +/// 1. Paths without generics +/// 2. Paths with generics +/// 3. Everything else +fn reorder_where_clause(where_clause: &mut Vec) { + let mut paths_without_generics = Vec::new(); + let mut paths_with_generics = Vec::new(); + let mut others = Vec::new(); + + for clause in std::mem::take(where_clause) { + if let UnresolvedTypeData::Named(_, generics, _) = &clause.typ.typ { + if generics.is_empty() { + paths_without_generics.push(clause); + } else { + paths_with_generics.push(clause); + } + } else { + others.push(clause); + } + } + where_clause.extend(paths_without_generics); + where_clause.extend(paths_with_generics); + where_clause.extend(others); +} diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f16ace4bad1..1d1dc954070 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1394,8 +1394,11 @@ impl<'context> Elaborator<'context> { }; let trait_constraint_trait_name = trait_constraint_trait.name.to_string(); - let trait_constraint_type = trait_constraint.typ.substitute(&bindings); - let trait_bound = &trait_constraint.trait_bound; + let mut trait_constraint = trait_constraint.clone(); + trait_constraint.apply_bindings(&bindings); + + let trait_constraint_type = trait_constraint.typ; + let trait_bound = trait_constraint.trait_bound; let mut named_generics = trait_bound.trait_generics.named.clone(); @@ -1407,29 +1410,36 @@ impl<'context> Elaborator<'context> { // so they'll unify (the bindings aren't applied here so this is fine). // If they are bound though, we won't replace them as we want to ensure the binding // matches. + // + // `bindings` is passed here because these implicitly added named generics might + // have a constraint on them later on and we want to remember what type they ended + // up being. self.replace_implicitly_added_unbound_named_generics_with_fresh_type_variables( &mut named_generics, + &mut bindings, ); - if self - .interner - .try_lookup_trait_implementation( - &trait_constraint_type, - trait_bound.trait_id, - &trait_bound.trait_generics.ordered, - &named_generics, - ) - .is_err() - { - let missing_trait = - format!("{}{}", trait_constraint_trait_name, trait_bound.trait_generics); - self.push_err(ResolverError::TraitNotImplemented { - impl_trait: impl_trait.clone(), - missing_trait, - type_missing_trait: trait_constraint_type.to_string(), - location: trait_impl.object_type.location, - missing_trait_location: trait_bound.location, - }); + match self.interner.try_lookup_trait_implementation( + &trait_constraint_type, + trait_bound.trait_id, + &trait_bound.trait_generics.ordered, + &named_generics, + ) { + Ok((_, impl_bindings, impl_instantiation_bindings)) => { + bindings.extend(impl_bindings); + bindings.extend(impl_instantiation_bindings); + } + Err(_) => { + let missing_trait = + format!("{}{}", trait_constraint_trait_name, trait_bound.trait_generics); + self.push_err(ResolverError::TraitNotImplemented { + impl_trait: impl_trait.clone(), + missing_trait, + type_missing_trait: trait_constraint_type.to_string(), + location: trait_impl.object_type.location, + missing_trait_location: trait_bound.location, + }); + } } } } @@ -1437,13 +1447,19 @@ impl<'context> Elaborator<'context> { fn replace_implicitly_added_unbound_named_generics_with_fresh_type_variables( &mut self, named_generics: &mut [NamedType], + bindings: &mut TypeBindings, ) { for named_type in named_generics.iter_mut() { match &named_type.typ { Type::NamedGeneric(NamedGeneric { type_var, implicit: true, .. }) if type_var.borrow().is_unbound() => { - named_type.typ = self.interner.next_type_variable(); + let type_var_id = type_var.id(); + let new_type_var_id = self.interner.next_type_variable_id(); + let kind = type_var.kind(); + let new_type_var = TypeVariable::unbound(new_type_var_id, kind.clone()); + named_type.typ = Type::TypeVariable(new_type_var.clone()); + bindings.insert(type_var_id, (new_type_var, kind, named_type.typ.clone())); } _ => (), }; diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index b7db9d1bf76..d6b2d1afb87 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -14,7 +14,7 @@ use crate::ast::{ Documented, Expression, FunctionDefinition, Ident, ItemVisibility, LetStatement, ModuleDeclaration, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, UnresolvedType, - UnresolvedTypeData, desugar_generic_trait_bounds, + UnresolvedTypeData, desugar_generic_trait_bounds_and_reorder_where_clause, }; use crate::elaborator::PrimitiveType; use crate::hir::resolution::errors::ResolverError; @@ -177,7 +177,10 @@ impl ModCollector<'_> { let module_id = ModuleId { krate, local_id: self.module_id }; for mut r#impl in impls { - desugar_generic_trait_bounds(&mut r#impl.generics, &mut r#impl.where_clause); + desugar_generic_trait_bounds_and_reorder_where_clause( + &mut r#impl.generics, + &mut r#impl.where_clause, + ); collect_impl( &mut context.def_interner, @@ -201,7 +204,7 @@ impl ModCollector<'_> { let mut errors = Vec::new(); for mut trait_impl in impls { - desugar_generic_trait_bounds( + desugar_generic_trait_bounds_and_reorder_where_clause( &mut trait_impl.impl_generics, &mut trait_impl.where_clause, ); @@ -298,7 +301,7 @@ impl ModCollector<'_> { // With this method we iterate each function in the Crate and not each module // This may not be great because we have to pull the module_data for each function let mut noir_function = function.item; - desugar_generic_trait_bounds( + desugar_generic_trait_bounds_and_reorder_where_clause( &mut noir_function.def.generics, &mut noir_function.def.where_clause, ); @@ -449,7 +452,7 @@ impl ModCollector<'_> { let has_allow_dead_code = trait_definition.attributes.iter().any(|attr| attr.kind.is_allow("dead_code")); - desugar_generic_trait_bounds( + desugar_generic_trait_bounds_and_reorder_where_clause( &mut trait_definition.generics, &mut trait_definition.where_clause, ); @@ -516,7 +519,7 @@ impl ModCollector<'_> { for item in &mut trait_definition.items { if let TraitItem::Function { generics, where_clause, .. } = &mut item.item { - desugar_generic_trait_bounds(generics, where_clause); + desugar_generic_trait_bounds_and_reorder_where_clause(generics, where_clause); } } @@ -1329,7 +1332,11 @@ pub fn collect_impl( let func_id = interner.push_empty_fn(); method.def.where_clause.extend(r#impl.where_clause.clone()); - desugar_generic_trait_bounds(&mut method.def.generics, &mut method.def.where_clause); + desugar_generic_trait_bounds_and_reorder_where_clause( + &mut method.def.generics, + &mut method.def.where_clause, + ); + let location = method.location(); interner.push_function(func_id, &method.def, module_id, location); unresolved_functions.push_fn(module_id.local_id, func_id, method); @@ -1454,7 +1461,7 @@ pub(crate) fn collect_trait_impl_items( let location = impl_method.location(); interner.push_function(func_id, &impl_method.def, module, location); interner.set_doc_comments(ReferenceId::Function(func_id), item.doc_comments); - desugar_generic_trait_bounds( + desugar_generic_trait_bounds_and_reorder_where_clause( &mut impl_method.def.generics, &mut impl_method.def.where_clause, ); diff --git a/compiler/noirc_frontend/src/tests/traits.rs b/compiler/noirc_frontend/src/tests/traits.rs index 79a3c15b743..6370629f578 100644 --- a/compiler/noirc_frontend/src/tests/traits.rs +++ b/compiler/noirc_frontend/src/tests/traits.rs @@ -2088,3 +2088,89 @@ fn trait_method_call_when_it_has_bounds_on_generic() { "#; assert_no_errors!(src); } + +#[named] +#[test] +fn trait_bound_constraining_two_generics() { + let src = r#" + pub trait Foo {} + + pub trait Baz + where + T: Foo, + {} + + pub struct HasFoo1 {} + impl Foo<()> for HasFoo1 {} + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + "#; + assert_no_errors!(src); +} + +#[named] +#[test] +fn trait_where_clause_associated_type_constraint_expected_order() { + let src = r#" + pub trait BarTrait {} + + pub trait Foo { + type Bar; + } + + pub trait Baz + where + T: Foo, + ::Bar: BarTrait, + {} + + pub struct HasBarTrait1 {} + impl BarTrait for HasBarTrait1 {} + + pub struct HasFoo1 {} + impl Foo for HasFoo1 { + type Bar = HasBarTrait1; + } + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + "#; + assert_no_errors!(src); +} + +#[named] +#[test] +fn trait_where_clause_associated_type_constraint_unexpected_order() { + let src = r#" + pub trait BarTrait {} + + pub trait Foo { + type Bar; + } + + pub trait Baz + where + ::Bar: BarTrait, + T: Foo, + {} + + pub struct HasBarTrait1 {} + impl BarTrait for HasBarTrait1 {} + + pub struct HasFoo1 {} + impl Foo for HasFoo1 { + type Bar = HasBarTrait1; + } + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + "#; + assert_no_errors!(src); +} diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/Nargo.toml new file mode 100644 index 00000000000..ab8c98ca309 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_traits_trait_bound_constraining_two_generics" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src/main.nr new file mode 100644 index 00000000000..bf1048ed206 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src/main.nr @@ -0,0 +1,16 @@ + + pub trait Foo {} + + pub trait Baz + where + T: Foo, + {} + + pub struct HasFoo1 {} + impl Foo<()> for HasFoo1 {} + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src_hash.txt new file mode 100644 index 00000000000..ee541a8b6d5 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/src_hash.txt @@ -0,0 +1 @@ +10102510944933110504 \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/Nargo.toml new file mode 100644 index 00000000000..361c9264560 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src/main.nr new file mode 100644 index 00000000000..ef4d1a22a4e --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src/main.nr @@ -0,0 +1,26 @@ + + pub trait BarTrait {} + + pub trait Foo { + type Bar; + } + + pub trait Baz + where + T: Foo, + ::Bar: BarTrait, + {} + + pub struct HasBarTrait1 {} + impl BarTrait for HasBarTrait1 {} + + pub struct HasFoo1 {} + impl Foo for HasFoo1 { + type Bar = HasBarTrait1; + } + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src_hash.txt new file mode 100644 index 00000000000..0706ce9f41c --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/src_hash.txt @@ -0,0 +1 @@ +17365504020734663832 \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/Nargo.toml b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/Nargo.toml new file mode 100644 index 00000000000..78d5a577a3b --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/Nargo.toml @@ -0,0 +1,7 @@ + + [package] + name = "noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order" + type = "bin" + authors = [""] + + [dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src/main.nr b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src/main.nr new file mode 100644 index 00000000000..c815d67d582 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src/main.nr @@ -0,0 +1,26 @@ + + pub trait BarTrait {} + + pub trait Foo { + type Bar; + } + + pub trait Baz + where + ::Bar: BarTrait, + T: Foo, + {} + + pub struct HasBarTrait1 {} + impl BarTrait for HasBarTrait1 {} + + pub struct HasFoo1 {} + impl Foo for HasFoo1 { + type Bar = HasBarTrait1; + } + + pub struct HasBaz1 {} + impl Baz for HasBaz1 {} + + fn main() {} + \ No newline at end of file diff --git a/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src_hash.txt b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src_hash.txt new file mode 100644 index 00000000000..90789653f05 --- /dev/null +++ b/test_programs/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order/src_hash.txt @@ -0,0 +1 @@ +10160444482254094352 \ No newline at end of file diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index d1cbc9b504a..1855b28463c 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -212,7 +212,7 @@ const IGNORED_NARGO_EXPAND_COMPILE_SUCCESS_EMPTY_TESTS: [&str; 13] = [ /// These tests are ignored because of existing bugs in `nargo expand`. /// As the bugs are fixed these tests should be removed from this list. -const IGNORED_NARGO_EXPAND_COMPILE_SUCCESS_NO_BUG_TESTS: [&str; 13] = [ +const IGNORED_NARGO_EXPAND_COMPILE_SUCCESS_NO_BUG_TESTS: [&str; 15] = [ "noirc_frontend_tests_arithmetic_generics_checked_casts_do_not_prevent_canonicalization", "noirc_frontend_tests_check_trait_as_type_as_fn_parameter", "noirc_frontend_tests_check_trait_as_type_as_two_fn_parameters", @@ -224,12 +224,12 @@ const IGNORED_NARGO_EXPAND_COMPILE_SUCCESS_NO_BUG_TESTS: [&str; 13] = [ "noirc_frontend_tests_traits_accesses_associated_type_inside_trait_impl_using_self", "noirc_frontend_tests_traits_accesses_associated_type_inside_trait_using_self", "noirc_frontend_tests_u32_globals_as_sizes_in_types", - // "noirc_frontend_tests_traits_as_trait_path_called_multiple_times_for_different_t_1", - // "noirc_frontend_tests_traits_as_trait_path_called_multiple_times_for_different_t_2", // This creates a struct at comptime which, expanded, gives a visibility error "noirc_frontend_tests_visibility_visibility_bug_inside_comptime", // bug "noirc_frontend_tests_traits_associated_constant_sum_of_other_constants_3", + "noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order", + "noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_unexpected_order", ]; const IGNORED_NARGO_EXPAND_COMPILE_SUCCESS_WITH_BUG_TESTS: [&str; 1] = diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/execute__tests__expanded.snap new file mode 100644 index 00000000000..1c439bbe2b8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_bound_constraining_two_generics/execute__tests__expanded.snap @@ -0,0 +1,20 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +pub trait Foo {} + +pub trait Baz +where + T: Foo, +{} + +pub struct HasFoo1 {} + +impl Foo<()> for HasFoo1 {} + +pub struct HasBaz1 {} + +impl Baz for HasBaz1 {} + +fn main() {} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/execute__tests__expanded.snap new file mode 100644 index 00000000000..c0f363ee061 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_no_bug/noirc_frontend_tests_traits_trait_where_clause_associated_type_constraint_expected_order/execute__tests__expanded.snap @@ -0,0 +1,31 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +pub trait BarTrait {} + +pub trait Foo { + type Bar; +} + +pub trait Baz +where + T: Foo, + ::Bar: BarTrait, +{} + +pub struct HasBarTrait1 {} + +impl BarTrait for HasBarTrait1 {} + +pub struct HasFoo1 {} + +impl Foo<()> for HasFoo1 { + type Bar = HasBarTrait1; +} + +pub struct HasBaz1 {} + +impl Baz for HasBaz1 {} + +fn main() {}