diff --git a/.noir-sync-commit b/.noir-sync-commit index 8f4fb5de09f..1981fed1aa8 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -56c931a8e59e9e67e8bd5aae4a114e85fe40ff98 +5300ec321fb99ddaad32e83f751aed28e175736f diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml index f357f501e8d..8e91ae17e93 100644 --- a/noir/noir-repo/.github/workflows/reports.yml +++ b/noir/noir-repo/.github/workflows/reports.yml @@ -274,7 +274,7 @@ jobs: run: | ./execution_report.sh 0 1 mv execution_report.json ../execution_report.json - + - name: Upload compilation report uses: actions/upload-artifact@v4 with: @@ -282,7 +282,7 @@ jobs: path: compilation_report.json retention-days: 3 overwrite: true - + - name: Upload execution report uses: actions/upload-artifact@v4 with: @@ -360,7 +360,7 @@ jobs: chmod +x parse_time.sh cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh ./compilation_report.sh 1 1 - + - name: Generate execution report without averages working-directory: ./test-repo/${{ matrix.project.path }} if: ${{ !matrix.project.is_library && !matrix.project.take_average }} @@ -395,7 +395,7 @@ jobs: PACKAGE_NAME=$(basename $PACKAGE_NAME) mv ./test-repo/${{ matrix.project.path }}/execution_report.json ./execution_report_$PACKAGE_NAME.json echo "execution_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT - + - name: Upload compilation report uses: actions/upload-artifact@v4 with: @@ -413,10 +413,10 @@ jobs: overwrite: true upload_compilation_report: - name: Upload compilation report + name: Upload compilation report needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report] # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails - if: always() + if: always() runs-on: ubuntu-latest permissions: pull-requests: write @@ -550,7 +550,7 @@ jobs: overwrite: true upload_compilation_memory_report: - name: Upload compilation memory report + name: Upload compilation memory report needs: [generate_memory_report, external_repo_memory_report] # We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails if: always() @@ -594,10 +594,10 @@ jobs: message: ${{ steps.compilation_mem_report.outputs.markdown }} upload_execution_memory_report: - name: Upload execution memory report + name: Upload execution memory report needs: [generate_memory_report, external_repo_memory_report] # We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails - if: always() + if: always() runs-on: ubuntu-latest permissions: pull-requests: write @@ -640,10 +640,10 @@ jobs: message: ${{ steps.execution_mem_report.outputs.markdown }} upload_execution_report: - name: Upload execution report + name: Upload execution report needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report] # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails - if: always() + if: always() runs-on: ubuntu-latest permissions: pull-requests: write diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml index 0915d3194ac..7f8370620fd 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/noir/noir-repo/.github/workflows/test-js-packages.yml @@ -605,6 +605,10 @@ jobs: fi env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true + + - name: Compare test results + working-directory: ./noir-repo + run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl - name: Compare test results working-directory: ./noir-repo diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 9b3df4631b4..72e46d36c2c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -1361,7 +1361,7 @@ impl<'context> Elaborator<'context> { span: Span, has_self_arg: bool, ) -> Option { - // First search in the struct methods + // First search in the type methods. If there is one, that's the one. if let Some(method_id) = self.interner.lookup_direct_method(object_type, method_name, has_self_arg) { @@ -1372,43 +1372,55 @@ impl<'context> Elaborator<'context> { let trait_methods = self.interner.lookup_trait_methods(object_type, method_name, has_self_arg); - if trait_methods.is_empty() { - // If we couldn't find any trait methods, search in - // impls for all types `T`, e.g. `impl Foo for T` - if let Some(func_id) = - self.interner.lookup_generic_method(object_type, method_name, has_self_arg) - { - return Some(HirMethodReference::FuncId(func_id)); - } + // If there's at least one matching trait method we need to see if only one is in scope. + if !trait_methods.is_empty() { + return self.return_trait_method_in_scope(&trait_methods, method_name, span); + } - if let Type::Struct(struct_type, _) = object_type { - let has_field_with_function_type = - struct_type.borrow().get_fields_as_written().into_iter().any(|field| { - field.name.0.contents == method_name && field.typ.is_function() - }); - if has_field_with_function_type { - self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType { - method_name: method_name.to_string(), - object_type: object_type.clone(), - span, - }); - } else { - self.push_err(TypeCheckError::UnresolvedMethodCall { - method_name: method_name.to_string(), - object_type: object_type.clone(), - span, - }); - } - return None; + // If we couldn't find any trait methods, search in + // impls for all types `T`, e.g. `impl Foo for T` + let generic_methods = + self.interner.lookup_generic_methods(object_type, method_name, has_self_arg); + if !generic_methods.is_empty() { + return self.return_trait_method_in_scope(&generic_methods, method_name, span); + } + + if let Type::Struct(struct_type, _) = object_type { + let has_field_with_function_type = struct_type + .borrow() + .get_fields_as_written() + .into_iter() + .any(|field| field.name.0.contents == method_name && field.typ.is_function()); + if has_field_with_function_type { + self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType { + method_name: method_name.to_string(), + object_type: object_type.clone(), + span, + }); } else { - // It could be that this type is a composite type that is bound to a trait, - // for example `x: (T, U) ... where (T, U): SomeTrait` - // (so this case is a generalization of the NamedGeneric case) - return self.lookup_method_in_trait_constraints(object_type, method_name, span); + self.push_err(TypeCheckError::UnresolvedMethodCall { + method_name: method_name.to_string(), + object_type: object_type.clone(), + span, + }); } + None + } else { + // It could be that this type is a composite type that is bound to a trait, + // for example `x: (T, U) ... where (T, U): SomeTrait` + // (so this case is a generalization of the NamedGeneric case) + self.lookup_method_in_trait_constraints(object_type, method_name, span) } + } - // We found some trait methods... but is only one of them currently in scope? + /// Given a list of functions and the trait they belong to, returns the one function + /// that is in scope. + fn return_trait_method_in_scope( + &mut self, + trait_methods: &[(FuncId, TraitId)], + method_name: &str, + span: Span, + ) -> Option { let module_id = self.module_id(); let module_data = self.get_module(module_id); @@ -1490,7 +1502,7 @@ impl<'context> Elaborator<'context> { fn trait_hir_method_reference( &self, trait_id: TraitId, - trait_methods: Vec<(FuncId, TraitId)>, + trait_methods: &[(FuncId, TraitId)], method_name: &str, span: Span, ) -> HirMethodReference { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index c1a405535f2..a2b1d7fc8f0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -1746,7 +1746,7 @@ impl NodeInterner { Ok(()) } - /// Looks up a method that's directly defined in the given struct. + /// Looks up a method that's directly defined in the given type. pub fn lookup_direct_method( &self, typ: &Type, @@ -1761,7 +1761,7 @@ impl NodeInterner { .and_then(|methods| methods.find_direct_method(typ, has_self_arg, self)) } - /// Looks up a methods that apply to the given struct but are defined in traits. + /// Looks up a methods that apply to the given type but are defined in traits. pub fn lookup_trait_methods( &self, typ: &Type, @@ -1780,43 +1780,18 @@ impl NodeInterner { } } - /// Select the 1 matching method with an object type matching `typ` - fn find_matching_method( - &self, - typ: &Type, - methods: Option<&Methods>, - method_name: &str, - has_self_arg: bool, - ) -> Option { - if let Some(method) = methods.and_then(|m| m.find_matching_method(typ, has_self_arg, self)) - { - Some(method) - } else { - self.lookup_generic_method(typ, method_name, has_self_arg) - } - } - - /// Looks up a method at impls for all types `T`, e.g. `impl Foo for T` - pub fn lookup_generic_method( + /// Looks up methods at impls for all types `T`, e.g. `impl Foo for T` + pub fn lookup_generic_methods( &self, typ: &Type, method_name: &str, has_self_arg: bool, - ) -> Option { - let global_methods = self.methods.get(&TypeMethodKey::Generic)?.get(method_name)?; - global_methods.find_matching_method(typ, has_self_arg, self) - } - - /// Looks up a given method name on the given primitive type. - pub fn lookup_primitive_method( - &self, - typ: &Type, - method_name: &str, - has_self_arg: bool, - ) -> Option { - let key = get_type_method_key(typ)?; - let methods = self.methods.get(&key)?.get(method_name)?; - self.find_matching_method(typ, Some(methods), method_name, has_self_arg) + ) -> Vec<(FuncId, TraitId)> { + self.methods + .get(&TypeMethodKey::Generic) + .and_then(|h| h.get(method_name)) + .map(|methods| methods.find_trait_methods(typ, has_self_arg, self)) + .unwrap_or_default() } /// Returns what the next trait impl id is expected to be. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs index 449b6274055..04eb9cda7ab 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs @@ -879,6 +879,43 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() { assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); } +#[test] +fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_method() { + let src = r#" + fn main() { + let bar = Bar { x: 42 }; + let _ = bar.foo(); + } + + pub struct Bar { + x: i32, + } + + mod private_mod { + pub trait Foo { + fn foo(self) -> i32; + } + + impl Foo for super::Bar { + fn foo(self) -> i32 { + self.x + } + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::TraitMethodNotInScope { ident, trait_name }, + )) = &errors[0].0 + else { + panic!("Expected a 'trait method not in scope' error"); + }; + assert_eq!(ident.to_string(), "foo"); + assert_eq!(trait_name, "private_mod::Foo"); +} + #[test] fn calls_trait_method_if_it_is_in_scope() { let src = r#" @@ -1166,3 +1203,36 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on assert_eq!(ident.to_string(), "foo"); assert_eq!(trait_name, "private_mod::Foo"); } + +#[test] +fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_one_trait_method() { + let src = r#" + fn main() { + let x: i32 = 1; + let _ = x.foo(); + } + + mod private_mod { + pub trait Foo { + fn foo(self) -> i32; + } + + impl Foo for T { + fn foo(self) -> i32 { + 42 + } + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::TraitMethodNotInScope { ident, trait_name }, + )) = &errors[0].0 + else { + panic!("Expected a 'trait method not in scope' error"); + }; + assert_eq!(ident.to_string(), "foo"); + assert_eq!(trait_name, "private_mod::Foo"); +} diff --git a/noir/noir-repo/noir_stdlib/src/meta/expr.nr b/noir/noir-repo/noir_stdlib/src/meta/expr.nr index a20156cf154..7538b26dc44 100644 --- a/noir/noir-repo/noir_stdlib/src/meta/expr.nr +++ b/noir/noir-repo/noir_stdlib/src/meta/expr.nr @@ -669,7 +669,7 @@ comptime fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr { comptime fn new_unsafe(exprs: [Expr]) -> Expr { let exprs = join_expressions(exprs, quote { ; }); - quote { + quote { /// Safety: generated by macro unsafe { $exprs } } diff --git a/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr index 8112371379c..25910685e87 100644 --- a/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/comptime_expr/src/main.nr @@ -553,9 +553,9 @@ mod tests { #[test] fn test_expr_as_unsafe() { comptime { - let expr = quote { + let expr = quote { /// Safety: test - unsafe { 1; 4; 23 } + unsafe { 1; 4; 23 } } .as_expr() .unwrap(); @@ -567,9 +567,9 @@ mod tests { #[test] fn test_expr_modify_for_unsafe() { comptime { - let expr = quote { + let expr = quote { /// Safety: test - unsafe { 1; 4; 23 } + unsafe { 1; 4; 23 } } .as_expr() .unwrap(); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs index be6595260fa..ef04276a605 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs @@ -1911,7 +1911,7 @@ global y = 1; #[test] fn format_unsafe_one_expression() { - let src = "global x = unsafe { + let src = "global x = unsafe { 1 } ;"; let expected = "global x = unsafe { 1 };\n"; assert_format(src, expected); @@ -1919,7 +1919,7 @@ global y = 1; #[test] fn format_unsafe_two_expressions() { - let src = "global x = unsafe { + let src = "global x = unsafe { 1; 2 } ;"; let expected = "global x = unsafe { 1; @@ -1932,7 +1932,7 @@ global y = 1; #[test] fn format_unsafe_with_doc_comment() { let src = "fn foo() { - /// Comment + /// Comment unsafe { 1 } }"; let expected = "fn foo() { /// Comment diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs index 2ae6d85780d..5d30b2aeca3 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs @@ -379,7 +379,7 @@ mod tests { #[test] fn format_let_statement_with_unsafe() { - let src = " fn foo() { + let src = " fn foo() { /// Safety: some doc let x = unsafe { 1 } ; } "; let expected = "fn foo() { @@ -505,7 +505,7 @@ mod tests { #[test] fn format_unsafe_statement() { - let src = " fn foo() { unsafe { + let src = " fn foo() { unsafe { 1 } } "; let expected = "fn foo() { unsafe {