From 5c3427a91f749a3955c9e5164ea71673139b5710 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 23 Jul 2024 08:04:35 -0300 Subject: [PATCH 1/5] fix: error on trait impl generics count mismatch --- compiler/noirc_frontend/src/elaborator/mod.rs | 52 +++++++++++++------ compiler/noirc_frontend/src/tests.rs | 26 ++++++++++ 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 3aeac115ad5..5ce4dcabe6e 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1418,22 +1418,42 @@ impl<'context> Elaborator<'context> { trait_impl.resolved_generics = self.generics.clone(); // Fetch trait constraints here - let trait_generics = if let Some(trait_id) = trait_impl.trait_id { - let trait_def = self.interner.get_trait(trait_id); - let resolved_generics = trait_def.generics.clone(); - assert_eq!(resolved_generics.len(), trait_impl.trait_generics.len()); - trait_impl - .trait_generics - .iter() - .enumerate() - .map(|(i, generic)| { - self.resolve_type_inner(generic.clone(), &resolved_generics[i].kind) - }) - .collect() - } else { - // We still resolve as to continue type checking - vecmap(&trait_impl.trait_generics, |generic| self.resolve_type(generic.clone())) - }; + let trait_generics = trait_impl + .trait_id + .and_then(|trait_id| { + let trait_def = self.interner.get_trait(trait_id); + let resolved_generics = trait_def.generics.clone(); + if resolved_generics.len() == trait_impl.trait_generics.len() { + Some( + trait_impl + .trait_generics + .iter() + .enumerate() + .map(|(i, generic)| { + self.resolve_type_inner( + generic.clone(), + &resolved_generics[i].kind, + ) + }) + .collect(), + ) + } else { + self.push_err(CompilationError::TypeError( + TypeCheckError::GenericCountMismatch { + item: trait_def.name.to_string(), + expected: resolved_generics.len(), + found: trait_impl.trait_generics.len(), + span: trait_impl.trait_path.span(), + }, + )); + + None + } + }) + .unwrap_or_else(|| { + // We still resolve as to continue type checking + vecmap(&trait_impl.trait_generics, |generic| self.resolve_type(generic.clone())) + }); trait_impl.resolved_trait_generics = trait_generics; diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index b4f17489ff7..3767dad103c 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -2459,3 +2459,29 @@ fn no_super() { assert_eq!(span.start(), 4); assert_eq!(span.end(), 9); } + +#[test] +fn trait_impl_generics_count_mismatch() { + let src = r#" + trait Foo {} + + impl Foo<()> for Field {} + + fn main() {}"#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::GenericCountMismatch { + item, + expected, + found, + .. + }) = &errors[0].0 + else { + panic!("Expected a generic count mismatch error, got {:?}", errors[0].0); + }; + + assert_eq!(item, "Foo"); + assert_eq!(*expected, 0); + assert_eq!(*found, 1); +} From 146d351823ab80f4a82ae6913ef90b11fedf52d6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 23 Jul 2024 11:16:13 -0300 Subject: [PATCH 2/5] Extract `resolve_trait_impl_generics` --- compiler/noirc_frontend/src/elaborator/mod.rs | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 5ce4dcabe6e..47340a57a70 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1420,36 +1420,7 @@ impl<'context> Elaborator<'context> { // Fetch trait constraints here let trait_generics = trait_impl .trait_id - .and_then(|trait_id| { - let trait_def = self.interner.get_trait(trait_id); - let resolved_generics = trait_def.generics.clone(); - if resolved_generics.len() == trait_impl.trait_generics.len() { - Some( - trait_impl - .trait_generics - .iter() - .enumerate() - .map(|(i, generic)| { - self.resolve_type_inner( - generic.clone(), - &resolved_generics[i].kind, - ) - }) - .collect(), - ) - } else { - self.push_err(CompilationError::TypeError( - TypeCheckError::GenericCountMismatch { - item: trait_def.name.to_string(), - expected: resolved_generics.len(), - found: trait_impl.trait_generics.len(), - span: trait_impl.trait_path.span(), - }, - )); - - None - } - }) + .and_then(|trait_id| self.resolve_trait_impl_generics(trait_impl, trait_id)) .unwrap_or_else(|| { // We still resolve as to continue type checking vecmap(&trait_impl.trait_generics, |generic| self.resolve_type(generic.clone())) @@ -1481,6 +1452,36 @@ impl<'context> Elaborator<'context> { } } + fn resolve_trait_impl_generics( + &mut self, + trait_impl: &UnresolvedTraitImpl, + trait_id: TraitId, + ) -> Option> { + let trait_def = self.interner.get_trait(trait_id); + let resolved_generics = trait_def.generics.clone(); + if resolved_generics.len() != trait_impl.trait_generics.len() { + self.push_err(CompilationError::TypeError(TypeCheckError::GenericCountMismatch { + item: trait_def.name.to_string(), + expected: resolved_generics.len(), + found: trait_impl.trait_generics.len(), + span: trait_impl.trait_path.span(), + })); + + return None; + } + + Some( + trait_impl + .trait_generics + .iter() + .zip(resolved_generics.iter()) + .map(|(generic, resolved_generic)| { + self.resolve_type_inner(generic.clone(), &resolved_generic.kind) + }) + .collect(), + ) + } + fn define_function_metas_for_functions(&mut self, function_set: &mut UnresolvedFunctions) { self.file = function_set.file_id; From afebc44537998fba1d2e1d14337159339d7e8d97 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 23 Jul 2024 11:43:33 -0300 Subject: [PATCH 3/5] Move resolve_trait_impl_generics to traits file --- compiler/noirc_frontend/src/elaborator/mod.rs | 30 ---------------- .../noirc_frontend/src/elaborator/traits.rs | 34 ++++++++++++++++++- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 47340a57a70..9845166afae 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1452,36 +1452,6 @@ impl<'context> Elaborator<'context> { } } - fn resolve_trait_impl_generics( - &mut self, - trait_impl: &UnresolvedTraitImpl, - trait_id: TraitId, - ) -> Option> { - let trait_def = self.interner.get_trait(trait_id); - let resolved_generics = trait_def.generics.clone(); - if resolved_generics.len() != trait_impl.trait_generics.len() { - self.push_err(CompilationError::TypeError(TypeCheckError::GenericCountMismatch { - item: trait_def.name.to_string(), - expected: resolved_generics.len(), - found: trait_impl.trait_generics.len(), - span: trait_impl.trait_path.span(), - })); - - return None; - } - - Some( - trait_impl - .trait_generics - .iter() - .zip(resolved_generics.iter()) - .map(|(generic, resolved_generic)| { - self.resolve_type_inner(generic.clone(), &resolved_generic.kind) - }) - .collect(), - ) - } - fn define_function_metas_for_functions(&mut self, function_set: &mut UnresolvedFunctions) { self.file = function_set.file_id; diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 8520885bcdc..a799149c672 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -8,7 +8,9 @@ use crate::{ FunctionKind, TraitItem, UnresolvedGeneric, UnresolvedGenerics, UnresolvedTraitConstraint, }, hir::{ - def_collector::dc_crate::{CollectedItems, UnresolvedTrait}, + def_collector::dc_crate::{ + CollectedItems, CompilationError, UnresolvedTrait, UnresolvedTraitImpl, + }, type_check::TypeCheckError, }, hir_def::{ @@ -215,6 +217,36 @@ impl<'context> Elaborator<'context> { // Don't check the scope tree for unused variables, they can't be used in a declaration anyway. self.generics.truncate(old_generic_count); } + + pub fn resolve_trait_impl_generics( + &mut self, + trait_impl: &UnresolvedTraitImpl, + trait_id: TraitId, + ) -> Option> { + let trait_def = self.interner.get_trait(trait_id); + let resolved_generics = trait_def.generics.clone(); + if resolved_generics.len() != trait_impl.trait_generics.len() { + self.push_err(CompilationError::TypeError(TypeCheckError::GenericCountMismatch { + item: trait_def.name.to_string(), + expected: resolved_generics.len(), + found: trait_impl.trait_generics.len(), + span: trait_impl.trait_path.span(), + })); + + return None; + } + + Some( + trait_impl + .trait_generics + .iter() + .zip(resolved_generics.iter()) + .map(|(generic, resolved_generic)| { + self.resolve_type_inner(generic.clone(), &resolved_generic.kind) + }) + .collect(), + ) + } } /// Checks that the type of a function in a trait impl matches the type From ebb85584e5e9271e0c5ea4bb4a16d2a7bd9ec5a3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 23 Jul 2024 11:46:53 -0300 Subject: [PATCH 4/5] Introduce some intermediate vars --- compiler/noirc_frontend/src/elaborator/traits.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index a799149c672..87abf407b4d 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -236,11 +236,11 @@ impl<'context> Elaborator<'context> { return None; } + let trait_generics = trait_impl.trait_generics.iter(); + let resolved_generics = resolved_generics.iter(); Some( - trait_impl - .trait_generics - .iter() - .zip(resolved_generics.iter()) + trait_generics + .zip(resolved_generics) .map(|(generic, resolved_generic)| { self.resolve_type_inner(generic.clone(), &resolved_generic.kind) }) From df3e906b6fa94785ac6bde6044204630e4c4bea1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 23 Jul 2024 12:23:25 -0300 Subject: [PATCH 5/5] Update compiler/noirc_frontend/src/elaborator/traits.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/elaborator/traits.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/traits.rs b/compiler/noirc_frontend/src/elaborator/traits.rs index 87abf407b4d..a00e770218e 100644 --- a/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/compiler/noirc_frontend/src/elaborator/traits.rs @@ -236,16 +236,11 @@ impl<'context> Elaborator<'context> { return None; } - let trait_generics = trait_impl.trait_generics.iter(); - let resolved_generics = resolved_generics.iter(); - Some( - trait_generics - .zip(resolved_generics) - .map(|(generic, resolved_generic)| { - self.resolve_type_inner(generic.clone(), &resolved_generic.kind) - }) - .collect(), - ) + let generics = trait_impl.trait_generics.iter().zip(resolved_generics.iter()); + let mapped = generics.map(|(generic, resolved_generic)| { + self.resolve_type_inner(generic.clone(), &resolved_generic.kind) + }); + Some(mapped.collect()) } }