PR feedback

Author: jfecher

compiler/noirc_frontend/src/elaborator/comptime.rs

@@ -19,7 +19,10 @@ use noirc_errors::Location;
 
 use crate::{
     Type, TypeBindings,
-    ast::{Documented, Expression, ExpressionKind, TypeImpl, UnresolvedGenerics, UnresolvedType},
+    ast::{
+        Documented, Expression, ExpressionKind, TypeImpl, UnresolvedGenerics,
+        UnresolvedTraitConstraint, UnresolvedType,
+    },
     hir::{
         comptime::{Interpreter, InterpreterError, Value},
         def_collector::{
@@ -58,6 +61,7 @@ struct AttributeContext {
 struct AttributeImplTarget {
     object_type: UnresolvedType,
     generics: UnresolvedGenerics,
+    where_clause: Vec<UnresolvedTraitConstraint>,
     type_location: Location,
 }
 
@@ -246,20 +250,7 @@ impl<'context> Elaborator<'context> {
         }
 
         self.collect_attributes_on_functions(functions, None, &mut attributes_to_run);
-        for ((object_type, _impl_module), impls_in_module) in impls {
-            for (generics, type_location, methods) in impls_in_module {
-                let impl_target = AttributeImplTarget {
-                    object_type: object_type.clone(),
-                    generics: generics.clone(),
-                    type_location: *type_location,
-                };
-                self.collect_attributes_on_functions(
-                    std::slice::from_ref(methods),
-                    Some(&impl_target),
-                    &mut attributes_to_run,
-                );
-            }
-        }
+        self.collect_attributes_on_impls(impls, &mut attributes_to_run);
         self.collect_attributes_on_modules(module_attributes, &mut attributes_to_run);
 
         self.sort_attributes_by_run_order(&mut attributes_to_run);
@@ -304,6 +295,29 @@ impl<'context> Elaborator<'context> {
         }
     }
 
+    #[tracing::instrument(level = "trace", skip_all)]
+    fn collect_attributes_on_impls(
+        &mut self,
+        impls: &ImplMap,
+        attributes_to_run: &mut CollectedAttributes,
+    ) {
+        for ((object_type, _impl_module), impls_in_module) in impls {
+            for (generics, where_clause, type_location, methods) in impls_in_module {
+                let impl_target = AttributeImplTarget {
+                    object_type: object_type.clone(),
+                    generics: generics.clone(),
+                    where_clause: where_clause.clone(),
+                    type_location: *type_location,
+                };
+                self.collect_attributes_on_functions(
+                    std::slice::from_ref(methods),
+                    Some(&impl_target),
+                    attributes_to_run,
+                );
+            }
+        }
+    }
+
     #[tracing::instrument(level = "trace", skip_all)]
     fn collect_attributes_on_functions(
         &mut self,
@@ -651,7 +665,7 @@ impl<'context> Elaborator<'context> {
                     object_type: target.object_type.clone(),
                     type_location: target.type_location,
                     generics: target.generics.clone(),
-                    where_clause: Vec::new(),
+                    where_clause: target.where_clause.clone(),
                     methods: vec![(Documented::new(function, item.doc_comments), location)],
                 };
                 let module = self.module_id();

compiler/noirc_frontend/src/elaborator/function.rs

@@ -134,11 +134,16 @@ impl Elaborator<'_> {
         &mut self,
         self_type: &UnresolvedType,
         local_module: LocalModuleId,
-        function_sets: &mut Vec<(UnresolvedGenerics, Location, UnresolvedFunctions)>,
+        function_sets: &mut Vec<(
+            UnresolvedGenerics,
+            Vec<UnresolvedTraitConstraint>,
+            Location,
+            UnresolvedFunctions,
+        )>,
     ) {
         self.local_module = Some(local_module);
 
-        for (generics, _, function_set) in function_sets {
+        for (generics, _, _, function_set) in function_sets {
             // Prepare the impl: adds the impl generics to scope so the self type can
             // reference them, then resolve the self type.
             self.add_generics(generics);

compiler/noirc_frontend/src/elaborator/impls.rs

@@ -80,7 +80,7 @@ use noirc_errors::Location;
 
 use crate::{
     Type,
-    ast::{UnresolvedGenerics, UnresolvedType},
+    ast::{UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType},
     hir::{
         def_collector::{dc_crate::UnresolvedFunctions, errors::DefCollectorErrorKind},
         def_map::LocalModuleId,
@@ -109,12 +109,17 @@ impl Elaborator<'_> {
     pub(super) fn collect_impls(
         &mut self,
         module: LocalModuleId,
-        impls: &mut [(UnresolvedGenerics, Location, UnresolvedFunctions)],
+        impls: &mut [(
+            UnresolvedGenerics,
+            Vec<UnresolvedTraitConstraint>,
+            Location,
+            UnresolvedFunctions,
+        )],
         self_type: &UnresolvedType,
     ) {
         self.local_module = Some(module);
 
-        for (generics, location, unresolved) in impls {
+        for (generics, _, location, unresolved) in impls {
             self.check_generics_appear_in_types(generics, &[self_type], &[]);
 
             self.recover_generics(|this| {

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -58,7 +58,7 @@ use std::{
 
 use crate::{
     Type,
-    ast::UnresolvedGenerics,
+    ast::{UnresolvedGenerics, UnresolvedTraitConstraint},
     elaborator::types::WildcardDisallowedContext,
     graph::CrateId,
     hir::{
@@ -805,8 +805,16 @@ impl<'context> Elaborator<'context> {
     }
 
     #[tracing::instrument(level = "trace", skip_all)]
-    fn elaborate_impls(&mut self, impls: Vec<(UnresolvedGenerics, Location, UnresolvedFunctions)>) {
-        for (_, _, functions) in impls {
+    fn elaborate_impls(
+        &mut self,
+        impls: Vec<(
+            UnresolvedGenerics,
+            Vec<UnresolvedTraitConstraint>,
+            Location,
+            UnresolvedFunctions,
+        )>,
+    ) {
+        for (_, _, _, functions) in impls {
             self.recover_generics(|this| this.elaborate_functions(functions));
         }
     }

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -187,7 +187,7 @@ impl CollectedItems {
 /// since it would be non-deterministic.
 pub(crate) type ImplMap = HashMap<
     (UnresolvedType, LocalModuleId),
-    Vec<(UnresolvedGenerics, Location, UnresolvedFunctions)>,
+    Vec<(UnresolvedGenerics, Vec<UnresolvedTraitConstraint>, Location, UnresolvedFunctions)>,
 >;
 
 /// Wraps a list of compilation errors.

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -1465,7 +1465,12 @@ pub fn collect_impl(
 
     let key = (r#impl.object_type, module_id.local_id);
     let methods = items.impls.entry(key).or_default();
-    methods.push((r#impl.generics, r#impl.type_location, unresolved_functions));
+    methods.push((
+        r#impl.generics,
+        r#impl.where_clause,
+        r#impl.type_location,
+        unresolved_functions,
+    ));
 }
 
 fn find_module(

compiler/noirc_frontend/src/tests/metaprogramming.rs

@@ -234,6 +234,37 @@ fn generate_function_with_macros_on_impl_method() {
     ");
 }
 
+// Regression for asterite's review on #12649: when an attribute on an impl method
+// generates a new function, the impl's `where_clause` must be carried into the
+// synthetic impl so the generated function can use the bounded generics.
+#[test]
+fn generate_function_with_macros_on_impl_method_carries_where_clause() {
+    let src = "
+    pub trait MyDefault {
+        fn my_default() -> Self;
+    }
+
+    pub struct Foo<T> {}
+
+    impl<T> Foo<T> where T: MyDefault {
+        #[generate_bar]
+        pub fn foo() {}
+    }
+
+    pub comptime fn generate_bar(_f: FunctionDefinition) -> Quoted {
+        quote {
+            pub fn bar() -> T {
+                T::my_default()
+            }
+        }
+    }
+
+    fn main() {}
+    ";
+
+    assert_no_errors(src);
+}
+
 #[test]
 fn generate_function_with_macros_on_trait() {
     let src = "