From 2cb1ef8a0ca6f356f95895018231bf25632334ce Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 10:22:23 -0500
Subject: [PATCH 01/13] Fix where clause issue

---
 compiler/noirc_frontend/src/elaborator/mod.rs |  5 +++++
 .../src/hir/def_collector/dc_mod.rs           |  2 --
 .../regression_5671/Nargo.toml                |  7 +++++++
 .../regression_5671/src/main.nr               | 20 +++++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 test_programs/compile_success_empty/regression_5671/Nargo.toml
 create mode 100644 test_programs/compile_success_empty/regression_5671/src/main.nr

diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs
index e7f53ebb916..cca2af56664 100644
--- a/compiler/noirc_frontend/src/elaborator/mod.rs
+++ b/compiler/noirc_frontend/src/elaborator/mod.rs
@@ -1300,6 +1300,11 @@ impl<'context> Elaborator<'context> {
             self.add_generics(&trait_impl.generics);
             trait_impl.resolved_generics = self.generics.clone();
 
+            for (_, _, method) in trait_impl.methods.functions.iter_mut() {
+                // Attach any trait constraints on the impl to the function
+                method.def.where_clause.append(&mut trait_impl.where_clause.clone());
+            }
+
             // Fetch trait constraints here
             let trait_generics = trait_impl
                 .trait_id
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 9eef1d7b77e..be2afd13507 100644
--- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
+++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -173,8 +173,6 @@ impl<'a> ModCollector<'a> {
             let module = ModuleId { krate, local_id: self.module_id };
 
             for (_, func_id, noir_function) in &mut unresolved_functions.functions {
-                // Attach any trait constraints on the impl to the function
-                noir_function.def.where_clause.append(&mut trait_impl.where_clause.clone());
                 let location = Location::new(noir_function.def.span, self.file_id);
                 context.def_interner.push_function(*func_id, &noir_function.def, module, location);
             }
diff --git a/test_programs/compile_success_empty/regression_5671/Nargo.toml b/test_programs/compile_success_empty/regression_5671/Nargo.toml
new file mode 100644
index 00000000000..4ddf3413e5e
--- /dev/null
+++ b/test_programs/compile_success_empty/regression_5671/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "regression_5671"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.32.0"
+
+[dependencies]
diff --git a/test_programs/compile_success_empty/regression_5671/src/main.nr b/test_programs/compile_success_empty/regression_5671/src/main.nr
new file mode 100644
index 00000000000..2bac98ef7c4
--- /dev/null
+++ b/test_programs/compile_success_empty/regression_5671/src/main.nr
@@ -0,0 +1,20 @@
+#[foo]
+struct MyOtherStruct<A, B> {
+    field1: A,
+    field2: B,
+}
+
+comptime fn foo(_s: StructDefinition) -> Quoted {
+    quote {
+        impl<A, B> Eq for MyOtherStruct<A, B> where A: Eq, B: Eq {
+            fn eq(self, other: Self) -> bool {
+                (self.field1 == other.field1) & (self.field2 == other.field2)
+            }
+        }
+    }
+}
+
+fn main() {
+    let x = MyOtherStruct { field1: 1, field2: 2 };
+    assert_eq(x, x);
+}

From 8c6a57168374f19e6eb52ad23fb4c2bcff250b1c Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 10:34:28 -0500
Subject: [PATCH 02/13] Fix 'derive' for generic types

---
 aztec_macros/src/utils/parse_utils.rs         |  1 +
 compiler/noirc_frontend/src/ast/expression.rs | 19 ++++-
 compiler/noirc_frontend/src/elaborator/mod.rs | 77 ++++++++++++++-----
 .../src/hir/comptime/interpreter/builtin.rs   | 10 +--
 .../noirc_frontend/src/hir/comptime/value.rs  |  7 +-
 .../src/hir/resolution/errors.rs              |  9 +++
 compiler/noirc_frontend/src/hir_def/types.rs  |  6 ++
 .../src/parser/parser/function.rs             | 12 ++-
 .../noirc_frontend/src/parser/parser/types.rs |  2 +-
 noir_stdlib/src/cmp.nr                        |  6 +-
 noir_stdlib/src/default.nr                    |  4 +-
 noir_stdlib/src/meta/struct_def.nr            |  5 +-
 .../execution_success/derive/src/main.nr      | 10 +--
 tooling/nargo_fmt/src/utils.rs                |  3 +
 14 files changed, 128 insertions(+), 43 deletions(-)

diff --git a/aztec_macros/src/utils/parse_utils.rs b/aztec_macros/src/utils/parse_utils.rs
index 06093a4753e..a2c177026c4 100644
--- a/aztec_macros/src/utils/parse_utils.rs
+++ b/aztec_macros/src/utils/parse_utils.rs
@@ -349,6 +349,7 @@ fn empty_unresolved_generic(unresolved_generic: &mut UnresolvedGeneric) {
             empty_ident(ident);
             empty_unresolved_type(typ);
         }
+        UnresolvedGeneric::Resolved(..) => (),
     }
 }
 
diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs
index e176c7fc8b4..7a324eb2600 100644
--- a/compiler/noirc_frontend/src/ast/expression.rs
+++ b/compiler/noirc_frontend/src/ast/expression.rs
@@ -7,7 +7,7 @@ use crate::ast::{
 };
 use crate::hir::def_collector::errors::DefCollectorErrorKind;
 use crate::macros_api::StructId;
-use crate::node_interner::ExprId;
+use crate::node_interner::{ExprId, QuotedTypeId};
 use crate::token::{Attributes, Token, Tokens};
 use crate::{Kind, Type};
 use acvm::{acir::AcirField, FieldElement};
