From 8cf81b659bed9522aede29c1ebb4a4ed2bfa1205 Mon Sep 17 00:00:00 2001 From: jfecher Date: Fri, 27 Oct 2023 08:56:13 -0500 Subject: [PATCH] feat: Add check for overlapping generic traits (#3307) --- .../src/hir/def_collector/dc_crate.rs | 55 ++-- .../src/hir/def_collector/errors.rs | 29 +- .../noirc_frontend/src/hir/type_check/expr.rs | 10 +- .../noirc_frontend/src/hir/type_check/mod.rs | 8 +- compiler/noirc_frontend/src/hir_def/traits.rs | 2 + .../src/monomorphization/mod.rs | 7 +- compiler/noirc_frontend/src/node_interner.rs | 251 ++++++++++++------ compiler/noirc_frontend/src/tests.rs | 41 ++- .../overlapping_generic_impls/Nargo.toml | 7 + .../overlapping_generic_impls/src/main.nr | 8 + .../trait_override_implementation/src/main.nr | 48 ++-- .../trait_impl_base_type/src/main.nr | 25 +- 12 files changed, 283 insertions(+), 208 deletions(-) create mode 100644 tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/Nargo.toml create mode 100644 tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 86b22992555..e59ab3e59f9 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -13,9 +13,7 @@ use crate::hir::resolution::{ use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker}; use crate::hir::Context; use crate::hir_def::traits::{Trait, TraitConstant, TraitFunction, TraitImpl, TraitType}; -use crate::node_interner::{ - FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId, -}; +use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId}; use crate::parser::{ParserError, SortedModule}; use crate::{ @@ -331,7 +329,6 @@ impl DefCollector { def_collector.collected_impls, &mut errors, ); - // resolve_trait_impls can fill different type of errors, therefore we pass errors by mut ref let file_trait_impls_ids = resolve_trait_impls( context, def_collector.collected_traits_impls, @@ -930,7 +927,7 @@ fn resolve_impls( let method_name = interner.function_name(method_id).to_owned(); if let Some(first_fn) = - interner.add_method(&self_type, method_name.clone(), *method_id) + interner.add_method(&self_type, method_name.clone(), *method_id, false) { let error = ResolverError::DuplicateDefinition { name: method_name, @@ -962,7 +959,6 @@ fn resolve_trait_impls( let local_mod_id = trait_impl.module_id; let module_id = ModuleId { krate: crate_id, local_id: local_mod_id }; let path_resolver = StandardPathResolver::new(module_id); - let trait_definition_ident = trait_impl.trait_path.last_segment(); let self_type_span = unresolved_type.span; @@ -989,6 +985,12 @@ fn resolve_trait_impls( } } + if matches!(self_type, Type::MutableReference(_)) { + let span = self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()); + let error = DefCollectorErrorKind::MutableReferenceInTraitImpl { span }; + errors.push((error.into(), trait_impl.file_id)); + } + let mut new_resolver = Resolver::new(interner, &path_resolver, &context.def_maps, trait_impl.file_id); new_resolver.set_self_type(Some(self_type.clone())); @@ -996,28 +998,27 @@ fn resolve_trait_impls( if let Some(trait_id) = maybe_trait_id { check_methods_signatures(&mut new_resolver, &impl_methods, trait_id, errors); - let key = TraitImplKey { typ: self_type.clone(), trait_id }; - if let Some(prev_trait_impl_ident) = interner.get_trait_implementation(&key) { - let err = DefCollectorErrorKind::Duplicate { - typ: DuplicateType::TraitImplementation, - first_def: prev_trait_impl_ident.borrow().ident.clone(), - second_def: trait_definition_ident.clone(), - }; - errors.push((err.into(), trait_impl.methods.file_id)); - } else { - let resolved_trait_impl = Shared::new(TraitImpl { - ident: trait_impl.trait_path.last_segment().clone(), + let resolved_trait_impl = Shared::new(TraitImpl { + ident: trait_impl.trait_path.last_segment().clone(), + typ: self_type.clone(), + trait_id, + file: trait_impl.file_id, + methods: vecmap(&impl_methods, |(_, func_id)| *func_id), + }); + + if let Some((prev_span, prev_file)) = + interner.add_trait_implementation(self_type.clone(), trait_id, resolved_trait_impl) + { + let error = DefCollectorErrorKind::OverlappingImpl { typ: self_type.clone(), - trait_id, - methods: vecmap(&impl_methods, |(_, func_id)| *func_id), - }); - if !interner.add_trait_implementation(&key, resolved_trait_impl.clone()) { - let error = DefCollectorErrorKind::TraitImplNotAllowedFor { - trait_path: trait_impl.trait_path.clone(), - span: self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()), - }; - errors.push((error.into(), trait_impl.file_id)); - } + span: self_type_span.unwrap_or_else(|| trait_impl.trait_path.span()), + }; + errors.push((error.into(), trait_impl.file_id)); + + // The 'previous impl defined here' note must be a separate error currently + // since it may be in a different file and all errors have the same file id. + let error = DefCollectorErrorKind::OverlappingImplNote { span: prev_span }; + errors.push((error.into(), prev_file)); } methods.append(&mut impl_methods); diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 2cc10f771f2..8658acb0864 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -33,8 +33,12 @@ pub enum DefCollectorErrorKind { PathResolutionError(PathResolutionError), #[error("Non-struct type used in impl")] NonStructTypeInImpl { span: Span }, - #[error("Trait implementation is not allowed for this")] - TraitImplNotAllowedFor { trait_path: Path, span: Span }, + #[error("Cannot implement trait on a mutable reference type")] + MutableReferenceInTraitImpl { span: Span }, + #[error("Impl overlaps with existing impl for type {typ}")] + OverlappingImpl { span: Span, typ: crate::Type }, + #[error("Previous impl defined here")] + OverlappingImplNote { span: Span }, #[error("Cannot `impl` a type defined outside the current crate")] ForeignImpl { span: Span, type_name: String }, #[error("Mismatch number of parameters in of trait implementation")] @@ -130,10 +134,25 @@ impl From for Diagnostic { "Only struct types may have implementation methods".into(), span, ), - DefCollectorErrorKind::TraitImplNotAllowedFor { trait_path, span } => { + DefCollectorErrorKind::MutableReferenceInTraitImpl { span } => Diagnostic::simple_error( + "Trait impls are not allowed on mutable reference types".into(), + "Try using a struct type here instead".into(), + span, + ), + DefCollectorErrorKind::OverlappingImpl { span, typ } => { + Diagnostic::simple_error( + format!("Impl overlaps with existing impl for type `{typ}`"), + "Overlapping impl".into(), + span, + ) + } + DefCollectorErrorKind::OverlappingImplNote { span } => { + // This should be a note or part of the previous error eventually. + // This must be an error to appear next to the previous OverlappingImpl + // error since we sort warnings first. Diagnostic::simple_error( - format!("Only limited types may implement trait `{trait_path}`"), - "Only limited types may implement traits".into(), + "Previous impl defined here".into(), + "Previous impl defined here".into(), span, ) } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 7f21bdbebda..00534cc3aa2 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -880,20 +880,14 @@ impl<'interner> TypeChecker<'interner> { // This may be a struct or a primitive type. Type::MutableReference(element) => self .interner - .lookup_mut_primitive_trait_method(element.as_ref(), method_name) + .lookup_primitive_trait_method_mut(element.as_ref(), method_name) .map(HirMethodReference::FuncId) .or_else(|| self.lookup_method(element, method_name, expr_id)), // If we fail to resolve the object to a struct type, we have no way of type // checking its arguments as we can't even resolve the name of the function Type::Error => None, - // In the future we could support methods for non-struct types if we have a context - // (in the interner?) essentially resembling HashMap - other => match self - .interner - .lookup_primitive_method(other, method_name) - .or_else(|| self.interner.lookup_primitive_trait_method(other, method_name)) - { + other => match self.interner.lookup_primitive_method(other, method_name) { Some(method_id) => Some(HirMethodReference::FuncId(method_id)), None => { self.errors.push(TypeCheckError::UnresolvedMethodCall { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 035dbbc3518..2a12d68406d 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -15,7 +15,7 @@ pub use errors::TypeCheckError; use crate::{ hir_def::{expr::HirExpression, stmt::HirStatement}, - node_interner::{ExprId, FuncId, NodeInterner, StmtId, TraitImplKey}, + node_interner::{ExprId, FuncId, NodeInterner, StmtId}, Type, }; @@ -65,8 +65,10 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec, // methods[i] is the implementation of trait.methods[i] for Type typ } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 6891bcf2872..0785f150a1e 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -24,7 +24,7 @@ use crate::{ stmt::{HirAssignStatement, HirLValue, HirLetStatement, HirPattern, HirStatement}, types, }, - node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitImplKey, TraitMethodId}, + node_interner::{self, DefinitionKind, NodeInterner, StmtId, TraitMethodId}, token::FunctionAttribute, ContractFunctionType, FunctionKind, Type, TypeBinding, TypeBindings, TypeVariableKind, Visibility, @@ -820,10 +820,7 @@ impl<'interner> Monomorphizer<'interner> { let trait_impl = self .interner - .get_trait_implementation(&TraitImplKey { - typ: self_type.follow_bindings(), - trait_id: method.trait_id, - }) + .lookup_trait_implementation(self_type.follow_bindings(), method.trait_id) .expect("ICE: missing trait impl - should be caught during type checking"); let hir_func_id = trait_impl.borrow().methods[method.method_index]; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 5febc3f4259..10d5ece2edc 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -25,13 +25,6 @@ use crate::{ TypeBinding, TypeBindings, TypeVariable, TypeVariableId, TypeVariableKind, }; -#[derive(Eq, PartialEq, Hash, Clone)] -pub struct TraitImplKey { - pub typ: Type, - pub trait_id: TraitId, - // pub generics: Generics - TODO -} - type StructAttributes = Vec; /// The node interner is the central storage location of all nodes in Noir's Hir (the @@ -92,7 +85,19 @@ pub struct NodeInterner { // Trait implementation map // For each type that implements a given Trait ( corresponding TraitId), there should be an entry here // The purpose for this hashmap is to detect duplication of trait implementations ( if any ) - trait_implementations: HashMap>, + // + // Indexed by TraitImplIds + trait_implementations: Vec>, + + /// Trait implementations on each type. This is expected to always have the same length as + /// `self.trait_implementations`. + /// + /// For lack of a better name, this maps a trait id and type combination + /// to a corresponding impl if one is available for the type. Due to generics, + /// we cannot map from Type directly to impl, we need to iterate a Vec of all impls + /// of that trait to see if any type may match. This can be further optimized later + /// by splitting it up by type. + trait_implementation_map: HashMap>, /// Map from ExprId (referring to a Function/Method call) to its corresponding TypeBindings, /// filled out during type checking from instantiated variables. Used during monomorphization @@ -113,16 +118,27 @@ pub struct NodeInterner { /// may have both `impl Struct { fn foo(){} }` and `impl Struct { fn foo(){} }`. /// If this happens, the returned Vec will have 2 entries and we'll need to further /// disambiguate them by checking the type of each function. - struct_methods: HashMap<(StructId, String), Vec>, + struct_methods: HashMap<(StructId, String), Methods>, /// Methods on primitive types defined in the stdlib. - primitive_methods: HashMap<(TypeMethodKey, String), FuncId>, + primitive_methods: HashMap<(TypeMethodKey, String), Methods>, // For trait implementation functions, this is their self type and trait they belong to func_id_to_trait: HashMap, +} - /// Trait implementations on primitive types - primitive_trait_impls: HashMap<(Type, String), FuncId>, +/// Represents the methods on a given type that each share the same name. +/// +/// Methods are split into inherent methods and trait methods. If there is +/// ever a name that is defined on both a type directly, and defined indirectly +/// via a trait impl, the direct (inherent) name will always take precedence. +/// +/// Additionally, types can define specialized impls with methods of the same name +/// as long as these specialized impls do not overlap. E.g. `impl Struct` and `impl Struct` +#[derive(Default)] +pub struct Methods { + direct: Vec, + trait_impl_methods: Vec, } /// All the information from a function that is filled out during definition collection rather than @@ -257,6 +273,9 @@ impl TraitId { } } +#[derive(Debug, Eq, PartialEq, Hash, Clone, Copy)] +pub struct TraitImplId(usize); + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TraitMethodId { pub trait_id: TraitId, @@ -380,14 +399,14 @@ impl Default for NodeInterner { struct_attributes: HashMap::new(), type_aliases: Vec::new(), traits: HashMap::new(), - trait_implementations: HashMap::new(), + trait_implementations: Vec::new(), + trait_implementation_map: HashMap::new(), instantiation_bindings: HashMap::new(), field_indices: HashMap::new(), next_type_variable_id: std::cell::Cell::new(0), globals: HashMap::new(), struct_methods: HashMap::new(), primitive_methods: HashMap::new(), - primitive_trait_impls: HashMap::new(), }; // An empty block expression is used often, we add this into the `node` on startup @@ -891,6 +910,7 @@ impl NodeInterner { self_type: &Type, method_name: String, method_id: FuncId, + is_trait_method: bool, ) -> Option { match self_type { Type::Struct(struct_type, _generics) => { @@ -901,68 +921,75 @@ impl NodeInterner { } let key = (id, method_name); - self.struct_methods.entry(key).or_default().push(method_id); + self.struct_methods.entry(key).or_default().add_method(method_id, is_trait_method); None } Type::Error => None, + Type::MutableReference(element) => { + self.add_method(element, method_name, method_id, is_trait_method) + } other => { let key = get_type_method_key(self_type).unwrap_or_else(|| { unreachable!("Cannot add a method to the unsupported type '{}'", other) }); - self.primitive_methods.insert((key, method_name), method_id) + self.primitive_methods + .entry((key, method_name)) + .or_default() + .add_method(method_id, is_trait_method); + None } } } - pub fn get_trait_implementation(&self, key: &TraitImplKey) -> Option> { - self.trait_implementations.get(key).cloned() + pub fn get_trait_implementation(&self, id: TraitImplId) -> Shared { + self.trait_implementations[id.0].clone() + } + + pub fn lookup_trait_implementation( + &self, + object_type: Type, + trait_id: TraitId, + ) -> Option> { + let impls = self.trait_implementation_map.get(&trait_id)?; + for (existing_object_type, impl_id) in impls { + if object_type.try_unify(existing_object_type).is_ok() { + return Some(self.get_trait_implementation(*impl_id)); + } + } + None } pub fn add_trait_implementation( &mut self, - key: &TraitImplKey, + object_type: Type, + trait_id: TraitId, trait_impl: Shared, - ) -> bool { - self.trait_implementations.insert(key.clone(), trait_impl.clone()); - match &key.typ { - Type::Struct(..) => { - for func_id in &trait_impl.borrow().methods { - let method_name = self.function_name(func_id).to_owned(); - self.add_method(&key.typ, method_name, *func_id); - } - true - } - Type::FieldElement - | Type::Unit - | Type::Array(..) - | Type::Integer(..) - | Type::Bool - | Type::Tuple(..) - | Type::String(..) - | Type::FmtString(..) - | Type::Function(..) - | Type::MutableReference(..) => { - for func_id in &trait_impl.borrow().methods { - let method_name = self.function_name(func_id).to_owned(); - let key = (key.typ.clone(), method_name); - self.primitive_trait_impls.insert(key, *func_id); - } - true + ) -> Option<(Span, FileId)> { + let id = TraitImplId(self.trait_implementations.len()); + + self.trait_implementations.push(trait_impl.clone()); + + let entries = self.trait_implementation_map.entry(trait_id).or_default(); + + // Check that this new impl does not overlap with any existing impls first + for (existing_object_type, existing_impl_id) in entries { + if object_type.try_unify(existing_object_type).is_ok() { + // Overlapping impl + let existing_impl = &self.trait_implementations[existing_impl_id.0]; + let existing_impl = existing_impl.borrow(); + return Some((existing_impl.ident.span(), existing_impl.file)); } - // We should allow implementing traits NamedGenerics will also eventually be possible once we support generics - // impl Foo for T - // but it's fine not to include these until we do. - Type::NamedGeneric(..) => false, - // prohibited are internal types (like NotConstant, TypeVariable, Forall, and Error) that - // aren't possible for users to write anyway - Type::TypeVariable(..) - | Type::Forall(..) - | Type::NotConstant - | Type::Constant(..) - | Type::TraitAsType(..) - | Type::Error => false, } + + for method in &trait_impl.borrow().methods { + let method_name = self.function_name(method).to_owned(); + self.add_method(&object_type, method_name, *method, true); + } + + let entries = self.trait_implementation_map.entry(trait_id).or_default(); + entries.push((object_type, id)); + None } /// Search by name for a method on the given struct. @@ -981,25 +1008,94 @@ impl NodeInterner { typ: &Type, id: StructId, method_name: &str, - check_type: bool, + force_type_check: bool, ) -> Option { let methods = self.struct_methods.get(&(id, method_name.to_owned()))?; // If there is only one method, just return it immediately. // It will still be typechecked later. - if !check_type && methods.len() == 1 { - return Some(methods[0]); + if !force_type_check { + if let Some(method) = methods.get_unambiguous() { + return Some(method); + } } + self.find_matching_method(typ, methods, method_name) + } + + /// Select the 1 matching method with an object type matching `typ` + fn find_matching_method( + &self, + typ: &Type, + methods: &Methods, + method_name: &str, + ) -> Option { + if let Some(method) = methods.find_matching_method(typ, self) { + Some(method) + } else { + // Failed to find a match for the type in question, switch to looking at impls + // for all types `T`, e.g. `impl Foo for T` + let key = &(TypeMethodKey::Generic, method_name.to_owned()); + let global_methods = self.primitive_methods.get(key)?; + global_methods.find_matching_method(typ, self) + } + } + + /// Looks up a given method name on the given primitive type. + pub fn lookup_primitive_method(&self, typ: &Type, method_name: &str) -> Option { + let key = get_type_method_key(typ)?; + let methods = self.primitive_methods.get(&(key, method_name.to_owned()))?; + self.find_matching_method(typ, methods, method_name) + } + + pub fn lookup_primitive_trait_method_mut( + &self, + typ: &Type, + method_name: &str, + ) -> Option { + let typ = Type::MutableReference(Box::new(typ.clone())); + self.lookup_primitive_method(&typ, method_name) + } +} + +impl Methods { + /// Get a single, unambiguous reference to a name if one exists. + /// If not, there may be multiple methods of the same name for a given + /// type or there may be no methods at all. + fn get_unambiguous(&self) -> Option { + if self.direct.len() == 1 { + Some(self.direct[0]) + } else if self.direct.is_empty() && self.trait_impl_methods.len() == 1 { + Some(self.trait_impl_methods[0]) + } else { + None + } + } + + fn add_method(&mut self, method: FuncId, is_trait_method: bool) { + if is_trait_method { + self.trait_impl_methods.push(method); + } else { + self.direct.push(method); + } + } + + /// Iterate through each method, starting with the direct methods + fn iter(&self) -> impl Iterator + '_ { + self.direct.iter().copied().chain(self.trait_impl_methods.iter().copied()) + } + + /// Select the 1 matching method with an object type matching `typ` + fn find_matching_method(&self, typ: &Type, interner: &NodeInterner) -> Option { // When adding methods we always check they do not overlap, so there should be // at most 1 matching method in this list. - for method in methods { - match self.function_meta(method).typ.instantiate(self).0 { + for method in self.iter() { + match interner.function_meta(&method).typ.instantiate(interner).0 { Type::Function(args, _, _) => { if let Some(object) = args.get(0) { // TODO #3089: This is dangerous! try_unify may commit type bindings even on failure if object.try_unify(typ).is_ok() { - return Some(*method); + return Some(method); } } } @@ -1007,29 +1103,8 @@ impl NodeInterner { other => unreachable!("Expected function type, found {other}"), } } - None } - - /// Looks up a given method name on the given primitive type. - pub fn lookup_primitive_method(&self, typ: &Type, method_name: &str) -> Option { - get_type_method_key(typ) - .and_then(|key| self.primitive_methods.get(&(key, method_name.to_owned())).copied()) - } - - pub fn lookup_primitive_trait_method(&self, typ: &Type, method_name: &str) -> Option { - self.primitive_trait_impls.get(&(typ.clone(), method_name.to_string())).copied() - } - - pub fn lookup_mut_primitive_trait_method( - &self, - typ: &Type, - method_name: &str, - ) -> Option { - self.primitive_trait_impls - .get(&(Type::MutableReference(Box::new(typ.clone())), method_name.to_string())) - .copied() - } } /// These are the primitive type variants that we support adding methods to @@ -1041,9 +1116,11 @@ enum TypeMethodKey { Array, Bool, String, + FmtString, Unit, Tuple, Function, + Generic, } fn get_type_method_key(typ: &Type) -> Option { @@ -1056,20 +1133,20 @@ fn get_type_method_key(typ: &Type) -> Option { Type::TypeVariable(_, TypeVariableKind::IntegerOrField) => Some(FieldOrInt), Type::Bool => Some(Bool), Type::String(_) => Some(String), + Type::FmtString(_, _) => Some(FmtString), Type::Unit => Some(Unit), Type::Tuple(_) => Some(Tuple), Type::Function(_, _, _) => Some(Function), + Type::NamedGeneric(_, _) => Some(Generic), Type::MutableReference(element) => get_type_method_key(element), // We do not support adding methods to these types Type::TypeVariable(_, _) - | Type::NamedGeneric(_, _) | Type::Forall(_, _) | Type::Constant(_) | Type::Error | Type::NotConstant | Type::Struct(_, _) - | Type::TraitAsType(_) - | Type::FmtString(_, _) => None, + | Type::TraitAsType(_) => None, } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index b7953d0797c..6a1cf80accd 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -537,14 +537,9 @@ mod test { "; let errors = get_program_errors(src); assert!(!has_parser_error(&errors)); - assert!(errors.len() == 2, "Expected 2 errors, got: {:?}", errors); + assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); for (err, _file_id) in errors { match &err { - CompilationError::DefinitionError( - DefCollectorErrorKind::TraitImplNotAllowedFor { trait_path, span: _ }, - ) => { - assert_eq!(trait_path.as_string(), "Default"); - } CompilationError::ResolverError(ResolverError::Expected { expected, got, .. }) => { @@ -663,18 +658,15 @@ mod test { "; let errors = get_program_errors(src); assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); + assert!(errors.len() == 2, "Expected 2 errors, got: {:?}", errors); for (err, _file_id) in errors { match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { - typ, - first_def, - second_def, - }) => { - assert_eq!(typ, &DuplicateType::TraitImplementation); - assert_eq!(first_def, "Default"); - assert_eq!(second_def, "Default"); - } + CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImpl { + .. + }) => (), + CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImplNote { + .. + }) => (), _ => { panic!("No other errors are expected! Found = {:?}", err); } @@ -704,18 +696,15 @@ mod test { "; let errors = get_program_errors(src); assert!(!has_parser_error(&errors)); - assert!(errors.len() == 1, "Expected 1 error, got: {:?}", errors); + assert!(errors.len() == 2, "Expected 2 errors, got: {:?}", errors); for (err, _file_id) in errors { match &err { - CompilationError::DefinitionError(DefCollectorErrorKind::Duplicate { - typ, - first_def, - second_def, - }) => { - assert_eq!(typ, &DuplicateType::TraitImplementation); - assert_eq!(first_def, "Default"); - assert_eq!(second_def, "Default"); - } + CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImpl { + .. + }) => (), + CompilationError::DefinitionError(DefCollectorErrorKind::OverlappingImplNote { + .. + }) => (), _ => { panic!("No other errors are expected! Found = {:?}", err); } diff --git a/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/Nargo.toml b/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/Nargo.toml new file mode 100644 index 00000000000..e48565a2111 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "overlapping_generic_impls" +type = "bin" +authors = [""] +compiler_version = "0.18.0" + +[dependencies] \ No newline at end of file diff --git a/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/src/main.nr b/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/src/main.nr new file mode 100644 index 00000000000..a835670e0f8 --- /dev/null +++ b/tooling/nargo_cli/tests/compile_failure/overlapping_generic_impls/src/main.nr @@ -0,0 +1,8 @@ + +trait Trait { fn t(self); } + +impl Trait for T { fn t(self){} } + +impl Trait for T { fn t(self){} } + +fn main() {} diff --git a/tooling/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr b/tooling/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr index f5f01c79ad6..f359937b739 100644 --- a/tooling/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr +++ b/tooling/nargo_cli/tests/compile_success_empty/trait_override_implementation/src/main.nr @@ -4,9 +4,8 @@ trait Default { fn default(x: Field, y: Field) -> Self; fn method2(x: Field) -> Field { - x + x } - } struct Foo { @@ -20,7 +19,7 @@ impl Default for Foo { } fn method2(x: Field) -> Field { - x * 3 + x * 3 } } @@ -40,32 +39,35 @@ impl F for Bar { fn f3(self) -> Field { 30 } } -impl F for &mut Bar { - fn f1(self) -> Field { 101 } - fn f5(self) -> Field { 505 } -} +// Impls on mutable references are temporarily disabled +// impl F for &mut Bar { +// fn f1(self) -> Field { 101 } +// fn f5(self) -> Field { 505 } +// } fn main(x: Field) { let first = Foo::method2(x); assert(first == 3 * x); let bar = Bar{}; - assert(bar.f1() == 10); - assert(bar.f2() == 2); - assert(bar.f3() == 30); - assert(bar.f4() == 4); - assert(bar.f5() == 50); + assert(bar.f1() == 10, "1"); + assert(bar.f2() == 2, "2"); + assert(bar.f3() == 30, "3"); + assert(bar.f4() == 4, "4"); + assert(bar.f5() == 50, "5"); let mut bar_mut = Bar{}; - assert((&mut bar_mut).f1() == 101); - assert((&mut bar_mut).f2() == 2); - assert((&mut bar_mut).f3() == 3); - assert((&mut bar_mut).f4() == 4); - assert((&mut bar_mut).f5() == 505); - assert(bar_mut.f1() == 10); - assert(bar_mut.f2() == 2); - assert(bar_mut.f3() == 30); - assert(bar_mut.f4() == 4); - assert(bar_mut.f5() == 50); -} \ No newline at end of file + // Impls on mutable references are temporarily disabled + // assert_eq((&mut bar_mut).f1(), 101); + // assert((&mut bar_mut).f2() == 2, "7"); + // assert((&mut bar_mut).f3() == 3, "8"); + // assert((&mut bar_mut).f4() == 4, "9"); + // assert((&mut bar_mut).f5() == 505, "10"); + + assert(bar_mut.f1() == 10, "10"); + assert(bar_mut.f2() == 2, "12"); + assert(bar_mut.f3() == 30, "13"); + assert(bar_mut.f4() == 4, "14"); + assert(bar_mut.f5() == 50, "15"); +} diff --git a/tooling/nargo_cli/tests/execution_success/trait_impl_base_type/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_impl_base_type/src/main.nr index 47e919cf39c..e01e243a78f 100644 --- a/tooling/nargo_cli/tests/execution_success/trait_impl_base_type/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/trait_impl_base_type/src/main.nr @@ -81,26 +81,6 @@ fn some_func(x: u32) -> u32 { } -trait MutFieldable { - fn mut_to_field(self) -> Field; -} - -impl MutFieldable for &mut u64 { - fn mut_to_field(self) -> Field { - 1337 as Field - } -} - -fn a(y: &mut u64) -> Field { - y.mut_to_field() -} - -impl Fieldable for &mut u64 { - fn to_field(self) -> Field { - 777 as Field - } -} - impl Fieldable for u64 { fn to_field(self) -> Field { 66 as Field @@ -134,8 +114,5 @@ fn main(x: u32) { assert(some_func.to_field() == 17); let mut y = 0 as u64; - assert(a(&mut y) == 1337); - assert((&mut y).mut_to_field() == 1337); - assert((&mut y).to_field() == 777); assert(y.to_field() == 66); -} \ No newline at end of file +}