From 0ec9bc6c26bf00e0d6e2d223545ea9fbf51bcfe4 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 01:04:07 +0000 Subject: [PATCH 1/8] fix: allow providing default implementations of unconstrained trait methods --- aztec_macros/src/transforms/storage.rs | 1 + compiler/noirc_frontend/src/ast/expression.rs | 3 +- .../noirc_frontend/src/elaborator/traits.rs | 35 ++++++++----------- .../src/hir/def_collector/dc_mod.rs | 1 + .../trait_unconstrained_method/Nargo.toml | 6 ++++ .../trait_unconstrained_method/src/main.nr | 18 ++++++++++ .../compile_success_empty/traits/Prover.toml | 2 -- 7 files changed, 42 insertions(+), 24 deletions(-) create mode 100644 test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml create mode 100644 test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr delete mode 100644 test_programs/compile_success_empty/traits/Prover.toml diff --git a/aztec_macros/src/transforms/storage.rs b/aztec_macros/src/transforms/storage.rs index e8ee1675546..3fd845f8202 100644 --- a/aztec_macros/src/transforms/storage.rs +++ b/aztec_macros/src/transforms/storage.rs @@ -238,6 +238,7 @@ pub fn generate_storage_implementation( let init = NoirFunction::normal(FunctionDefinition::normal( &ident("init"), + false, &vec![], &[(ident("context"), generic_context_type.clone())], &BlockExpression { statements: vec![storage_constructor_statement] }, diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index c581ea9d62a..7b0a6d028de 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -802,6 +802,7 @@ impl Display for TypePath { impl FunctionDefinition { pub fn normal( name: &Ident, + is_unconstrained: bool, generics: &UnresolvedGenerics, parameters: &[(Ident, UnresolvedType)], body: &BlockExpression, @@ -821,7 +822,7 @@ impl FunctionDefinition { FunctionDefinition { name: name.clone(), attributes: Attributes::empty(), - is_unconstrained: false, + is_unconstrained, is_comptime: false, visibility: ItemVisibility::Private, generics: generics.clone(), diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 0faaf409e6c..d7c8769620d 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -11,10 +11,9 @@ use crate::{ hir_def::{function::Parameters, traits::TraitFunction}, macros_api::{ BlockExpression, FunctionDefinition, FunctionReturnType, Ident, ItemVisibility, - NodeInterner, NoirFunction, Param, Pattern, UnresolvedType, Visibility, + NodeInterner, NoirFunction, UnresolvedType, }, node_interner::{FuncId, ReferenceId, TraitId}, - token::Attributes, Kind, ResolvedGeneric, Type, TypeBindings, TypeVariableKind, }; @@ -103,6 +102,7 @@ impl<'context> Elaborator<'context> { this.resolve_trait_function( trait_id, name, + *is_unconstrained, generics, parameters, return_type, @@ -164,6 +164,7 @@ impl<'context> Elaborator<'context> { &mut self, trait_id: TraitId, name: &Ident, + is_unconstrained: bool, generics: &UnresolvedGenerics, parameters: &[(Ident, UnresolvedType)], return_type: &FunctionReturnType, @@ -175,25 +176,17 @@ impl<'context> Elaborator<'context> { self.scopes.start_function(); let kind = FunctionKind::Normal; - let def = FunctionDefinition { - name: name.clone(), - attributes: Attributes::empty(), - is_unconstrained: false, - is_comptime: false, - visibility: ItemVisibility::Public, // Trait functions are always public - generics: generics.clone(), - parameters: vecmap(parameters, |(name, typ)| Param { - visibility: Visibility::Private, - pattern: Pattern::Identifier(name.clone()), - typ: typ.clone(), - span: name.span(), - }), - body: BlockExpression { statements: Vec::new() }, - span: name.span(), - where_clause: where_clause.to_vec(), - return_type: return_type.clone(), - return_visibility: Visibility::Private, - }; + let mut def = FunctionDefinition::normal( + name, + is_unconstrained, + generics, + parameters, + &BlockExpression { statements: Vec::new() }, + where_clause, + return_type, + ); + // Trait functions are always public + def.visibility = ItemVisibility::Public; let mut function = NoirFunction { kind, def }; self.define_function_meta(&mut function, func_id, Some(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 508765f943c..a6ee92be685 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -474,6 +474,7 @@ impl<'a> ModCollector<'a> { let impl_method = NoirFunction::normal(FunctionDefinition::normal( name, + *is_unconstrained, generics, parameters, body, diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml b/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml new file mode 100644 index 00000000000..3b2d220d821 --- /dev/null +++ b/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "trait_unconstrained_method" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr new file mode 100644 index 00000000000..6ae1c736acb --- /dev/null +++ b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr @@ -0,0 +1,18 @@ +trait Foo { + unconstrained fn identity(self) -> Self { + self + } + + unconstrained fn foo(self) -> u64; +} + +impl Foo for Field { + unconstrained fn foo(self) -> u64 { + self as u64 + } +} + +fn main() { + assert_eq(2.foo() as Field, 2.identity()); + +} diff --git a/test_programs/compile_success_empty/traits/Prover.toml b/test_programs/compile_success_empty/traits/Prover.toml deleted file mode 100644 index 71805e71e8e..00000000000 --- a/test_programs/compile_success_empty/traits/Prover.toml +++ /dev/null @@ -1,2 +0,0 @@ -x = "5" -y = "1" \ No newline at end of file From 146169eff5ab247a8ae674e1a66612f41ddbd3a1 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 24 Sep 2024 02:25:37 +0100 Subject: [PATCH 2/8] Update main.nr --- .../trait_unconstrained_method/src/main.nr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr index 6ae1c736acb..ee0a09d6606 100644 --- a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr +++ b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr @@ -13,6 +13,5 @@ impl Foo for Field { } fn main() { - assert_eq(2.foo() as Field, 2.identity()); - + assert_eq(2.foo() as Field, 2.identity()); } From 9b1e6743201c5aaeffd18a2a44d2fab3b14652ec Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 09:19:43 +0000 Subject: [PATCH 3/8] fmt --- .../trait_unconstrained_method/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr index ee0a09d6606..2c2d2ce2125 100644 --- a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr +++ b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr @@ -13,5 +13,5 @@ impl Foo for Field { } fn main() { - assert_eq(2.foo() as Field, 2.identity()); + assert_eq(2.foo() as Field, 2.identity()); } From ed7a1dda28167d0250ec5af78543f358a762a9fc Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 09:59:19 +0000 Subject: [PATCH 4/8] chore: move test into `noirc_frontend` --- compiler/noirc_frontend/src/tests.rs | 32 +++++++++++++++++++ .../trait_unconstrained_method/Nargo.toml | 6 ---- .../trait_unconstrained_method/src/main.nr | 17 ---------- 3 files changed, 32 insertions(+), 23 deletions(-) delete mode 100644 test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml delete mode 100644 test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index dc54fd624e4..5c42549e5a7 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3748,3 +3748,35 @@ fn macro_result_type_mismatch() { CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) )); } + + +#[test] +fn trait_unconstrained_methods_typechecked_correctly() { + // This test checks that we properly track whether a method has been declared as unconstrained on the trait definition + // and preserves that through typechecking. + let src = r#" + trait Foo { + unconstrained fn identity(self) -> Self { + self + } + + unconstrained fn foo(self) -> u64; + } + + impl Foo for Field { + unconstrained fn foo(self) -> u64 { + self as u64 + } + } + + fn main() { + assert_eq(2.foo() as Field, 2.identity()); + } + "#; + + let errors = get_program_errors(src); + println!("{errors:?}"); + assert_eq!(errors.len(), 0); +} + + diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml b/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml deleted file mode 100644 index 3b2d220d821..00000000000 --- a/test_programs/compile_success_empty/trait_unconstrained_method/Nargo.toml +++ /dev/null @@ -1,6 +0,0 @@ -[package] -name = "trait_unconstrained_method" -type = "bin" -authors = [""] - -[dependencies] diff --git a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr b/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr deleted file mode 100644 index 2c2d2ce2125..00000000000 --- a/test_programs/compile_success_empty/trait_unconstrained_method/src/main.nr +++ /dev/null @@ -1,17 +0,0 @@ -trait Foo { - unconstrained fn identity(self) -> Self { - self - } - - unconstrained fn foo(self) -> u64; -} - -impl Foo for Field { - unconstrained fn foo(self) -> u64 { - self as u64 - } -} - -fn main() { - assert_eq(2.foo() as Field, 2.identity()); -} From e298532ad35fa85ecb631f8bd0a822e4ae19a2e8 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 10:03:07 +0000 Subject: [PATCH 5/8] . --- compiler/noirc_frontend/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 5c42549e5a7..dc1e21f012c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3769,7 +3769,7 @@ fn trait_unconstrained_methods_typechecked_correctly() { } } - fn main() { + unconstrained fn main() { assert_eq(2.foo() as Field, 2.identity()); } "#; From bb16922dc919f34246f0fb58d33af98dd62e7c87 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 10:05:07 +0000 Subject: [PATCH 6/8] . --- compiler/noirc_frontend/src/tests.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index dc1e21f012c..4f71d53cbce 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3749,7 +3749,6 @@ fn macro_result_type_mismatch() { )); } - #[test] fn trait_unconstrained_methods_typechecked_correctly() { // This test checks that we properly track whether a method has been declared as unconstrained on the trait definition @@ -3778,5 +3777,3 @@ fn trait_unconstrained_methods_typechecked_correctly() { println!("{errors:?}"); assert_eq!(errors.len(), 0); } - - From 5c27fdbecc5462950860d6cde4a0fe0f7a5df354 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 13:45:27 +0000 Subject: [PATCH 7/8] . --- compiler/noirc_frontend/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1d38428884b..0528fce306b 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3066,4 +3066,4 @@ fn trait_unconstrained_methods_typechecked_correctly() { let errors = get_program_errors(src); println!("{errors:?}"); assert_eq!(errors.len(), 0); -} \ No newline at end of file +} From 7d4bd4d4bb56853370f1419fa8caeb764a18ec2b Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 24 Sep 2024 14:22:31 +0000 Subject: [PATCH 8/8] . --- compiler/noirc_frontend/src/tests.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 0528fce306b..91e4115ff69 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3039,6 +3039,27 @@ fn use_numeric_generic_in_trait_method() { assert_eq!(errors.len(), 0); } +#[test] +fn errors_once_on_unused_import_that_is_not_accessible() { + // Tests that we don't get an "unused import" here given that the import is not accessible + let src = r#" + mod moo { + struct Foo {} + } + use moo::Foo; + fn main() {} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( + PathResolutionError::Private { .. } + )) + )); +} + #[test] fn trait_unconstrained_methods_typechecked_correctly() { // This test checks that we properly track whether a method has been declared as unconstrained on the trait definition