@@ -51,7 +51,16 @@ pub type UnresolvedGenerics = Vec<UnresolvedGeneric>;
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub enum UnresolvedGeneric {
     Variable(Ident),
-    Numeric { ident: Ident, typ: UnresolvedType },
+    Numeric {
+        ident: Ident,
+        typ: UnresolvedType,
+    },
+
+    /// Already-resolved generics can be parsed as generics when a macro
+    /// splices existing types into a generic list. In this case we have
+    /// to validate the type refers to a named generic and treat that
+    /// as a ResolvedGeneric when this is resolved.
+    Resolved(QuotedTypeId, Span),
 }
 
 impl UnresolvedGeneric {
@@ -61,6 +70,7 @@ impl UnresolvedGeneric {
             UnresolvedGeneric::Numeric { ident, typ } => {
                 ident.0.span().merge(typ.span.unwrap_or_default())
             }
+            UnresolvedGeneric::Resolved(_, span) => *span,
         }
     }
 
@@ -71,6 +81,9 @@ impl UnresolvedGeneric {
                 let typ = self.resolve_numeric_kind_type(typ)?;
                 Ok(Kind::Numeric(Box::new(typ)))
             }
+            UnresolvedGeneric::Resolved(..) => {
+                panic!("Don't know the kind of a resolved generic here")
+            }
         }
     }
 
@@ -94,6 +107,7 @@ impl UnresolvedGeneric {
     pub(crate) fn ident(&self) -> &Ident {
         match self {
             UnresolvedGeneric::Variable(ident) | UnresolvedGeneric::Numeric { ident, .. } => ident,
+            UnresolvedGeneric::Resolved(..) => panic!("UnresolvedGeneric::Resolved no ident"),
         }
     }
 }
@@ -103,6 +117,7 @@ impl Display for UnresolvedGeneric {
         match self {
             UnresolvedGeneric::Variable(ident) => write!(f, "{ident}"),
             UnresolvedGeneric::Numeric { ident, typ } => write!(f, "let {ident}: {typ}"),
+            UnresolvedGeneric::Resolved(..) => write!(f, "(resolved)"),
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs
index cca2af56664..e6b8350a744 100644
--- a/compiler/noirc_frontend/src/elaborator/mod.rs
+++ b/compiler/noirc_frontend/src/elaborator/mod.rs
@@ -502,35 +502,72 @@ impl<'context> Elaborator<'context> {
     /// Each generic will have a fresh Shared<TypeBinding> associated with it.
     pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics {
         vecmap(generics, |generic| {
-            // Map the generic to a fresh type variable
-            let id = self.interner.next_type_variable_id();
-            let typevar = TypeVariable::unbound(id);
-            let ident = generic.ident();
-            let span = ident.0.span();
+            let mut is_error = false;
+            let (type_var, name, kind) = match self.resolve_generic(generic) {
+                Ok(values) => values,
+                Err(error) => {
+                    self.push_err(error);
+                    is_error = true;
+                    let id = self.interner.next_type_variable_id();
+                    (TypeVariable::unbound(id), Rc::new("(error)".into()), Kind::Normal)
+                }
+            };
 
-            // Resolve the generic's kind
-            let kind = self.resolve_generic_kind(generic);
+            let span = generic.span();
+            let name_owned = name.as_ref().clone();
+            let resolved_generic = ResolvedGeneric { name, type_var, kind, span };
 
             // Check for name collisions of this generic
-            let name = Rc::new(ident.0.contents.clone());
-
-            let resolved_generic =
-                ResolvedGeneric { name: name.clone(), type_var: typevar.clone(), kind, span };
-
-            if let Some(generic) = self.find_generic(&name) {
-                self.push_err(ResolverError::DuplicateDefinition {
-                    name: ident.0.contents.clone(),
-                    first_span: generic.span,
-                    second_span: span,
-                });
-            } else {
-                self.generics.push(resolved_generic.clone());
+            // Checking `is_error` here prevents DuplicateDefinition errors when
+            // we have multiple generics from macros which fail to resolve and
+            // are all given the same default name "(error)".
+            if !is_error {
+                if let Some(generic) = self.find_generic(&name_owned) {
+                    self.push_err(ResolverError::DuplicateDefinition {
+                        name: name_owned,
+                        first_span: generic.span,
+                        second_span: span,
+                    });
+                } else {
+                    self.generics.push(resolved_generic.clone());
+                }
             }
 
             resolved_generic
         })
     }
 
+    fn resolve_generic(
+        &mut self,
+        generic: &UnresolvedGeneric,
+    ) -> Result<(TypeVariable, Rc<String>, Kind), ResolverError> {
+        // Map the generic to a fresh type variable
+        match generic {
+            UnresolvedGeneric::Variable(_) | UnresolvedGeneric::Numeric { .. } => {
+                let id = self.interner.next_type_variable_id();
+                let typevar = TypeVariable::unbound(id);
+                let ident = generic.ident();
+
+                let kind = self.resolve_generic_kind(generic);
+                let name = Rc::new(ident.0.contents.clone());
+                Ok((typevar, name, kind))
+            }
+            // An already-resolved generic is only possible if it is the result of a
+            // previous macro call being inserted into a generics list.
+            UnresolvedGeneric::Resolved(id, span) => {
+                match self.interner.get_quoted_type(*id).follow_bindings() {
+                    Type::NamedGeneric(type_variable, name, kind) => {
+                        Ok((type_variable, name, kind))
+                    }
+                    other => Err(ResolverError::MacroResultInGenericsListNotAGeneric {
+                        span: *span,
+                        typ: other.clone(),
+                    }),
+                }
+            }
+        }
+    }
+
     /// Return the kind of an unresolved generic.
     /// If a numeric generic has been specified, resolve the annotated type to make
     /// sure only primitive numeric types are being used.
diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
index 76f85740195..5d2bb9544a5 100644
--- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
+++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
@@ -249,7 +249,7 @@ fn struct_def_as_type(
     Ok(Value::Type(Type::Struct(struct_def_rc, generics)))
 }
 
-/// fn generics(self) -> [Quoted]
+/// fn generics(self) -> [Type]
 fn struct_def_generics(
     interner: &NodeInterner,
     mut arguments: Vec<(Value, Location)>,
@@ -269,12 +269,10 @@ fn struct_def_generics(
     let struct_def = interner.get_struct(struct_def);
     let struct_def = struct_def.borrow();
 
-    let generics = struct_def.generics.iter().map(|generic| {
-        let name = Token::Ident(generic.type_var.borrow().to_string());
-        Value::Quoted(Rc::new(vec![name]))
-    });
+    let generics =
+        struct_def.generics.iter().map(|generic| Value::Type(generic.clone().as_named_generic()));
 
-    let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Quoted)));
+    let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Type)));
     Ok(Value::Slice(generics.collect(), typ))
 }
 
diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs
index 61475fa60e1..d89873077fa 100644
--- a/compiler/noirc_frontend/src/hir/comptime/value.rs
+++ b/compiler/noirc_frontend/src/hir/comptime/value.rs
@@ -497,7 +497,12 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
             Value::Quoted(tokens) => {
                 write!(f, "quote {{")?;
                 for token in tokens.iter() {
-                    write!(f, " {token}")?;
+                    match token {
+                        Token::QuotedType(id) => {
+                            write!(f, " {:?}", self.interner.get_quoted_type(*id))?;
+                        }
+                        other => write!(f, " {other}")?,
+                    }
                 }
                 write!(f, " }}")
             }
diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs
index 1cc1abfa495..cfaa2063c40 100644
--- a/compiler/noirc_frontend/src/hir/resolution/errors.rs
+++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs
@@ -114,6 +114,8 @@ pub enum ResolverError {
     MacroIsNotComptime { span: Span },
     #[error("Annotation name must refer to a comptime function")]
     NonFunctionInAnnotation { span: Span },
+    #[error("Type `{typ}` was inserted into the generics list from a macro, but is not a generic")]
+    MacroResultInGenericsListNotAGeneric { span: Span, typ: Type },
 }
 
 impl ResolverError {
@@ -458,6 +460,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                     *span,
                 )
             },
+            ResolverError::MacroResultInGenericsListNotAGeneric { span, typ } => {
+                Diagnostic::simple_error(
+                    format!("Type `{typ}` was inserted into a generics list from a macro, but it is not a generic"),
+                    format!("Type `{typ}` is not a generic"),
+                    *span,
+                )
+            }
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs
index fc1af63540a..8083f8754b3 100644
--- a/compiler/noirc_frontend/src/hir_def/types.rs
+++ b/compiler/noirc_frontend/src/hir_def/types.rs
@@ -191,6 +191,12 @@ pub struct ResolvedGeneric {
     pub span: Span,
 }
 
