diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 4b3916ae083..0bb8641b6b3 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -419,7 +419,7 @@ impl<'context> Elaborator<'context> { // If we get here the type has no field named 'access.rhs'. // Now we specialize the error message based on whether we know the object type in question yet. if let Type::TypeVariable(..) = &lhs_type { - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); + self.push_err(TypeCheckError::TypeAnnotationsNeededForFieldAccess { span }); } else if lhs_type != Type::Error { self.push_err(TypeCheckError::AccessUnknownMember { lhs_type, diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index a82a4fa2d20..3403053a183 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -32,8 +32,8 @@ use crate::{ SecondaryAttribute, Signedness, UnaryOp, UnresolvedType, UnresolvedTypeData, }, node_interner::{ - DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, TraitId, TraitImplKind, - TraitMethodId, + DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ImplSearchErrorKind, TraitId, + TraitImplKind, TraitMethodId, }, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, TypeVariable, TypeVariableKind, @@ -122,7 +122,7 @@ impl<'context> Elaborator<'context> { Unit => Type::Unit, Unspecified => { let span = typ.span; - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); + self.push_err(TypeCheckError::UnspecifiedType { span }); Type::Error } Error => Type::Error, @@ -482,7 +482,7 @@ impl<'context> Elaborator<'context> { match self.interner.lookup_trait_implementation(&object_type, trait_id, &ordered, &named) { Ok(impl_kind) => self.get_associated_type_from_trait_impl(path, impl_kind), Err(constraints) => { - self.push_trait_constraint_error(constraints, span); + self.push_trait_constraint_error(&object_type, constraints, span); Type::Error } } @@ -1332,7 +1332,7 @@ impl<'context> Elaborator<'context> { // The type variable must be unbound at this point since follow_bindings was called Type::TypeVariable(_, TypeVariableKind::Normal) => { - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); + self.push_err(TypeCheckError::TypeAnnotationsNeededForMethodCall { span }); None } @@ -1596,16 +1596,34 @@ impl<'context> Elaborator<'context> { Ok(impl_kind) => { self.interner.select_impl_for_expression(function_ident_id, impl_kind); } - Err(constraints) => self.push_trait_constraint_error(constraints, span), + Err(error) => self.push_trait_constraint_error(object_type, error, span), } } - fn push_trait_constraint_error(&mut self, constraints: Vec, span: Span) { - if constraints.is_empty() { - self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); - } else if let Some(error) = NoMatchingImplFoundError::new(self.interner, constraints, span) - { - self.push_err(TypeCheckError::NoMatchingImplFound(error)); + fn push_trait_constraint_error( + &mut self, + object_type: &Type, + error: ImplSearchErrorKind, + span: Span, + ) { + match error { + ImplSearchErrorKind::TypeAnnotationsNeededOnObjectType => { + self.push_err(TypeCheckError::TypeAnnotationsNeededForMethodCall { span }); + } + ImplSearchErrorKind::Nested(constraints) => { + if let Some(error) = NoMatchingImplFoundError::new(self.interner, constraints, span) + { + self.push_err(TypeCheckError::NoMatchingImplFound(error)); + } + } + ImplSearchErrorKind::MultipleMatching(candidates) => { + let object_type = object_type.clone(); + self.push_err(TypeCheckError::MultipleMatchingImpls { + object_type, + span, + candidates, + }); + } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index b7b49090232..fd916485eaf 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -188,11 +188,19 @@ pub enum InterpreterError { FunctionAlreadyResolved { location: Location, }, + MultipleMatchingImpls { + object_type: Type, + candidates: Vec, + location: Location, + }, Unimplemented { item: String, location: Location, }, + TypeAnnotationsNeededForMethodCall { + location: Location, + }, // These cases are not errors, they are just used to prevent us from running more code // until the loop can be resumed properly. These cases will never be displayed to users. @@ -257,8 +265,10 @@ impl InterpreterError { | InterpreterError::ContinueNotInLoop { location, .. } | InterpreterError::TraitDefinitionMustBeAPath { location } | InterpreterError::FailedToResolveTraitDefinition { location } - | InterpreterError::FailedToResolveTraitBound { location, .. } => *location, - InterpreterError::FunctionAlreadyResolved { location, .. } => *location, + | InterpreterError::FailedToResolveTraitBound { location, .. } + | InterpreterError::FunctionAlreadyResolved { location, .. } + | InterpreterError::MultipleMatchingImpls { location, .. } + | InterpreterError::TypeAnnotationsNeededForMethodCall { location } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) @@ -527,6 +537,26 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { .to_string(); CustomDiagnostic::simple_error(msg, secondary, location.span) } + InterpreterError::MultipleMatchingImpls { object_type, candidates, location } => { + let message = format!("Multiple trait impls match the object type `{object_type}`"); + let secondary = "Ambiguous impl".to_string(); + let mut error = CustomDiagnostic::simple_error(message, secondary, location.span); + for (i, candidate) in candidates.iter().enumerate() { + error.add_note(format!("Candidate {}: `{candidate}`", i + 1)); + } + error + } + InterpreterError::TypeAnnotationsNeededForMethodCall { location } => { + let mut error = CustomDiagnostic::simple_error( + "Object type is unknown in method call".to_string(), + "Type must be known by this point to know which method to call".to_string(), + location.span, + ); + let message = + "Try adding a type annotation for the object type before this method call"; + error.add_note(message.to_string()); + error + } } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 316f1579961..17642843757 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -104,8 +104,12 @@ pub enum TypeCheckError { second_type: String, second_index: usize, }, - #[error("Cannot infer type of expression, type annotations needed before this point")] - TypeAnnotationsNeeded { span: Span }, + #[error("Object type is unknown in method call")] + TypeAnnotationsNeededForMethodCall { span: Span }, + #[error("Object type is unknown in field access")] + TypeAnnotationsNeededForFieldAccess { span: Span }, + #[error("Multiple trait impls may apply to this object type")] + MultipleMatchingImpls { object_type: Type, candidates: Vec, span: Span }, #[error("use of deprecated function {name}")] CallDeprecated { name: String, note: Option, span: Span }, #[error("{0}")] @@ -166,6 +170,10 @@ pub enum TypeCheckError { NoSuchNamedTypeArg { name: Ident, item: String }, #[error("`{item}` is missing the associated type `{name}`")] MissingNamedTypeArg { name: Rc, item: String, span: Span }, + #[error("Internal compiler error: type unspecified for value")] + UnspecifiedType { span: Span }, + #[error("Binding `{typ}` here to the `_` inside would create a cyclic type")] + CyclicType { typ: Type, span: Span }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -286,11 +294,33 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { format!("return type is {typ}"), *span, ), - TypeCheckError::TypeAnnotationsNeeded { span } => Diagnostic::simple_error( - "Expression type is ambiguous".to_string(), - "Type must be known at this point".to_string(), - *span, - ), + TypeCheckError::TypeAnnotationsNeededForMethodCall { span } => { + let mut error = Diagnostic::simple_error( + "Object type is unknown in method call".to_string(), + "Type must be known by this point to know which method to call".to_string(), + *span, + ); + error.add_note("Try adding a type annotation for the object type before this method call".to_string()); + error + }, + TypeCheckError::TypeAnnotationsNeededForFieldAccess { span } => { + let mut error = Diagnostic::simple_error( + "Object type is unknown in field access".to_string(), + "Type must be known by this point".to_string(), + *span, + ); + error.add_note("Try adding a type annotation for the object type before this expression".to_string()); + error + }, + TypeCheckError::MultipleMatchingImpls { object_type, candidates, span } => { + let message = format!("Multiple trait impls match the object type `{object_type}`"); + let secondary = "Ambiguous impl".to_string(); + let mut error = Diagnostic::simple_error(message, secondary, *span); + for (i, candidate) in candidates.iter().enumerate() { + error.add_note(format!("Candidate {}: `{candidate}`", i + 1)); + } + error + }, TypeCheckError::ResolverError(error) => error.into(), TypeCheckError::TypeMismatchWithSource { expected, actual, span, source } => { let message = match source { @@ -388,6 +418,12 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { TypeCheckError::UnsafeFn { span } => { Diagnostic::simple_warning(error.to_string(), String::new(), *span) } + TypeCheckError::UnspecifiedType { span } => { + Diagnostic::simple_error(error.to_string(), String::new(), *span) + } + TypeCheckError::CyclicType { typ: _, span } => { + Diagnostic::simple_error(error.to_string(), "Cyclic types have unlimited size and are prohibited in Noir".into(), *span) + } } } } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 6fd1727df21..807666f9af9 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -544,7 +544,7 @@ impl TypeVariable { }; if binding.occurs(id) { - Err(TypeCheckError::TypeAnnotationsNeeded { span }) + Err(TypeCheckError::CyclicType { span, typ: binding }) } else { *self.1.borrow_mut() = TypeBinding::Bound(binding); Ok(()) diff --git a/compiler/noirc_frontend/src/monomorphization/errors.rs b/compiler/noirc_frontend/src/monomorphization/errors.rs index df61c138c02..665bf26f7b9 100644 --- a/compiler/noirc_frontend/src/monomorphization/errors.rs +++ b/compiler/noirc_frontend/src/monomorphization/errors.rs @@ -1,11 +1,11 @@ use noirc_errors::{CustomDiagnostic, FileDiagnostic, Location}; -use crate::hir::comptime::InterpreterError; +use crate::{hir::comptime::InterpreterError, Type}; #[derive(Debug)] pub enum MonomorphizationError { - UnknownArrayLength { location: Location }, - TypeAnnotationsNeeded { location: Location }, + UnknownArrayLength { length: Type, location: Location }, + NoDefaultType { location: Location }, InternalError { message: &'static str, location: Location }, InterpreterError(InterpreterError), } @@ -13,9 +13,9 @@ pub enum MonomorphizationError { impl MonomorphizationError { fn location(&self) -> Location { match self { - MonomorphizationError::UnknownArrayLength { location } + MonomorphizationError::UnknownArrayLength { location, .. } | MonomorphizationError::InternalError { location, .. } - | MonomorphizationError::TypeAnnotationsNeeded { location } => *location, + | MonomorphizationError::NoDefaultType { location, .. } => *location, MonomorphizationError::InterpreterError(error) => error.get_location(), } } @@ -32,16 +32,20 @@ impl From for FileDiagnostic { impl MonomorphizationError { fn into_diagnostic(self) -> CustomDiagnostic { - let message = match self { - MonomorphizationError::UnknownArrayLength { .. } => { - "Length of generic array could not be determined." + let message = match &self { + MonomorphizationError::UnknownArrayLength { length, .. } => { + format!("ICE: Could not determine array length `{length}`") } - MonomorphizationError::TypeAnnotationsNeeded { .. } => "Type annotations needed", - MonomorphizationError::InterpreterError(error) => return (&error).into(), - MonomorphizationError::InternalError { message, .. } => message, + MonomorphizationError::NoDefaultType { location } => { + let message = "Type annotation needed".into(); + let secondary = "Could not determine type of generic argument".into(); + return CustomDiagnostic::simple_error(message, secondary, location.span); + } + MonomorphizationError::InterpreterError(error) => return error.into(), + MonomorphizationError::InternalError { message, .. } => message.to_string(), }; let location = self.location(); - CustomDiagnostic::simple_error(message.into(), String::new(), location.span) + CustomDiagnostic::simple_error(message, String::new(), location.span) } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index c45a759aab6..edb831b2158 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -11,7 +11,7 @@ use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; use crate::hir::type_check::NoMatchingImplFoundError; -use crate::node_interner::ExprId; +use crate::node_interner::{ExprId, ImplSearchErrorKind}; use crate::{ debug::DebugInstrumenter, hir_def::{ @@ -569,7 +569,7 @@ impl<'interner> Monomorphizer<'interner> { let length = length.evaluate_to_u32().ok_or_else(|| { let location = self.interner.expr_location(&array); - MonomorphizationError::UnknownArrayLength { location } + MonomorphizationError::UnknownArrayLength { location, length } })?; let contents = try_vecmap(0..length, |_| self.expr(repeated_element))?; @@ -936,7 +936,10 @@ impl<'interner> Monomorphizer<'interner> { let element = Box::new(Self::convert_type(element.as_ref(), location)?); let length = match length.evaluate_to_u32() { Some(length) => length, - None => return Err(MonomorphizationError::TypeAnnotationsNeeded { location }), + None => { + let length = length.as_ref().clone(); + return Err(MonomorphizationError::UnknownArrayLength { location, length }); + } }; ast::Type::Array(length, element) } @@ -969,7 +972,7 @@ impl<'interner> Monomorphizer<'interner> { // and within a larger generic type. let default = match kind.default_type() { Some(typ) => typ, - None => return Err(MonomorphizationError::TypeAnnotationsNeeded { location }), + None => return Err(MonomorphizationError::NoDefaultType { location }), }; let monomorphized_default = Self::convert_type(&default, location)?; @@ -1074,7 +1077,7 @@ impl<'interner> Monomorphizer<'interner> { // and within a larger generic type. let default = match kind.default_type() { Some(typ) => typ, - None => return Err(MonomorphizationError::TypeAnnotationsNeeded { location }), + None => return Err(MonomorphizationError::NoDefaultType { location }), }; Self::check_type(&default, location) @@ -1946,6 +1949,7 @@ pub fn resolve_trait_method( let impl_id = match trait_impl { TraitImplKind::Normal(impl_id) => impl_id, TraitImplKind::Assumed { object_type, trait_generics } => { + let location = interner.expr_location(&expr_id); match interner.lookup_trait_implementation( &object_type, method.trait_id, @@ -1954,21 +1958,28 @@ pub fn resolve_trait_method( ) { Ok(TraitImplKind::Normal(impl_id)) => impl_id, Ok(TraitImplKind::Assumed { .. }) => { - let location = interner.expr_location(&expr_id); return Err(InterpreterError::NoImpl { location }); } - Err(constraints) => { - let location = interner.expr_location(&expr_id); + Err(ImplSearchErrorKind::TypeAnnotationsNeededOnObjectType) => { + return Err(InterpreterError::TypeAnnotationsNeededForMethodCall { location }); + } + Err(ImplSearchErrorKind::Nested(constraints)) => { if let Some(error) = NoMatchingImplFoundError::new(interner, constraints, location.span) { let file = location.file; return Err(InterpreterError::NoMatchingImplFound { error, file }); } else { - let location = interner.expr_location(&expr_id); return Err(InterpreterError::NoImpl { location }); } } + Err(ImplSearchErrorKind::MultipleMatching(candidates)) => { + return Err(InterpreterError::MultipleMatchingImpls { + object_type, + location, + candidates, + }); + } } } }; diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index b3d9cf25d42..2c0426f6938 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -318,6 +318,13 @@ pub enum TraitImplKind { }, } +/// When searching for a trait impl, these are the types of errors we can expect +pub enum ImplSearchErrorKind { + TypeAnnotationsNeededOnObjectType, + Nested(Vec), + MultipleMatching(Vec), +} + /// 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 @@ -1383,7 +1390,7 @@ impl NodeInterner { trait_id: TraitId, trait_generics: &[Type], trait_associated_types: &[NamedType], - ) -> Result> { + ) -> Result { let (impl_kind, bindings) = self.try_lookup_trait_implementation( object_type, trait_id, @@ -1425,7 +1432,7 @@ impl NodeInterner { trait_id: TraitId, trait_generics: &[Type], trait_associated_types: &[NamedType], - ) -> Result<(TraitImplKind, TypeBindings), Vec> { + ) -> Result<(TraitImplKind, TypeBindings), ImplSearchErrorKind> { let mut bindings = TypeBindings::new(); let impl_kind = self.lookup_trait_implementation_helper( object_type, @@ -1451,7 +1458,7 @@ impl NodeInterner { trait_associated_types: &[NamedType], type_bindings: &mut TypeBindings, recursion_limit: u32, - ) -> Result> { + ) -> Result { let make_constraint = || { let ordered = trait_generics.to_vec(); let named = trait_associated_types.to_vec(); @@ -1463,29 +1470,29 @@ impl NodeInterner { } }; + let nested_error = || ImplSearchErrorKind::Nested(vec![make_constraint()]); + // Prevent infinite recursion when looking for impls if recursion_limit == 0 { - return Err(vec![make_constraint()]); + return Err(nested_error()); } let object_type = object_type.substitute(type_bindings); // If the object type isn't known, just return an error saying type annotations are needed. if object_type.is_bindable() { - return Err(Vec::new()); + return Err(ImplSearchErrorKind::TypeAnnotationsNeededOnObjectType); } - let impls = - self.trait_implementation_map.get(&trait_id).ok_or_else(|| vec![make_constraint()])?; + let impls = self.trait_implementation_map.get(&trait_id).ok_or_else(nested_error)?; let mut matching_impls = Vec::new(); + let mut where_clause_error = None; - let mut where_clause_errors = Vec::new(); - - for (existing_object_type2, impl_kind) in impls { + for (existing_object_type, impl_kind) in impls { // Bug: We're instantiating only the object type's generics here, not all of the trait's generics like we need to let (existing_object_type, instantiation_bindings) = - existing_object_type2.instantiate(self); + existing_object_type.instantiate(self); let mut fresh_bindings = type_bindings.clone(); @@ -1503,19 +1510,18 @@ impl NodeInterner { ) }; - let generics_match = match impl_kind { + let trait_generics = match impl_kind { TraitImplKind::Normal(id) => { let shared_impl = self.get_trait_implementation(*id); let shared_impl = shared_impl.borrow(); - let impl_associated_types = self.get_associated_types_for_impl(*id); - check_trait_generics(&shared_impl.trait_generics, impl_associated_types) - } - TraitImplKind::Assumed { trait_generics, .. } => { - check_trait_generics(&trait_generics.ordered, &trait_generics.named) + let named = self.get_associated_types_for_impl(*id).to_vec(); + let ordered = shared_impl.trait_generics.clone(); + TraitGenerics { named, ordered } } + TraitImplKind::Assumed { trait_generics, .. } => trait_generics.clone(), }; - if !generics_match { + if !check_trait_generics(&trait_generics.ordered, &trait_generics.named) { continue; } @@ -1524,34 +1530,48 @@ impl NodeInterner { let trait_impl = self.get_trait_implementation(*impl_id); let trait_impl = trait_impl.borrow(); - if let Err(errors) = self.validate_where_clause( + if let Err(error) = self.validate_where_clause( &trait_impl.where_clause, &mut fresh_bindings, &instantiation_bindings, recursion_limit, ) { // Only keep the first errors we get from a failing where clause - if where_clause_errors.is_empty() { - where_clause_errors.extend(errors); + if where_clause_error.is_none() { + where_clause_error = Some(error); } continue; } } - matching_impls.push((impl_kind.clone(), fresh_bindings)); + let constraint = TraitConstraint { + typ: existing_object_type, + trait_id, + trait_generics, + span: Span::default(), + }; + matching_impls.push((impl_kind.clone(), fresh_bindings, constraint)); } } if matching_impls.len() == 1 { - let (impl_, fresh_bindings) = matching_impls.pop().unwrap(); + let (impl_, fresh_bindings, _) = matching_impls.pop().unwrap(); *type_bindings = fresh_bindings; Ok(impl_) } else if matching_impls.is_empty() { - where_clause_errors.push(make_constraint()); - Err(where_clause_errors) + let mut errors = match where_clause_error { + Some((_, ImplSearchErrorKind::Nested(errors))) => errors, + Some((constraint, _other)) => vec![constraint], + None => vec![], + }; + errors.push(make_constraint()); + Err(ImplSearchErrorKind::Nested(errors)) } else { - // multiple matching impls, type annotations needed - Err(vec![]) + let impls = vecmap(matching_impls, |(_, _, constraint)| { + let name = &self.get_trait(constraint.trait_id).name; + format!("{}: {name}{}", constraint.typ, constraint.trait_generics) + }); + Err(ImplSearchErrorKind::MultipleMatching(impls)) } } @@ -1563,7 +1583,7 @@ impl NodeInterner { type_bindings: &mut TypeBindings, instantiation_bindings: &TypeBindings, recursion_limit: u32, - ) -> Result<(), Vec> { + ) -> Result<(), (TraitConstraint, ImplSearchErrorKind)> { for constraint in where_clause { // Instantiation bindings are generally safe to force substitute into the same type. // This is needed here to undo any bindings done to trait methods by monomorphization. @@ -1591,7 +1611,8 @@ impl NodeInterner { // our impl list, which we don't want to bind to. type_bindings, recursion_limit - 1, - )?; + ) + .map_err(|error| (constraint.clone(), error))?; } Ok(())