fix: don't always select trait impl when verifying trait constraints (#…

…7041)

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -775,7 +775,10 @@ impl<'context> Elaborator<'context> {
                             span,
                         },
                     };
-                    self.push_trait_constraint(constraint, expr_id);
+                    self.push_trait_constraint(
+                        constraint, expr_id,
+                        true, // this constraint should lead to choosing a trait impl
+                    );
                     self.type_check_operator_method(expr_id, trait_id, operand_type, span);
                 }
                 typ

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -197,7 +197,11 @@ struct FunctionContext {
     /// verified at the end of a function. This is because constraints arise
     /// on each variable, but it is only until function calls when the types
     /// needed for the trait constraint may become known.
-    trait_constraints: Vec<(TraitConstraint, ExprId)>,
+    /// The `select impl` bool indicates whether, after verifying the trait constraint,
+    /// the resulting trait impl should be the one used for a call (sometimes trait
+    /// constraints are verified but there's no call associated with them, like in the
+    /// case of checking generic arguments)
+    trait_constraints: Vec<(TraitConstraint, ExprId, bool /* select impl */)>,
 }
 
 impl<'context> Elaborator<'context> {
@@ -550,7 +554,7 @@ impl<'context> Elaborator<'context> {
             }
         }
 
-        for (mut constraint, expr_id) in context.trait_constraints {
+        for (mut constraint, expr_id, select_impl) in context.trait_constraints {
             let span = self.interner.expr_span(&expr_id);
 
             if matches!(&constraint.typ, Type::MutableReference(_)) {
@@ -565,6 +569,7 @@ impl<'context> Elaborator<'context> {
                 &constraint.trait_bound.trait_generics.ordered,
                 &constraint.trait_bound.trait_generics.named,
                 expr_id,
+                select_impl,
                 span,
             );
         }

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -751,7 +751,11 @@ impl<'context> Elaborator<'context> {
                 let function = self.interner.function_meta(&function);
                 for mut constraint in function.trait_constraints.clone() {
                     constraint.apply_bindings(&bindings);
-                    self.push_trait_constraint(constraint, expr_id);
+
+                    self.push_trait_constraint(
+                        constraint, expr_id,
+                        false, // This constraint shouldn't lead to choosing a trait impl method
+                    );
                 }
             }
         }
@@ -767,7 +771,11 @@ impl<'context> Elaborator<'context> {
                 // Currently only one impl can be selected per expr_id, so this
                 // constraint needs to be pushed after any other constraints so
                 // that monomorphization can resolve this trait method to the correct impl.
-                self.push_trait_constraint(method.constraint, expr_id);
+                self.push_trait_constraint(
+                    method.constraint,
+                    expr_id,
+                    true, // this constraint should lead to choosing a trait impl method
+                );
             }
         }
 

compiler/noirc_frontend/src/elaborator/traits.rs

