From fbda87a04d9f64848a8489f14e65faab0163e34e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 May 2025 10:17:57 -0300 Subject: [PATCH 01/12] fix: general solution for accessing associated constants --- .../noirc_frontend/src/elaborator/patterns.rs | 32 ++++++++++++++----- .../src/hir/comptime/interpreter.rs | 3 ++ .../src/monomorphization/mod.rs | 18 +++++++++++ compiler/noirc_frontend/src/node_interner.rs | 3 ++ .../trait_associated_constant/Nargo.toml | 5 +++ .../trait_associated_constant/src/main.nr | 20 ++++++++++++ .../src/cli/expand_cmd/printer/hir.rs | 3 ++ 7 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 test_programs/execution_success/trait_associated_constant/Nargo.toml create mode 100644 test_programs/execution_success/trait_associated_constant/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 19545906568..7536f0f33ad 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -19,7 +19,6 @@ use crate::{ hir_def::{ expr::{HirExpression, HirIdent, HirLiteral, HirMethodReference, ImplKind, TraitMethod}, stmt::HirPattern, - traits::NamedType, }, node_interner::{ DefinitionId, DefinitionInfo, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind, @@ -653,6 +652,7 @@ impl Elaborator<'_> { return None; } + let location = variable.location; let name = variable.segments[1].ident.as_str(); // Check the `Self::AssociatedConstant` case when inside a trait @@ -664,7 +664,7 @@ impl Elaborator<'_> { // produce code (only trait impl methods do) let numeric_type: Type = *numeric_type.clone(); let value = SignedField::zero(); - return Some(self.constant_integer(numeric_type, value, variable.location)); + return Some(self.constant_integer(numeric_type, value, location)); } } } @@ -680,12 +680,25 @@ impl Elaborator<'_> { // Check the `Self::AssociatedConstant` case when inside a trait impl let associated_types = self.interner.get_associated_types_for_impl(*trait_impl_id); let associated_type = associated_types.iter().find(|typ| typ.name.as_str() == name); - if let Some(NamedType { typ: Type::Constant(field, Kind::Numeric(numeric_type)), .. }) = - associated_type - { - let numeric_type: Type = *numeric_type.clone(); - let value = SignedField::positive(*field); - return Some(self.constant_integer(numeric_type, value, variable.location)); + if let Some(associated_type) = associated_type { + if let Kind::Numeric(numeric_type) = associated_type.typ.kind() { + let definition_id = self.interner.push_definition( + associated_type.name.to_string(), + false, + false, + DefinitionKind::AssociatedConstant( + *trait_impl_id, + associated_type.name.to_string(), + ), + location, + ); + let hir_ident = HirIdent::non_trait_method(definition_id, location); + let hir_expr = HirExpression::Ident(hir_ident, None); + let id = self.interner.push_expr(hir_expr); + self.interner.push_expr_location(id, location); + self.interner.push_expr_type(id, *numeric_type.clone()); + return Some((id, *numeric_type.clone())); + } } // Check the `Self::method_name` case when `Self` is a primitive type @@ -964,6 +977,9 @@ impl Elaborator<'_> { self.interner.add_local_reference(hir_ident.id, location); } + DefinitionKind::AssociatedConstant(..) => { + // TODO + } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 605c46115ac..6d929263824 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -636,6 +636,9 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { self.evaluate_integer(value.into(), id) } + DefinitionKind::AssociatedConstant(..) => { + todo!() + } } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 6ca3df03977..78e4df63ff5 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1078,6 +1078,24 @@ impl<'interner> Monomorphizer<'interner> { let value = SignedField::positive(value); ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) } + DefinitionKind::AssociatedConstant(trait_impl_id, name) => { + let location = ident.location; + let associated_types = self.interner.get_associated_types_for_impl(*trait_impl_id); + let associated_type = associated_types + .iter() + .find(|typ| typ.name.as_str() == name) + .expect("Expected to find associated type"); + let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { + unreachable!("Expected associated type to be numeric"); + }; + let field = associated_type + .typ + .evaluate_to_field_element(&associated_type.typ.kind(), location) + .expect("Expected to be able to evaluate associated type"); + let typ = Self::convert_type(&numeric_type, location)?; + let value = SignedField::positive(field); + ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) + } }; Ok(ident) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index bcb5dd269c3..f9fe4c07af7 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -597,6 +597,8 @@ pub enum DefinitionKind { /// Generic types in functions (T, U in `fn foo(...)` are declared as variables /// in scope in case they resolve to numeric generics later. NumericGeneric(TypeVariable, Box), + + AssociatedConstant(TraitImplId, String), } impl DefinitionKind { @@ -612,6 +614,7 @@ impl DefinitionKind { DefinitionKind::Global(_) => None, DefinitionKind::Local(id) => *id, DefinitionKind::NumericGeneric(_, _) => None, + DefinitionKind::AssociatedConstant(..) => None, } } } diff --git a/test_programs/execution_success/trait_associated_constant/Nargo.toml b/test_programs/execution_success/trait_associated_constant/Nargo.toml new file mode 100644 index 00000000000..c0fd0400c52 --- /dev/null +++ b/test_programs/execution_success/trait_associated_constant/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "trait_associated_constant" +type = "bin" +authors = [""] +[dependencies] diff --git a/test_programs/execution_success/trait_associated_constant/src/main.nr b/test_programs/execution_success/trait_associated_constant/src/main.nr new file mode 100644 index 00000000000..78ce876acb2 --- /dev/null +++ b/test_programs/execution_success/trait_associated_constant/src/main.nr @@ -0,0 +1,20 @@ +pub trait Trait { + let N: u32; + + fn foo() -> u32; +} + +struct Foo {} + +impl Trait for Foo { + let N: u32 = A + B; + + fn foo() -> u32 { + Self::N + } +} + +fn main() { + let x = Foo::<10, 20>::foo(); + assert_eq(x, 30); +} diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs index e5cf1e96ea0..47070bba523 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs @@ -728,6 +728,9 @@ impl ItemPrinter<'_, '_, '_> { self.push_str(&name); } + DefinitionKind::AssociatedConstant(..) => { + todo!() + } } } From 7c1ddea14a3c7dc9b57f9682fa41071976ac2e7c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 11:59:04 -0300 Subject: [PATCH 02/12] Fixes --- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 4 +-- .../src/cli/expand_cmd/printer/hir.rs | 16 ++++----- ...ig_false_inliner_-9223372036854775808.snap | 26 ++++++++++++++ ..._tests__force_brillig_false_inliner_0.snap | 26 ++++++++++++++ ...lig_false_inliner_9223372036854775807.snap | 26 ++++++++++++++ ...lig_true_inliner_-9223372036854775808.snap | 36 +++++++++++++++++++ ...__tests__force_brillig_true_inliner_0.snap | 36 +++++++++++++++++++ ...llig_true_inliner_9223372036854775807.snap | 36 +++++++++++++++++++ .../execute__tests__expanded.snap.new | 25 +++++++++++++ 10 files changed, 222 insertions(+), 11 deletions(-) create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_0.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_9223372036854775807.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_0.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_9223372036854775807.snap create mode 100644 tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index fe347661195..92c13e69197 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1490,7 +1490,7 @@ impl Type { } } - pub(crate) fn kind(&self) -> Kind { + pub fn kind(&self) -> Kind { match self { Type::CheckedCast { to, .. } => to.kind(), Type::NamedGeneric(NamedGeneric { type_var, .. }) => type_var.kind(), diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 9e63af16732..6367cbba0d3 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -424,11 +424,11 @@ impl<'interner, 'def_map, 'string> ItemPrinter<'interner, 'def_map, 'string> { self.write_indent(); - if let Type::Constant(_, Kind::Numeric(numeric_type)) = &named_type.typ { + if let Kind::Numeric(numeric_type) = named_type.typ.kind() { self.push_str("let "); self.push_str(&named_type.name.to_string()); self.push_str(": "); - self.show_type(numeric_type); + self.show_type(&numeric_type); self.push_str(" = "); } else { self.push_str("type "); diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs index 47070bba523..9bfef135693 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs @@ -684,24 +684,24 @@ impl ItemPrinter<'_, '_, '_> { fn show_hir_ident(&mut self, ident: HirIdent) { let definition = self.interner.definition(ident.id); - match definition.kind { + match &definition.kind { DefinitionKind::Function(func_id) => { - let func_meta = self.interner.function_meta(&func_id); + let func_meta = self.interner.function_meta(func_id); if func_meta.self_type.is_some() && func_meta.self_type == self.self_type { // No need to fully-qualify the function name if its self type is the current self type - let name = self.interner.function_name(&func_id); + let name = self.interner.function_name(func_id); self.push_str("Self::"); self.push_str(name); } else { let use_import = true; self.show_reference_to_module_def_id( - ModuleDefId::FunctionId(func_id), + ModuleDefId::FunctionId(*func_id), use_import, ); } } DefinitionKind::Global(global_id) => { - let global_info = self.interner.get_global(global_id); + let global_info = self.interner.get_global(*global_id); let typ = self.interner.definition_type(global_info.definition_id); // Special case: the global is an enum value @@ -714,7 +714,7 @@ impl ItemPrinter<'_, '_, '_> { } } let use_import = true; - self.show_reference_to_module_def_id(ModuleDefId::GlobalId(global_id), use_import); + self.show_reference_to_module_def_id(ModuleDefId::GlobalId(*global_id), use_import); } DefinitionKind::Local(..) | DefinitionKind::NumericGeneric(..) => { let name = self.interner.definition_name(ident.id); @@ -728,8 +728,8 @@ impl ItemPrinter<'_, '_, '_> { self.push_str(&name); } - DefinitionKind::AssociatedConstant(..) => { - todo!() + DefinitionKind::AssociatedConstant(_, name) => { + self.push_str(name); } } } diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_0.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_0.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_false_inliner_9223372036854775807.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_0.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_0.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__force_brillig_true_inliner_9223372036854775807.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new b/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new new file mode 100644 index 00000000000..56261099ca9 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new @@ -0,0 +1,25 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 377 +expression: expanded_code +--- +pub trait Trait { + let N: u32; + + fn foo() -> u32; +} + +struct Foo {} + +impl Trait for Foo { + let N: u32 = A + B; + + fn foo() -> u32 { + N + } +} + +fn main() { + let x: u32 = Trait::foo(); + assert(x == 30); +} From 02cb03e8e34b92b19cf288e5a2a7283a68fb70d5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 13:52:42 -0300 Subject: [PATCH 03/12] Ignore new nargo_expand test --- tooling/nargo_cli/build.rs | 4 ++- .../execute__tests__expanded.snap.new | 25 ------------------- 2 files changed, 3 insertions(+), 26 deletions(-) delete mode 100644 tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index c6afa5dc3bd..ecb00881e5a 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -117,7 +117,7 @@ const TESTS_WITH_EXPECTED_WARNINGS: [&str; 4] = [ /// might not be worth it. /// Others 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_EXECUTION_TESTS: [&str; 8] = [ +const IGNORED_NARGO_EXPAND_EXECUTION_TESTS: [&str; 9] = [ // There's nothing special about this program but making it work with a custom entry would involve // having to parse the Nargo.toml file, etc., which is not worth it "custom_entry", @@ -131,6 +131,8 @@ const IGNORED_NARGO_EXPAND_EXECUTION_TESTS: [&str; 8] = [ "regression_5045", // bug "regression_7744", + // bug + "trait_associated_constant", // There's no "src/main.nr" here so it's trickier to make this work "workspace", // There's no "src/main.nr" here so it's trickier to make this work diff --git a/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new b/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new deleted file mode 100644 index 56261099ca9..00000000000 --- a/tooling/nargo_cli/tests/snapshots/expand/execution_success/trait_associated_constant/execute__tests__expanded.snap.new +++ /dev/null @@ -1,25 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 377 -expression: expanded_code ---- -pub trait Trait { - let N: u32; - - fn foo() -> u32; -} - -struct Foo {} - -impl Trait for Foo { - let N: u32 = A + B; - - fn foo() -> u32 { - N - } -} - -fn main() { - let x: u32 = Trait::foo(); - assert(x == 30); -} From dc09a567634b90491693a326ca8d757b1cc55e9e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 14:57:11 -0300 Subject: [PATCH 04/12] Wrap associated types in type variables --- .../noirc_frontend/src/elaborator/patterns.rs | 12 +++++----- .../src/hir/comptime/interpreter.rs | 3 --- .../src/monomorphization/mod.rs | 18 --------------- compiler/noirc_frontend/src/node_interner.rs | 22 +++++++++++++++---- .../src/cli/expand_cmd/printer/hir.rs | 3 --- 5 files changed, 23 insertions(+), 35 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 7536f0f33ad..9f6c2ba8481 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -682,14 +682,15 @@ impl Elaborator<'_> { let associated_type = associated_types.iter().find(|typ| typ.name.as_str() == name); if let Some(associated_type) = associated_type { if let Kind::Numeric(numeric_type) = associated_type.typ.kind() { + let Type::TypeVariable(type_var) = associated_type.typ.clone() else { + panic!("Expected associated type to be a type variable") + }; + let definition_id = self.interner.push_definition( associated_type.name.to_string(), false, false, - DefinitionKind::AssociatedConstant( - *trait_impl_id, - associated_type.name.to_string(), - ), + DefinitionKind::NumericGeneric(type_var, numeric_type.clone()), location, ); let hir_ident = HirIdent::non_trait_method(definition_id, location); @@ -977,9 +978,6 @@ impl Elaborator<'_> { self.interner.add_local_reference(hir_ident.id, location); } - DefinitionKind::AssociatedConstant(..) => { - // TODO - } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 6d929263824..605c46115ac 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -636,9 +636,6 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { self.evaluate_integer(value.into(), id) } - DefinitionKind::AssociatedConstant(..) => { - todo!() - } } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 78e4df63ff5..6ca3df03977 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1078,24 +1078,6 @@ impl<'interner> Monomorphizer<'interner> { let value = SignedField::positive(value); ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) } - DefinitionKind::AssociatedConstant(trait_impl_id, name) => { - let location = ident.location; - let associated_types = self.interner.get_associated_types_for_impl(*trait_impl_id); - let associated_type = associated_types - .iter() - .find(|typ| typ.name.as_str() == name) - .expect("Expected to find associated type"); - let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { - unreachable!("Expected associated type to be numeric"); - }; - let field = associated_type - .typ - .evaluate_to_field_element(&associated_type.typ.kind(), location) - .expect("Expected to be able to evaluate associated type"); - let typ = Self::convert_type(&numeric_type, location)?; - let value = SignedField::positive(field); - ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) - } }; Ok(ident) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index f9fe4c07af7..73ad47e3374 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -597,8 +597,6 @@ pub enum DefinitionKind { /// Generic types in functions (T, U in `fn foo(...)` are declared as variables /// in scope in case they resolve to numeric generics later. NumericGeneric(TypeVariable, Box), - - AssociatedConstant(TraitImplId, String), } impl DefinitionKind { @@ -614,7 +612,6 @@ impl DefinitionKind { DefinitionKind::Global(_) => None, DefinitionKind::Local(id) => *id, DefinitionKind::NumericGeneric(_, _) => None, - DefinitionKind::AssociatedConstant(..) => None, } } } @@ -2273,14 +2270,31 @@ impl NodeInterner { self.lsp_mode } + /// Sets the associated types for the given trait impl. + /// Each type in [`NamedType`] will be wrapped in a [`Type::TypeVariable`]. pub fn set_associated_types_for_impl( &mut self, impl_id: TraitImplId, - associated_types: Vec, + mut associated_types: Vec, ) { + // Wrap the named generics in type variables to be able to refer them as type variables + for associated_type in &mut associated_types { + let mut wrapper = self.next_type_variable_with_kind(associated_type.typ.kind()); + let Type::TypeVariable(type_variable) = &mut wrapper else { + unreachable!( + "Expected `next_type_variable_with_kind` to create a `Type::TypeVariable`" + ); + }; + let named_generic_type = std::mem::replace(&mut associated_type.typ, Type::Error); + type_variable.bind(named_generic_type); + associated_type.typ = wrapper; + } + self.trait_impl_associated_types.insert(impl_id, associated_types); } + /// Returns the associated types for the given trait impl. + /// The Type of each [`NamedType`] is guaranteed to be a [`Type::TypeVariable`] pub fn get_associated_types_for_impl(&self, impl_id: TraitImplId) -> &[NamedType] { &self.trait_impl_associated_types[&impl_id] } diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs index 9bfef135693..3bd5d26d60f 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs @@ -728,9 +728,6 @@ impl ItemPrinter<'_, '_, '_> { self.push_str(&name); } - DefinitionKind::AssociatedConstant(_, name) => { - self.push_str(name); - } } } From 0502b3f956445bfc76c124e93bd4651983555c58 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 15:15:15 -0300 Subject: [PATCH 05/12] Register definition IDs for each associated constant --- .../noirc_frontend/src/elaborator/patterns.rs | 12 ++---- compiler/noirc_frontend/src/node_interner.rs | 39 +++++++++++++++++-- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 9f6c2ba8481..5dbdff83c3d 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -682,17 +682,11 @@ impl Elaborator<'_> { let associated_type = associated_types.iter().find(|typ| typ.name.as_str() == name); if let Some(associated_type) = associated_type { if let Kind::Numeric(numeric_type) = associated_type.typ.kind() { - let Type::TypeVariable(type_var) = associated_type.typ.clone() else { + let Type::TypeVariable(type_variable) = associated_type.typ.clone() else { panic!("Expected associated type to be a type variable") }; - - let definition_id = self.interner.push_definition( - associated_type.name.to_string(), - false, - false, - DefinitionKind::NumericGeneric(type_var, numeric_type.clone()), - location, - ); + let definition_id = + self.interner.get_associated_constant_definition_id(type_variable.id()); let hir_ident = HirIdent::non_trait_method(definition_id, location); let hir_expr = HirExpression::Ident(hir_ident, None); let id = self.interner.push_expr(hir_expr); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 73ad47e3374..10d469fd9c9 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -146,6 +146,10 @@ pub struct NodeInterner { /// created, when resolving the type signature of each method in the impl. trait_impl_associated_types: HashMap>, + /// Maps each type variable ID of each trait impl associated constant to it's definition, + /// whose kind is [`DefinitionKind::NumericGeneric`]. + trait_impl_associated_constant_definition_ids: HashMap, + /// Trait implementations on each type. This is expected to always have the same length as /// `self.trait_implementations`. /// @@ -717,6 +721,7 @@ impl Default for NodeInterner { auto_import_names: HashMap::default(), comptime_scopes: vec![HashMap::default()], trait_impl_associated_types: HashMap::default(), + trait_impl_associated_constant_definition_ids: HashMap::default(), doc_comments: HashMap::default(), reexports: HashMap::default(), } @@ -2271,30 +2276,48 @@ impl NodeInterner { } /// Sets the associated types for the given trait impl. - /// Each type in [`NamedType`] will be wrapped in a [`Type::TypeVariable`]. - pub fn set_associated_types_for_impl( + /// Each type in [`NamedType`] will be wrapped in a [`Type::TypeVariable`] if it's of kind [`Kind::Numeric`]. + pub(crate) fn set_associated_types_for_impl( &mut self, impl_id: TraitImplId, mut associated_types: Vec, ) { // Wrap the named generics in type variables to be able to refer them as type variables for associated_type in &mut associated_types { - let mut wrapper = self.next_type_variable_with_kind(associated_type.typ.kind()); + let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { + continue; + }; + + let mut wrapper = + self.next_type_variable_with_kind(Kind::Numeric(numeric_type.clone())); let Type::TypeVariable(type_variable) = &mut wrapper else { unreachable!( "Expected `next_type_variable_with_kind` to create a `Type::TypeVariable`" ); }; + let type_variable = type_variable.clone(); + let type_variable_id = type_variable.id(); + let named_generic_type = std::mem::replace(&mut associated_type.typ, Type::Error); type_variable.bind(named_generic_type); associated_type.typ = wrapper; + + let definition_id = self.push_definition( + associated_type.name.to_string(), + false, + false, + DefinitionKind::NumericGeneric(type_variable, numeric_type), + associated_type.name.location(), + ); + self.trait_impl_associated_constant_definition_ids + .insert(type_variable_id, definition_id); } self.trait_impl_associated_types.insert(impl_id, associated_types); } /// Returns the associated types for the given trait impl. - /// The Type of each [`NamedType`] is guaranteed to be a [`Type::TypeVariable`] + /// The Type of each [`NamedType`] that is an associated constant is guaranteed to be a [`Type::TypeVariable`]. pub fn get_associated_types_for_impl(&self, impl_id: TraitImplId) -> &[NamedType] { &self.trait_impl_associated_types[&impl_id] } @@ -2308,6 +2331,14 @@ impl NodeInterner { types.iter().find(|typ| typ.name.as_str() == type_name).map(|typ| &typ.typ) } + /// Returns the definition id for the associated constant of the given type variable. + pub fn get_associated_constant_definition_id( + &self, + type_variable_id: TypeVariableId, + ) -> DefinitionId { + self.trait_impl_associated_constant_definition_ids[&type_variable_id] + } + /// Return a set of TypeBindings to bind types from the parent trait to those from the trait impl. pub fn trait_to_impl_bindings( &self, From cfcd411d8def9659d60e6389307a66946ec3d270 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 15:49:51 -0300 Subject: [PATCH 06/12] Don't use type variables --- .../noirc_frontend/src/elaborator/patterns.rs | 8 ++-- .../src/hir/comptime/interpreter.rs | 3 ++ .../src/monomorphization/mod.rs | 18 +++++++++ compiler/noirc_frontend/src/node_interner.rs | 40 ++++++++----------- .../src/cli/expand_cmd/printer/hir.rs | 4 +- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 5dbdff83c3d..74d0fb795f2 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -682,11 +682,8 @@ impl Elaborator<'_> { let associated_type = associated_types.iter().find(|typ| typ.name.as_str() == name); if let Some(associated_type) = associated_type { if let Kind::Numeric(numeric_type) = associated_type.typ.kind() { - let Type::TypeVariable(type_variable) = associated_type.typ.clone() else { - panic!("Expected associated type to be a type variable") - }; let definition_id = - self.interner.get_associated_constant_definition_id(type_variable.id()); + self.interner.get_associated_constant_definition_id(*trait_impl_id, name); let hir_ident = HirIdent::non_trait_method(definition_id, location); let hir_expr = HirExpression::Ident(hir_ident, None); let id = self.interner.push_expr(hir_expr); @@ -972,6 +969,9 @@ impl Elaborator<'_> { self.interner.add_local_reference(hir_ident.id, location); } + DefinitionKind::AssociatedConstant(..) => { + // Nothing to do here + } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 605c46115ac..1b497668a95 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -636,6 +636,9 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { self.evaluate_integer(value.into(), id) } + DefinitionKind::AssociatedConstant(..) => { + todo!("Implement accessing an associated constant in the comptime interpreter") + } } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 6ca3df03977..78e4df63ff5 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1078,6 +1078,24 @@ impl<'interner> Monomorphizer<'interner> { let value = SignedField::positive(value); ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) } + DefinitionKind::AssociatedConstant(trait_impl_id, name) => { + let location = ident.location; + let associated_types = self.interner.get_associated_types_for_impl(*trait_impl_id); + let associated_type = associated_types + .iter() + .find(|typ| typ.name.as_str() == name) + .expect("Expected to find associated type"); + let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { + unreachable!("Expected associated type to be numeric"); + }; + let field = associated_type + .typ + .evaluate_to_field_element(&associated_type.typ.kind(), location) + .expect("Expected to be able to evaluate associated type"); + let typ = Self::convert_type(&numeric_type, location)?; + let value = SignedField::positive(field); + ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) + } }; Ok(ident) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 10d469fd9c9..8b89a889bb1 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -146,9 +146,8 @@ pub struct NodeInterner { /// created, when resolving the type signature of each method in the impl. trait_impl_associated_types: HashMap>, - /// Maps each type variable ID of each trait impl associated constant to it's definition, - /// whose kind is [`DefinitionKind::NumericGeneric`]. - trait_impl_associated_constant_definition_ids: HashMap, + trait_impl_associated_constant_definition_ids: + HashMap>, /// Trait implementations on each type. This is expected to always have the same length as /// `self.trait_implementations`. @@ -601,6 +600,8 @@ pub enum DefinitionKind { /// Generic types in functions (T, U in `fn foo(...)` are declared as variables /// in scope in case they resolve to numeric generics later. NumericGeneric(TypeVariable, Box), + + AssociatedConstant(TraitImplId, String), } impl DefinitionKind { @@ -616,6 +617,7 @@ impl DefinitionKind { DefinitionKind::Global(_) => None, DefinitionKind::Local(id) => *id, DefinitionKind::NumericGeneric(_, _) => None, + DefinitionKind::AssociatedConstant(_, _) => None, } } } @@ -2280,37 +2282,26 @@ impl NodeInterner { pub(crate) fn set_associated_types_for_impl( &mut self, impl_id: TraitImplId, - mut associated_types: Vec, + associated_types: Vec, ) { // Wrap the named generics in type variables to be able to refer them as type variables - for associated_type in &mut associated_types { - let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { + for associated_type in &associated_types { + let Kind::Numeric(..) = associated_type.typ.kind() else { continue; }; - let mut wrapper = - self.next_type_variable_with_kind(Kind::Numeric(numeric_type.clone())); - let Type::TypeVariable(type_variable) = &mut wrapper else { - unreachable!( - "Expected `next_type_variable_with_kind` to create a `Type::TypeVariable`" - ); - }; - let type_variable = type_variable.clone(); - let type_variable_id = type_variable.id(); - - let named_generic_type = std::mem::replace(&mut associated_type.typ, Type::Error); - type_variable.bind(named_generic_type); - associated_type.typ = wrapper; - + let name = associated_type.name.to_string(); let definition_id = self.push_definition( associated_type.name.to_string(), false, false, - DefinitionKind::NumericGeneric(type_variable, numeric_type), + DefinitionKind::AssociatedConstant(impl_id, name.clone()), associated_type.name.location(), ); self.trait_impl_associated_constant_definition_ids - .insert(type_variable_id, definition_id); + .entry(impl_id) + .or_default() + .insert(name, definition_id); } self.trait_impl_associated_types.insert(impl_id, associated_types); @@ -2334,9 +2325,10 @@ impl NodeInterner { /// Returns the definition id for the associated constant of the given type variable. pub fn get_associated_constant_definition_id( &self, - type_variable_id: TypeVariableId, + impl_id: TraitImplId, + name: &str, ) -> DefinitionId { - self.trait_impl_associated_constant_definition_ids[&type_variable_id] + self.trait_impl_associated_constant_definition_ids[&impl_id][name] } /// Return a set of TypeBindings to bind types from the parent trait to those from the trait impl. diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs index 3bd5d26d60f..b5723e0e046 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs @@ -716,7 +716,9 @@ impl ItemPrinter<'_, '_, '_> { let use_import = true; self.show_reference_to_module_def_id(ModuleDefId::GlobalId(*global_id), use_import); } - DefinitionKind::Local(..) | DefinitionKind::NumericGeneric(..) => { + DefinitionKind::Local(..) + | DefinitionKind::NumericGeneric(..) + | DefinitionKind::AssociatedConstant(..) => { let name = self.interner.definition_name(ident.id); // The compiler uses '$' for some internal identifiers. From 1f33012a9c3064b842a78ef4ea51c709d4e8cc9a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 15:54:23 -0300 Subject: [PATCH 07/12] Make it work for the interpreter --- .../src/hir/comptime/interpreter.rs | 28 +++++++++++++++---- .../trait_associated_constant/src/main.nr | 5 ++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 1b497668a95..d2da3aba0cb 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -554,10 +554,12 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } pub(super) fn evaluate_ident(&mut self, ident: HirIdent, id: ExprId) -> IResult { - let definition = self.elaborator.interner.try_definition(ident.id).ok_or_else(|| { - let location = self.elaborator.interner.expr_location(&id); - InterpreterError::VariableNotInScope { location } - })?; + let location = self.elaborator.interner.expr_location(&id); + let definition = self + .elaborator + .interner + .try_definition(ident.id) + .ok_or_else(|| InterpreterError::VariableNotInScope { location })?; if let ImplKind::TraitMethod(method) = ident.impl_kind { let method_id = resolve_trait_method(self.elaborator.interner, method.method_id, id)?; @@ -636,8 +638,22 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { self.evaluate_integer(value.into(), id) } - DefinitionKind::AssociatedConstant(..) => { - todo!("Implement accessing an associated constant in the comptime interpreter") + DefinitionKind::AssociatedConstant(trait_impl_id, name) => { + let associated_types = + self.elaborator.interner.get_associated_types_for_impl(*trait_impl_id); + let associated_type = associated_types + .iter() + .find(|typ| typ.name.as_str() == name) + .expect("Expected to find associated type"); + let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { + unreachable!("Expected associated type to be numeric"); + }; + let value = associated_type + .typ + .evaluate_to_field_element(&associated_type.typ.kind(), location) + .expect("Expected to be able to evaluate associated type"); + + self.evaluate_integer(value.into(), id) } } } diff --git a/test_programs/execution_success/trait_associated_constant/src/main.nr b/test_programs/execution_success/trait_associated_constant/src/main.nr index 78ce876acb2..c42f08b5cce 100644 --- a/test_programs/execution_success/trait_associated_constant/src/main.nr +++ b/test_programs/execution_success/trait_associated_constant/src/main.nr @@ -17,4 +17,9 @@ impl Trait for Foo { fn main() { let x = Foo::<10, 20>::foo(); assert_eq(x, 30); + + comptime { + let x = Foo::<10, 20>::foo(); + assert_eq(x, 30); + } } From b0166efe97af3e18153fe6c3ee80d5f31274f231 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 16 May 2025 15:59:36 -0300 Subject: [PATCH 08/12] Undo unneeded change --- tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs index b5723e0e046..1a25206ebb5 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer/hir.rs @@ -684,24 +684,24 @@ impl ItemPrinter<'_, '_, '_> { fn show_hir_ident(&mut self, ident: HirIdent) { let definition = self.interner.definition(ident.id); - match &definition.kind { + match definition.kind { DefinitionKind::Function(func_id) => { - let func_meta = self.interner.function_meta(func_id); + let func_meta = self.interner.function_meta(&func_id); if func_meta.self_type.is_some() && func_meta.self_type == self.self_type { // No need to fully-qualify the function name if its self type is the current self type - let name = self.interner.function_name(func_id); + let name = self.interner.function_name(&func_id); self.push_str("Self::"); self.push_str(name); } else { let use_import = true; self.show_reference_to_module_def_id( - ModuleDefId::FunctionId(*func_id), + ModuleDefId::FunctionId(func_id), use_import, ); } } DefinitionKind::Global(global_id) => { - let global_info = self.interner.get_global(*global_id); + let global_info = self.interner.get_global(global_id); let typ = self.interner.definition_type(global_info.definition_id); // Special case: the global is an enum value @@ -714,7 +714,7 @@ impl ItemPrinter<'_, '_, '_> { } } let use_import = true; - self.show_reference_to_module_def_id(ModuleDefId::GlobalId(*global_id), use_import); + self.show_reference_to_module_def_id(ModuleDefId::GlobalId(global_id), use_import); } DefinitionKind::Local(..) | DefinitionKind::NumericGeneric(..) From 5a2d07b234ab1adad386848d9cc630689b95857d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 19 May 2025 10:35:34 -0300 Subject: [PATCH 09/12] Properly handle error during comptime interpreter --- .../noirc_frontend/src/hir/comptime/errors.rs | 17 +++++++++++++++++ .../src/hir/comptime/interpreter.rs | 13 +++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 3d3c964d52b..e62a0918f88 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -92,6 +92,11 @@ pub enum InterpreterError { err: Option>, location: Location, }, + NonIntegerAssociatedConstant { + typ: Type, + err: Option>, + location: Location, + }, NonNumericCasted { typ: Type, location: Location, @@ -289,6 +294,7 @@ impl InterpreterError { | InterpreterError::NonIntegerUsedAsIndex { location, .. } | InterpreterError::NonIntegerIntegerLiteral { location, .. } | InterpreterError::NonIntegerArrayLength { location, .. } + | InterpreterError::NonIntegerAssociatedConstant { location, .. } | InterpreterError::NonNumericCasted { location, .. } | InterpreterError::IndexOutOfBounds { location, .. } | InterpreterError::ExpectedStructToHaveField { location, .. } @@ -461,6 +467,17 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { }; CustomDiagnostic::simple_error(msg, secondary, *location) } + InterpreterError::NonIntegerAssociatedConstant { typ, err, location } => { + let msg = format!("Non-integer associated constant: `{typ}`"); + let secondary = if let Some(err) = err { + format!( + "Associated constants must be integers, but evaluating `{typ}` resulted in `{err}`" + ) + } else { + "Associated constants must be integers".to_string() + }; + CustomDiagnostic::simple_error(msg, secondary, *location) + } InterpreterError::NonNumericCasted { typ, location } => { let msg = "Only numeric types may be casted".into(); let secondary = format!("`{typ}` is non-numeric"); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index d2da3aba0cb..016a10654b6 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -648,12 +648,17 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { unreachable!("Expected associated type to be numeric"); }; - let value = associated_type + match associated_type .typ .evaluate_to_field_element(&associated_type.typ.kind(), location) - .expect("Expected to be able to evaluate associated type"); - - self.evaluate_integer(value.into(), id) + { + Ok(value) => self.evaluate_integer(value.into(), id), + Err(err) => Err(InterpreterError::NonIntegerArrayLength { + typ: associated_type.typ.clone(), + err: Some(Box::new(err)), + location, + }), + } } } } From 8232e19e498f5a44a83897afa5bfdbff00453774 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 19 May 2025 10:41:07 -0300 Subject: [PATCH 10/12] Properly handle error during monomorphization --- .../src/monomorphization/errors.rs | 13 +++++++++++- .../src/monomorphization/mod.rs | 20 ++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index 03a93d8fc85..287799192c8 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -51,6 +51,11 @@ pub enum MonomorphizationError { typ: Type, location: Location, }, + CannotComputeAssociatedConstant { + name: String, + err: TypeCheckError, + location: Location, + }, } impl MonomorphizationError { @@ -65,7 +70,8 @@ impl MonomorphizationError { | MonomorphizationError::CheckedCastFailed { location, .. } | MonomorphizationError::RecursiveType { location, .. } | MonomorphizationError::NoDefaultType { location, .. } - | MonomorphizationError::NoDefaultTypeInItem { location, .. } => *location, + | MonomorphizationError::NoDefaultTypeInItem { location, .. } + | MonomorphizationError::CannotComputeAssociatedConstant { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.location(), } } @@ -121,6 +127,11 @@ impl From for CustomDiagnostic { let secondary = "All types in Noir must have a known size at compile-time".into(); return CustomDiagnostic::simple_error(message, secondary, *location); } + MonomorphizationError::CannotComputeAssociatedConstant { name, err, .. } => { + format!( + "Could not determine the value of associated constant `{name}`, encountered error: `{err}`" + ) + } }; let location = error.location(); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 78e4df63ff5..cfa066eb078 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1088,13 +1088,23 @@ impl<'interner> Monomorphizer<'interner> { let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { unreachable!("Expected associated type to be numeric"); }; - let field = associated_type + match associated_type .typ .evaluate_to_field_element(&associated_type.typ.kind(), location) - .expect("Expected to be able to evaluate associated type"); - let typ = Self::convert_type(&numeric_type, location)?; - let value = SignedField::positive(field); - ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) + { + Ok(value) => { + let typ = Self::convert_type(&numeric_type, location)?; + let value = SignedField::positive(value); + ast::Expression::Literal(ast::Literal::Integer(value, typ, location)) + } + Err(err) => { + return Err(MonomorphizationError::CannotComputeAssociatedConstant { + name: name.clone(), + err, + location, + }); + } + } } }; From ea7ed64af71a3dc8156d6035d7eefdea67070314 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 22 May 2025 10:01:00 -0300 Subject: [PATCH 11/12] Snapshot --- .../trait_associated_constant/execute__tests__stdout.snap | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__stdout.snap diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__stdout.snap new file mode 100644 index 00000000000..e86e3de90e1 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/trait_associated_constant/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- + From 48ac45ff20473355e861640af9059ee9f54edfbd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 22 May 2025 18:16:36 -0300 Subject: [PATCH 12/12] Only fetch location when needed --- .../noirc_frontend/src/hir/comptime/interpreter.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 6ad72aa57be..a6dd484ba9d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -573,12 +573,10 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } pub(super) fn evaluate_ident(&mut self, ident: HirIdent, id: ExprId) -> IResult { - let location = self.elaborator.interner.expr_location(&id); - let definition = self - .elaborator - .interner - .try_definition(ident.id) - .ok_or_else(|| InterpreterError::VariableNotInScope { location })?; + let definition = self.elaborator.interner.try_definition(ident.id).ok_or_else(|| { + let location = self.elaborator.interner.expr_location(&id); + InterpreterError::VariableNotInScope { location } + })?; if let ImplKind::TraitMethod(method) = ident.impl_kind { let method_id = resolve_trait_method(self.elaborator.interner, method.method_id, id)?; @@ -667,6 +665,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let Kind::Numeric(numeric_type) = associated_type.typ.kind() else { unreachable!("Expected associated type to be numeric"); }; + let location = self.elaborator.interner.expr_location(&id); match associated_type .typ .evaluate_to_field_element(&associated_type.typ.kind(), location)