+impl ResolvedGeneric {
+    pub fn as_named_generic(self) -> Type {
+        Type::NamedGeneric(self.type_var, self.name, self.kind)
+    }
+}
+
 impl std::hash::Hash for StructType {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.id.hash(state);
diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs
index 2fd337e1cb1..3de48d2e02a 100644
--- a/compiler/noirc_frontend/src/parser/parser/function.rs
+++ b/compiler/noirc_frontend/src/parser/parser/function.rs
@@ -2,9 +2,10 @@ use super::{
     attributes::{attributes, validate_attributes},
     block, fresh_statement, ident, keyword, maybe_comp_time, nothing, optional_visibility,
     parameter_name_recovery, parameter_recovery, parenthesized, parse_type, pattern,
+    primitives::token_kind,
     self_parameter, where_clause, NoirParser,
 };
-use crate::token::{Keyword, Token};
+use crate::token::{Keyword, Token, TokenKind};
 use crate::{ast::IntegerBitSize, parser::spanned};
 use crate::{
     ast::{
@@ -110,8 +111,15 @@ pub(super) fn generic_type() -> impl NoirParser<UnresolvedGeneric> {
     ident().map(UnresolvedGeneric::Variable)
 }
 
+pub(super) fn resolved_generic() -> impl NoirParser<UnresolvedGeneric> {
+    token_kind(TokenKind::QuotedType).map_with_span(|token, span| match token {
+        Token::QuotedType(id) => UnresolvedGeneric::Resolved(id, span),
+        _ => unreachable!("token_kind(QuotedType) guarantees we parse a quoted type"),
+    })
+}
+
 pub(super) fn generic() -> impl NoirParser<UnresolvedGeneric> {
-    generic_type().or(numeric_generic())
+    generic_type().or(numeric_generic()).or(resolved_generic())
 }
 
 /// non_empty_ident_list: ident ',' non_empty_ident_list
diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs
index fca29bf1e7a..7c2bdcb9fa3 100644
--- a/compiler/noirc_frontend/src/parser/parser/types.rs
+++ b/compiler/noirc_frontend/src/parser/parser/types.rs
@@ -133,7 +133,7 @@ fn quoted_type() -> impl NoirParser<UnresolvedType> {
 /// This is the type of an already resolved type.
 /// The only way this can appear in the token input is if an already resolved `Type` object
 /// was spliced into a macro's token stream via the `$` operator.
-fn resolved_type() -> impl NoirParser<UnresolvedType> {
+pub(super) fn resolved_type() -> impl NoirParser<UnresolvedType> {
     token_kind(TokenKind::QuotedType).map_with_span(|token, span| match token {
         Token::QuotedType(id) => UnresolvedTypeData::Resolved(id).with_span(span),
         _ => unreachable!("token_kind(QuotedType) guarantees we parse a quoted type"),
diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index d2cf6b3836a..3dfde287aa5 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -10,9 +10,11 @@ trait Eq {
 comptime fn derive_eq(s: StructDefinition) -> Quoted {
     let typ = s.as_type();
 
-    let impl_generics = s.generics().join(quote {,});
+    let impl_generics = s.generics()
+        .map(|g| quote { $g })
+        .join(quote {,});
 
-    let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});
+    let where_clause = s.generics().map(|name| quote { $name: Eq }).join(quote {,});
 
     // `(self.a == other.a) & (self.b == other.b) & ...`
     let equalities = s.fields().map(
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index f0d98205a90..77cd130c698 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -10,7 +10,9 @@ trait Default {
 comptime fn derive_default(s: StructDefinition) -> Quoted {
     let typ = s.as_type();
 
-    let impl_generics = s.generics().join(quote {,});
+    let impl_generics = s.generics()
+        .map(|g| quote { $g })
+        .join(quote {,});
 
     let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});
 
diff --git a/noir_stdlib/src/meta/struct_def.nr b/noir_stdlib/src/meta/struct_def.nr
index 7382aeb8e75..8d3f9ceb8a5 100644
--- a/noir_stdlib/src/meta/struct_def.nr
+++ b/noir_stdlib/src/meta/struct_def.nr
@@ -4,10 +4,9 @@ impl StructDefinition {
     #[builtin(struct_def_as_type)]
     fn as_type(self) -> Type {}
 
-    /// Return each generic on this struct. The names of these generics are unchanged
-    /// so users may need to keep name collisions in mind if this is used directly in a macro.
+    /// Return each generic on this struct.
     #[builtin(struct_def_generics)]
-    fn generics(self) -> [Quoted] {}
+    fn generics(self) -> [Type] {}
 
     /// Returns (name, type) pairs of each field in this struct. Each type is as-is
     /// with any generic arguments unchanged.
diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr
index e4148f2c944..0bc139a0ddd 100644
--- a/test_programs/execution_success/derive/src/main.nr
+++ b/test_programs/execution_success/derive/src/main.nr
@@ -8,7 +8,7 @@ struct MyStruct { my_field: u32 }
 
 comptime fn derive_do_nothing(s: StructDefinition) -> Quoted {
     let typ = s.as_type();
-    let generics = s.generics().join(quote {,});
+    let generics = s.generics().map(|g| quote { $g }).join(quote {,});
     quote {
         impl<$generics> DoNothing for $typ {
             fn do_nothing(_self: Self) {
@@ -21,15 +21,15 @@ comptime fn derive_do_nothing(s: StructDefinition) -> Quoted {
 
 // Test stdlib derive fns & multiple traits
 #[derive(Eq, Default)]
-struct MyOtherStruct {
-    field1: u32,
-    field2: u64,
+struct MyOtherStruct<A, B> {
+    field1: A,
+    field2: B,
 }
 
 fn main() {
     let s = MyStruct { my_field: 1 };
     s.do_nothing();
 
-    let o = MyOtherStruct::default();
+    let o: MyOtherStruct<Field, u32> = MyOtherStruct::default();
     assert_eq(o, o);
 }
diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs
index 293351055e1..83634b718e2 100644
--- a/tooling/nargo_fmt/src/utils.rs
+++ b/tooling/nargo_fmt/src/utils.rs
@@ -188,6 +188,9 @@ impl HasItem for UnresolvedGeneric {
                 result.push_str(&typ);
                 result
             }
+            UnresolvedGeneric::Resolved(..) => {
+                unreachable!("Found macro result UnresolvedGeneric::Resolved in formatter")
+            }
         }
     }
 }

From 2f1ec2b90b20463163a47bf99a531d061b233cbb Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 10:36:10 -0500
Subject: [PATCH 03/13] Add another generic

---
 test_programs/execution_success/derive/src/main.nr | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr
index 0bc139a0ddd..4bb89789c9e 100644
--- a/test_programs/execution_success/derive/src/main.nr
+++ b/test_programs/execution_success/derive/src/main.nr
@@ -32,4 +32,7 @@ fn main() {
 
     let o: MyOtherStruct<Field, u32> = MyOtherStruct::default();
     assert_eq(o, o);
+
+    let o: MyOtherStruct<u8, [str<2>]> = MyOtherStruct::default();
+    assert_eq(o, o);
 }

From 5277cfc4c2b014243415c7568156e2e90cf7e334 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 10:48:30 -0500
Subject: [PATCH 04/13] Format stdlib

---
 noir_stdlib/src/cmp.nr     | 4 +---
 noir_stdlib/src/default.nr | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index 3dfde287aa5..94cd284e238 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -10,9 +10,7 @@ trait Eq {
 comptime fn derive_eq(s: StructDefinition) -> Quoted {
     let typ = s.as_type();
 
-    let impl_generics = s.generics()
-        .map(|g| quote { $g })
-        .join(quote {,});
+    let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
 
     let where_clause = s.generics().map(|name| quote { $name: Eq }).join(quote {,});
 
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index 77cd130c698..4fbde09b512 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -10,9 +10,7 @@ trait Default {
 comptime fn derive_default(s: StructDefinition) -> Quoted {
     let typ = s.as_type();
 
-    let impl_generics = s.generics()
-        .map(|g| quote { $g })
-        .join(quote {,});
+    let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
 
     let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});
 

From f610fbfd7153217fb037c6877b5f7160d68ca8af Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 11:41:18 -0500
Subject: [PATCH 05/13] Fix test

---
 test_programs/compile_success_empty/derive_impl/src/main.nr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test_programs/compile_success_empty/derive_impl/src/main.nr b/test_programs/compile_success_empty/derive_impl/src/main.nr
index 62b8a0421e6..69cb641e7c7 100644
--- a/test_programs/compile_success_empty/derive_impl/src/main.nr
+++ b/test_programs/compile_success_empty/derive_impl/src/main.nr
@@ -1,5 +1,5 @@
 comptime fn derive_default(typ: StructDefinition) -> Quoted {
-    let generics: [Quoted] = typ.generics();
+    let generics = typ.generics();
     assert_eq(
         generics.len(), 0, "derive_default: Deriving Default on generic types is currently unimplemented"
     );

From bfd0bca97ff7e6a88c2d0a84f0688979cfff7798 Mon Sep 17 00:00:00 2001
From: jfecher <jake@aztecprotocol.com>
Date: Fri, 2 Aug 2024 12:03:54 -0500
Subject: [PATCH 06/13] Update
 compiler/noirc_frontend/src/hir/comptime/value.rs

Co-authored-by: Michael J Klein <michaeljklein@users.noreply.github.com>
---
 compiler/noirc_frontend/src/hir/comptime/value.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs
index d89873077fa..1264cd21635 100644
--- a/compiler/noirc_frontend/src/hir/comptime/value.rs
+++ b/compiler/noirc_frontend/src/hir/comptime/value.rs
@@ -499,7 +499,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
                 for token in tokens.iter() {
                     match token {
                         Token::QuotedType(id) => {
-                            write!(f, " {:?}", self.interner.get_quoted_type(*id))?;
+                            write!(f, " {}", self.interner.get_quoted_type(*id))?;
                         }
                         other => write!(f, " {other}")?,
                     }

From 42829e8542885edcd28bcd10cb4d4f18463ab5be Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Fri, 2 Aug 2024 12:06:40 -0500
Subject: [PATCH 07/13] Add MyOtherOtherStruct

---
 test_programs/execution_success/derive/src/main.nr | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr
index 4bb89789c9e..f344defe41e 100644
--- a/test_programs/execution_success/derive/src/main.nr
+++ b/test_programs/execution_success/derive/src/main.nr
@@ -24,6 +24,12 @@ comptime fn derive_do_nothing(s: StructDefinition) -> Quoted {
 struct MyOtherStruct<A, B> {
     field1: A,
     field2: B,
+    field3: MyOtherOtherStruct<B>,
+}
+
+#[derive(Eq, Default)]
+struct MyOtherOtherStruct<T> {
+    x: T,
 }
 
 fn main() {

From 2b8df546cd3df7cc2aa0434a9ada4bae9f62d059 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Mon, 5 Aug 2024 12:36:33 -0500
Subject: [PATCH 08/13] Shelve work; need closures

---
 noir_stdlib/src/cmp.nr      | 61 ++++++++++++++++++++++++-------------
 noir_stdlib/src/default.nr  | 26 +++-------------
 noir_stdlib/src/meta/mod.nr | 40 ++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 44 deletions(-)

diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index 94cd284e238..204cf8c0189 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -8,28 +8,10 @@ trait Eq {
 // docs:end:eq-trait
 
 comptime fn derive_eq(s: StructDefinition) -> Quoted {
-    let typ = s.as_type();
-
-    let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
-
-    let where_clause = s.generics().map(|name| quote { $name: Eq }).join(quote {,});
-
-    // `(self.a == other.a) & (self.b == other.b) & ...`
-    let equalities = s.fields().map(
-        |f: (Quoted, Type)| {
-        let name = f.0;
-        quote { (self.$name == other.$name) }
-    }
-    );
-    let body = equalities.join(quote { & });
-
-    quote {
-        impl<$impl_generics> Eq for $typ where $where_clause {
-            fn eq(self, other: Self) -> bool {
-                $body
-            }
-        }
-    }
+    let signature = quote { fn eq(self, other: Self) -> bool };
+    let for_each_field = |name| quote { (self.$name == other.$name) };
+    let join_fields_with = quote { & };
+    crate::meta::make_impl(s, quote { Eq }, signature, for_each_field, join_fields_with)
 }
 
 impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } }
@@ -127,12 +109,47 @@ impl Ordering {
     }
 }
 
+// #[derive_via(derive_ord)]
 // docs:start:ord-trait
 trait Ord {
     fn cmp(self, other: Self) -> Ordering;
 }
 // docs:end:ord-trait
 
+// comptime fn derive_ord(s: StructDefinition) -> Quoted {
+//     let typ = s.as_type();
+// 
+//     let impl_generics = s.generics().join(quote {,});
+// 
+//     let where_clause = s.generics().map(|name| quote { $name: Ord }).join(quote {,});
+// 
+//     // For each field:
+//     // if result == Ordering::equal() {
+//     //     result = self.$field.cmp(other.$field);
+//     // }
+//     let body = s.fields().map(
+//         |f: (Quoted, Type)| {
+//         let name = f.0;
+//         quote {
+//             if result == Ordering::equal() {
+//                 result = self.$name.cmp(other.$name);
+//             }
+//         }
+//     }
+//     );
+//     let body = equalities.join(quote{});
+// 
+//     quote {
+//         impl<$impl_generics> Eq for $typ where $where_clause {
+//             fn cmp(self, other: Self) -> Ordering {
+//                 let mut result = Ordering::equal();
+//                 $body
+//                 result
+//             }
+//         }
+//     }
+// }
+
 // Note: Field deliberately does not implement Ord
 
 impl Ord for u64 {
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index 4fbde09b512..afbf8159e95 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -8,28 +8,10 @@ trait Default {
 // docs:end:default-trait
 
 comptime fn derive_default(s: StructDefinition) -> Quoted {
-    let typ = s.as_type();
-
-    let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
-
-    let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});
-
-    // `foo: Default::default(), bar: Default::default(), ...`
-    let fields = s.fields().map(
-        |f: (Quoted, Type)| {
-        let name = f.0;
-        quote { $name: Default::default() }
-    }
-    );
-    let fields = fields.join(quote {,});
-
-    quote {
-        impl<$impl_generics> Default for $typ where $where_clause {
-            fn default() -> Self {
-                Self { $fields }
-            }
-        }
-    }
+    let signature = quote { fn default() -> Self };
+    let for_each_field = |name| quote { $name: Default::default() };
+    let join_fields_with = quote { , };
+    crate::meta::make_impl(s, quote { Default }, signature, for_each_field, join_fields_with)
 }
 
 impl Default for Field { fn default() -> Field { 0 } }
diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr
index 7ed5e3ff44f..8639aa6e379 100644
--- a/noir_stdlib/src/meta/mod.nr
+++ b/noir_stdlib/src/meta/mod.nr
@@ -41,3 +41,43 @@ pub comptime fn derive(s: StructDefinition, traits: [TraitDefinition]) -> Quoted
 unconstrained pub comptime fn derive_via(t: TraitDefinition, f: DeriveFunction) {
     HANDLERS.insert(t, f);
 }
+
+/// `make_impl` is a helper function to make a simple impl, usually while deriving a trait.
+/// This impl has a couple assumptions:
+/// 1. The impl only has one function, with the signature `function_signature`
+/// 2. The trait itself does not have any generics.
+///
+/// While these assumptions are met, `make_impl` will create an impl from a StructDefinition,
+/// automatically filling in the required generics from the struct, along with the where clause.
+/// The function body is created by mapping each field with `for_each_field` and joining the
+/// results with `join_fields_with`.
+///
+/// See `derive_eq` and `derive_default` for example usage.
+pub comptime fn make_impl<Env>(
+    s: StructDefinition,
+    trait_name: Quoted,
+    function_signature: Quoted,
+    for_each_field: fn[Env](Quoted) -> Quoted,
+    join_fields_with: Quoted,
+) -> Quoted {
+    let typ = s.as_type();
+
+    let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
+
+    let where_clause = s.generics().map(|name| quote { $name: $trait_name }).join(quote {,});
+
+    // `for_each_field(field1) $join_fields_with for_each_field(field2) $join_fields_with ...`
+    let equalities = s.fields().map(|f: (Quoted, Type)| {
+        let name = f.0;
+        for_each_field(name)
+    });
+    let body = equalities.join(join_fields_with);
+
+    quote {
+        impl<$impl_generics> $trait_name for $typ where $where_clause {
+            $function_signature {
+                $body
+            }
+        }
+    }
+}

From 966be8ed4095b3316f392aa724de0127dd3828d4 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Mon, 5 Aug 2024 13:06:22 -0500
Subject: [PATCH 09/13] Comptime closures

---
 .../src/hir/comptime/interpreter.rs           |  7 +++-
 .../comptime_closures/Nargo.toml              |  7 ++++
 .../comptime_closures/src/main.nr             | 39 +++++++++++++++++++
 3 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 test_programs/compile_success_empty/comptime_closures/Nargo.toml
 create mode 100644 test_programs/compile_success_empty/comptime_closures/src/main.nr

diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs
index 0bb53432b78..72b92e288c7 100644
--- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs
+++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs
@@ -226,8 +226,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
     fn call_closure(
         &mut self,
         closure: HirLambda,
-        // TODO: How to define environment here?
-        _environment: Vec<Value>,
+        environment: Vec<Value>,
         arguments: Vec<(Value, Location)>,
         call_location: Location,
     ) -> IResult<Value> {
@@ -246,6 +245,10 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
             self.define_pattern(parameter, typ, argument, arg_location)?;
         }
 
+        for (param, arg) in closure.captures.into_iter().zip(environment) {
+            self.define(param.ident.id, arg);
+        }
+
         let result = self.evaluate(closure.body)?;
 
         self.exit_function(previous_state);
diff --git a/test_programs/compile_success_empty/comptime_closures/Nargo.toml b/test_programs/compile_success_empty/comptime_closures/Nargo.toml
new file mode 100644
index 00000000000..50cde2ec2dd
--- /dev/null
+++ b/test_programs/compile_success_empty/comptime_closures/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "comptime_closures"
+type = "bin"
+authors = [""]
+compiler_version = ">=0.32.0"
+
+[dependencies]
diff --git a/test_programs/compile_success_empty/comptime_closures/src/main.nr b/test_programs/compile_success_empty/comptime_closures/src/main.nr
new file mode 100644
index 00000000000..95f602e04bf
--- /dev/null
+++ b/test_programs/compile_success_empty/comptime_closures/src/main.nr
@@ -0,0 +1,39 @@
+fn main() {
+    comptime
+    {
+        closure_test(0);
+    }
+}
+
+fn closure_test(mut x: Field) {
+    let one = 1;
+    let add1 = |z| {
+        (|| {
+            *z += one;
+        })()
+    };
+
+    let two = 2;
+    let add2 = |z| {
+        *z = *z + two;
+    };
+
+    add1(&mut x);
+    assert(x == 1);
+
+    add2(&mut x);
+    assert(x == 3);
+
+    issue_2120();
+}
+
+fn issue_2120() {
+    let x1 = &mut 42;
+    let set_x1 = |y| { *x1 = y; };
+
+    assert(*x1 == 42);
+    set_x1(44);
+    assert(*x1 == 44);
+    set_x1(*x1);
+    assert(*x1 == 44);
+}

From cae1d1542740e912c5f938148d889def0298e920 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Mon, 5 Aug 2024 13:57:26 -0500
Subject: [PATCH 10/13] Format

---
 noir_stdlib/src/cmp.nr      | 24 ++++++++++++++++--------
 noir_stdlib/src/default.nr  | 13 ++++++++-----
 noir_stdlib/src/hash/mod.nr | 13 ++++++++-----
 noir_stdlib/src/meta/mod.nr | 12 ++++++------
 4 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index b1fb554f1d6..e864464ced7 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -8,11 +8,14 @@ trait Eq {
 // docs:end:eq-trait
 
 comptime fn derive_eq(s: StructDefinition) -> Quoted {
-    let signature = quote { fn eq(_self: Self, _other: Self) -> bool };
-    let for_each_field = |name| quote { (_self.$name == _other.$name) };
-    let join_fields_with = quote { & };
-    let body = |fields| fields;
-    crate::meta::make_impl(s, quote { Eq }, signature, for_each_field, join_fields_with, body)
+    crate::meta::make_impl(
+        s,
+        quote { Eq },
+        quote { fn eq(_self: Self, _other: Self) -> bool },
+        |name| quote { (_self.$name == _other.$name) },
+        quote { & },
+        |fields| fields,
+    )
 }
 
 impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } }
@@ -118,19 +121,24 @@ trait Ord {
 // docs:end:ord-trait
 
 comptime fn derive_ord(s: StructDefinition) -> Quoted {
-    let signature = quote { fn cmp(_self: Self, _other: Self) -> std::cmp::Ordering };
     let for_each_field = |name| quote {
         if result == std::cmp::Ordering::equal() {
             result = _self.$name.cmp(_other.$name);
         }
     };
-    let join_fields_with = quote {};
     let body = |fields| quote {
         let mut result = std::cmp::Ordering::equal();
         $fields
         result
     };
-    crate::meta::make_impl(s, quote { Ord }, signature, for_each_field, join_fields_with, body)
+    crate::meta::make_impl(
+        s,
+        quote { Ord },
+        quote { fn cmp(_self: Self, _other: Self) -> std::cmp::Ordering },
+        for_each_field,
+        quote {},
+        body
+    )
 }
 
 // Note: Field deliberately does not implement Ord
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index 8ca788f0e64..183793aa779 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -8,11 +8,14 @@ trait Default {
 // docs:end:default-trait
 
 comptime fn derive_default(s: StructDefinition) -> Quoted {
-    let signature = quote { fn default() -> Self };
-    let for_each_field = |name| quote { $name: Default::default() };
-    let join_fields_with = quote { , };
-    let body = |fields| quote { Self { $fields } };
-    crate::meta::make_impl(s, quote { Default }, signature, for_each_field, join_fields_with, body)
+    crate::meta::make_impl(
+        s,
+        quote { Default },
+        quote { fn default() -> Self },
+        |name| quote { $name: Default::default() },
+        quote { , },
+        |fields| quote { Self { $fields } },
+    )
 }
 
 impl Default for Field { fn default() -> Field { 0 } }
diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr
index 93dd45f67aa..06841b8d271 100644
--- a/noir_stdlib/src/hash/mod.nr
+++ b/noir_stdlib/src/hash/mod.nr
@@ -148,11 +148,14 @@ trait Hash {
 }
 
 comptime fn derive_hash(s: StructDefinition) -> Quoted {
-    let signature = quote { fn hash<H>(_self: Self, _state: &mut H) where H: std::hash::Hasher };
-    let for_each_field = |name| quote { _self.$name.hash(_state); };
-    let join_fields_with = quote {};
-    let body = |fields| fields;
-    crate::meta::make_impl(s, quote { Hash }, signature, for_each_field, join_fields_with, body)
+    crate::meta::make_impl(
+        s,
+        quote { Hash },
+        quote { fn hash<H>(_self: Self, _state: &mut H) where H: std::hash::Hasher },
+        |name| quote { _self.$name.hash(_state); },
+        quote {},
+        |fields| fields,
+    )
 }
 
 // Hasher trait shall be implemented by algorithms to provide hash-agnostic means.
diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr
index 8bbcde4f759..839460c82ab 100644
--- a/noir_stdlib/src/meta/mod.nr
+++ b/noir_stdlib/src/meta/mod.nr
@@ -60,20 +60,20 @@ pub comptime fn make_impl<Env1, Env2>(
     function_signature: Quoted,
     for_each_field: fn[Env1](Quoted) -> Quoted,
     join_fields_with: Quoted,
-    body: fn[Env2](Quoted) -> Quoted,
+    body: fn[Env2](Quoted) -> Quoted
 ) -> Quoted {
     let typ = s.as_type();
-
     let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});
-
     let where_clause = s.generics().map(|name| quote { $name: $trait_name }).join(quote {,});
 
     // `for_each_field(field1) $join_fields_with for_each_field(field2) $join_fields_with ...`
-    let equalities = s.fields().map(|f: (Quoted, Type)| {
+    let fields = s.fields().map(
+        |f: (Quoted, Type)| {
         let name = f.0;
         for_each_field(name)
-    });
-    let body = body(equalities.join(join_fields_with));
+    }
+    );
+    let body = body(fields.join(join_fields_with));
 
     quote {
         impl<$impl_generics> $trait_name for $typ where $where_clause {

From 5f99c5edee943fdb08277ad1627bbcdfcadec3e7 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Mon, 5 Aug 2024 14:38:50 -0500
Subject: [PATCH 11/13] Format

---
 noir_stdlib/src/cmp.nr      | 2 +-
 noir_stdlib/src/default.nr  | 2 +-
 noir_stdlib/src/hash/mod.nr | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index e864464ced7..b1f9eb47ceb 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -14,7 +14,7 @@ comptime fn derive_eq(s: StructDefinition) -> Quoted {
         quote { fn eq(_self: Self, _other: Self) -> bool },
         |name| quote { (_self.$name == _other.$name) },
         quote { & },
-        |fields| fields,
+        |fields| fields
     )
 }
 
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index 183793aa779..22a6d3f3b39 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -14,7 +14,7 @@ comptime fn derive_default(s: StructDefinition) -> Quoted {
         quote { fn default() -> Self },
         |name| quote { $name: Default::default() },
         quote { , },
-        |fields| quote { Self { $fields } },
+        |fields| quote { Self { $fields } }
     )
 }
 
diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr
index 06841b8d271..d93ef17f50b 100644
--- a/noir_stdlib/src/hash/mod.nr
+++ b/noir_stdlib/src/hash/mod.nr
@@ -154,7 +154,7 @@ comptime fn derive_hash(s: StructDefinition) -> Quoted {
         quote { fn hash<H>(_self: Self, _state: &mut H) where H: std::hash::Hasher },
         |name| quote { _self.$name.hash(_state); },
         quote {},
-        |fields| fields,
+        |fields| fields
     )
 }
 

From 2c23b9e2e08ec855c90d610917db83b4d177b6de Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Mon, 5 Aug 2024 15:10:52 -0500
Subject: [PATCH 12/13] Format

---
 test_programs/execution_success/derive/src/main.nr | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr
index efbffd4e584..92d43d2523a 100644
--- a/test_programs/execution_success/derive/src/main.nr
+++ b/test_programs/execution_success/derive/src/main.nr
@@ -46,11 +46,7 @@ fn main() {
     assert_eq(o, o);
 
     // Field & str<2> above don't implement Ord
-    let o = MyOtherStruct {
-        field1: 12 as u32,
-        field2: 24 as i8,
-        field3: MyOtherOtherStruct { x: 54 as i8 },
-    };
+    let o = MyOtherStruct { field1: 12 as u32, field2: 24 as i8, field3: MyOtherOtherStruct { x: 54 as i8 } };
     assert_eq(o.cmp(o), std::cmp::Ordering::equal());
 
     let mut hasher = TestHasher { result: 0 };

From 6c1b1297d78eb992dd8cbf5ada49753a49562c66 Mon Sep 17 00:00:00 2001
From: Jake Fecher <jfecher11@gmail.com>
Date: Tue, 6 Aug 2024 11:13:24 -0500
Subject: [PATCH 13/13] PR feedback

---
 noir_stdlib/src/cmp.nr                        | 22 +++++--------------
 noir_stdlib/src/default.nr                    | 13 +++++------
 noir_stdlib/src/hash/mod.nr                   | 12 ++++------
 noir_stdlib/src/meta/mod.nr                   |  2 +-
 .../execution_success/derive/src/main.nr      |  9 ++++----
 5 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr
index b1f9eb47ceb..10182ca83b0 100644
--- a/noir_stdlib/src/cmp.nr
+++ b/noir_stdlib/src/cmp.nr
@@ -8,14 +8,10 @@ trait Eq {
 // docs:end:eq-trait
 
 comptime fn derive_eq(s: StructDefinition) -> Quoted {
-    crate::meta::make_impl(
-        s,
-        quote { Eq },
-        quote { fn eq(_self: Self, _other: Self) -> bool },
-        |name| quote { (_self.$name == _other.$name) },
-        quote { & },
-        |fields| fields
-    )
+    let signature = quote { fn eq(_self: Self, _other: Self) -> bool };
+    let for_each_field = |name| quote { (_self.$name == _other.$name) };
+    let body = |fields| fields;
+    crate::meta::make_trait_impl(s, quote { Eq }, signature, for_each_field, quote { & }, body)
 }
 
 impl Eq for Field { fn eq(self, other: Field) -> bool { self == other } }
@@ -121,6 +117,7 @@ trait Ord {
 // docs:end:ord-trait
 
 comptime fn derive_ord(s: StructDefinition) -> Quoted {
+    let signature = quote { fn cmp(_self: Self, _other: Self) -> std::cmp::Ordering };
     let for_each_field = |name| quote {
         if result == std::cmp::Ordering::equal() {
             result = _self.$name.cmp(_other.$name);
@@ -131,14 +128,7 @@ comptime fn derive_ord(s: StructDefinition) -> Quoted {
         $fields
         result
     };
-    crate::meta::make_impl(
-        s,
-        quote { Ord },
-        quote { fn cmp(_self: Self, _other: Self) -> std::cmp::Ordering },
-        for_each_field,
-        quote {},
-        body
-    )
+    crate::meta::make_trait_impl(s, quote { Ord }, signature, for_each_field, quote {}, body)
 }
 
 // Note: Field deliberately does not implement Ord
diff --git a/noir_stdlib/src/default.nr b/noir_stdlib/src/default.nr
index 22a6d3f3b39..f9399bfb865 100644
--- a/noir_stdlib/src/default.nr
+++ b/noir_stdlib/src/default.nr
@@ -8,14 +8,11 @@ trait Default {
 // docs:end:default-trait
 
 comptime fn derive_default(s: StructDefinition) -> Quoted {
-    crate::meta::make_impl(
-        s,
-        quote { Default },
-        quote { fn default() -> Self },
-        |name| quote { $name: Default::default() },
-        quote { , },
-        |fields| quote { Self { $fields } }
-    )
+    let name = quote { Default };
+    let signature = quote { fn default() -> Self };
+    let for_each_field = |name| quote { $name: Default::default() };
+    let body = |fields| quote { Self { $fields } };
+    crate::meta::make_trait_impl(s, name, signature, for_each_field, quote { , }, body)
 }
 
 impl Default for Field { fn default() -> Field { 0 } }
diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr
index d93ef17f50b..84ad7c22bb3 100644
--- a/noir_stdlib/src/hash/mod.nr
+++ b/noir_stdlib/src/hash/mod.nr
@@ -148,14 +148,10 @@ trait Hash {
 }
 
 comptime fn derive_hash(s: StructDefinition) -> Quoted {
-    crate::meta::make_impl(
-        s,
-        quote { Hash },
-        quote { fn hash<H>(_self: Self, _state: &mut H) where H: std::hash::Hasher },
-        |name| quote { _self.$name.hash(_state); },
-        quote {},
-        |fields| fields
-    )
+    let name = quote { Hash };
+    let signature = quote { fn hash<H>(_self: Self, _state: &mut H) where H: std::hash::Hasher };
+    let for_each_field = |name| quote { _self.$name.hash(_state); };
+    crate::meta::make_trait_impl(s, name, signature, for_each_field, quote {}, |fields| fields)
 }
 
 // Hasher trait shall be implemented by algorithms to provide hash-agnostic means.
diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr
index 839460c82ab..14aebc2bca1 100644
--- a/noir_stdlib/src/meta/mod.nr
+++ b/noir_stdlib/src/meta/mod.nr
@@ -54,7 +54,7 @@ unconstrained pub comptime fn derive_via(t: TraitDefinition, f: DeriveFunction)
 /// any final processing - e.g. wrapping each field in a `StructConstructor { .. }` expression.
 ///
 /// See `derive_eq` and `derive_default` for example usage.
-pub comptime fn make_impl<Env1, Env2>(
+pub comptime fn make_trait_impl<Env1, Env2>(
     s: StructDefinition,
     trait_name: Quoted,
     function_signature: Quoted,
diff --git a/test_programs/execution_success/derive/src/main.nr b/test_programs/execution_success/derive/src/main.nr
index 92d43d2523a..5ec2fb32a79 100644
--- a/test_programs/execution_success/derive/src/main.nr
+++ b/test_programs/execution_success/derive/src/main.nr
@@ -46,12 +46,13 @@ fn main() {
     assert_eq(o, o);
 
     // Field & str<2> above don't implement Ord
-    let o = MyOtherStruct { field1: 12 as u32, field2: 24 as i8, field3: MyOtherOtherStruct { x: 54 as i8 } };
-    assert_eq(o.cmp(o), std::cmp::Ordering::equal());
+    let o1 = MyOtherStruct { field1: 12 as u32, field2: 24 as i8, field3: MyOtherOtherStruct { x: 54 as i8 } };
+    let o2 = MyOtherStruct { field1: 12 as u32, field2: 24 as i8, field3: MyOtherOtherStruct { x: 55 as i8 } };
+    assert(o1 < o2);
 
     let mut hasher = TestHasher { result: 0 };
-    o.hash(&mut hasher);
-    assert(hasher.finish() != 0);
+    o1.hash(&mut hasher);
+    assert_eq(hasher.finish(), 12 + 24 + 54);
 }
 
 struct TestHasher {