@@ -132,7 +132,7 @@ impl<'context> Elaborator<'context> {
                     let func_id = unresolved_trait.method_ids[&name.0.contents];
                     let mut where_clause = where_clause.to_vec();
 
-                    // Attach any trait constraints on the trait to the function
+                    // Attach any trait constraints on the trait to the function,
                     where_clause.extend(unresolved_trait.trait_def.where_clause.clone());
 
                     this.resolve_trait_function(

compiler/noirc_frontend/src/elaborator/types.rs

@@ -1845,13 +1845,15 @@ impl<'context> Elaborator<'context> {
         (expr_span, empty_function)
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn verify_trait_constraint(
         &mut self,
         object_type: &Type,
         trait_id: TraitId,
         trait_generics: &[Type],
         associated_types: &[NamedType],
         function_ident_id: ExprId,
+        select_impl: bool,
         span: Span,
     ) {
         match self.interner.lookup_trait_implementation(
@@ -1861,7 +1863,9 @@ impl<'context> Elaborator<'context> {
             associated_types,
         ) {
             Ok(impl_kind) => {
-                self.interner.select_impl_for_expression(function_ident_id, impl_kind);
+                if select_impl {
+                    self.interner.select_impl_for_expression(function_ident_id, impl_kind);
+                }
             }
             Err(error) => self.push_trait_constraint_error(object_type, error, span),
         }
@@ -1935,10 +1939,15 @@ impl<'context> Elaborator<'context> {
 
     /// Push a trait constraint into the current FunctionContext to be solved if needed
     /// at the end of the earlier of either the current function or the current comptime scope.
-    pub fn push_trait_constraint(&mut self, constraint: TraitConstraint, expr_id: ExprId) {
+    pub fn push_trait_constraint(
+        &mut self,
+        constraint: TraitConstraint,
+        expr_id: ExprId,
+        select_impl: bool,
+    ) {
         let context = self.function_context.last_mut();
         let context = context.expect("The function_context stack should always be non-empty");
-        context.trait_constraints.push((constraint, expr_id));
+        context.trait_constraints.push((constraint, expr_id, select_impl));
     }
 
     pub fn bind_generics_from_trait_constraint(

test_programs/compile_success_empty/regression_7038/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_7038"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.30.0"
+
+[dependencies]

test_programs/compile_success_empty/regression_7038/src/main.nr

@@ -0,0 +1,40 @@
+trait BigNumTrait {}
+
+pub struct MyBigNum;
+
+impl crate::BigNumTrait for MyBigNum {}
+
+trait CurveParamsTrait<BigNum>
+where
+    BigNum: BigNumTrait,
+{
+    fn one();
+}
+
+pub struct BN254Params;
+impl CurveParamsTrait<MyBigNum> for BN254Params {
+
+    fn one() {}
+}
+
+trait BigCurveTrait {
+    fn two();
+}
+
+pub struct BigCurve<BigNum, CurveParams> {}
+
+type BN254 = BigCurve<MyBigNum, BN254Params>;
+
+impl<BigNum, CurveParams> BigCurveTrait for BigCurve<BigNum, CurveParams>
+where
+    BigNum: BigNumTrait,
+    CurveParams: CurveParamsTrait<BigNum>,
+{
+    fn two() {
+        let _ = CurveParams::one();
+    }
+}
+
+fn main() {
+    let _ = BN254::two();
+}

test_programs/compile_success_empty/regression_7038_2/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_7038_2"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.30.0"
+
+[dependencies]

test_programs/compile_success_empty/regression_7038_2/src/main.nr

@@ -0,0 +1,39 @@
+trait BigNumTrait {}
+
+pub struct MyBigNum;
+
+impl crate::BigNumTrait for MyBigNum {}
+
+trait CurveParamsTrait<BigNum>
+where
+    BigNum: BigNumTrait,
+{
+    // The difference between this and regression_7083 is that here
+    // this is a default method.
+    fn one() {}
+}
+
+pub struct BN254Params;
+impl CurveParamsTrait<MyBigNum> for BN254Params {}
+
+trait BigCurveTrait {
+    fn two();
+}
+
+pub struct BigCurve<BigNum, CurveParams> {}
+
+type BN254 = BigCurve<MyBigNum, BN254Params>;
+
+impl<BigNum, CurveParams> BigCurveTrait for BigCurve<BigNum, CurveParams>
+where
+    BigNum: BigNumTrait,
+    CurveParams: CurveParamsTrait<BigNum>,
+{
+    fn two() {
+        let _ = CurveParams::one();
+    }
+}
+
+fn main() {
+    let _ = BN254::two();
+}

test_programs/compile_success_empty/regression_7038_3/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_7038_3"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.30.0"
+
+[dependencies]

test_programs/compile_success_empty/regression_7038_3/src/main.nr

@@ -0,0 +1,40 @@
+trait BigNumTrait {}
+
+pub struct MyBigNum;
+
+impl crate::BigNumTrait for MyBigNum {}
+
+trait CurveParamsTrait<BigNum> {
+    // The difference between this and regression_7038 and regression_7038_2 is that
+    // here the where clause is on the method, not the trait
+    fn one()
+    where
+        BigNum: BigNumTrait;
+}
+
+pub struct BN254Params;
+impl CurveParamsTrait<MyBigNum> for BN254Params {
+    fn one() {}
+}
+
+trait BigCurveTrait {
+    fn two();
+}
+
+pub struct BigCurve<BigNum, CurveParams> {}
+
+type BN254 = BigCurve<MyBigNum, BN254Params>;
+
+impl<BigNum, CurveParams> BigCurveTrait for BigCurve<BigNum, CurveParams>
+where
+    BigNum: BigNumTrait,
+    CurveParams: CurveParamsTrait<BigNum>,
+{
+    fn two() {
+        let _ = CurveParams::one();
+    }
+}
+
+fn main() {
+    let _ = BN254::two();
+}

test_programs/compile_success_empty/regression_7038_4/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "regression_7038_4"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.30.0"
+
+[dependencies]

test_programs/compile_success_empty/regression_7038_4/src/main.nr

@@ -0,0 +1,29 @@
+// This program is a reduction of regression_7038_3 that led to a monomorphizer crash
+trait BigNumTrait {}
+
+trait CurveParamsTrait<BigNum> {
+    fn one()
+    where
+        BigNum: BigNumTrait;
+}
+
+pub struct MyBigNum;
+
+impl BigNumTrait for MyBigNum {}
+
+pub struct Params;
+impl CurveParamsTrait<MyBigNum> for Params {
+
+    fn one() {}
+}
+
+fn foo<C>()
+where
+    C: CurveParamsTrait<MyBigNum>,
+{
+    let _ = C::one();
+}
+
+fn main() {
+    foo::<Params>();
+}