diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 7d304990dd8..9c72529e11a 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -547,7 +547,7 @@ impl<'context> Elaborator<'context> { trait_id: trait_id.trait_id, trait_generics: Vec::new(), }; - self.trait_constraints.push((constraint, expr_id)); + self.push_trait_constraint(constraint, expr_id); self.type_check_operator_method(expr_id, trait_id, &lhs_type, span); } typ @@ -663,7 +663,14 @@ impl<'context> Elaborator<'context> { } fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) { + // We have to push a new FunctionContext so that we can resolve any constraints + // in this comptime block early before the function as a whole finishes elaborating. + // Otherwise the interpreter below may find expressions for which the underlying trait + // call is not yet solved for. + self.function_context.push(Default::default()); let (block, _typ) = self.elaborate_block_expression(block); + self.check_and_pop_function_context(); + let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id); let value = interpreter.evaluate_block(block); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 3938e08ba76..4b4cf33aa2a 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -138,16 +138,14 @@ pub struct Elaborator<'context> { current_function: Option, - /// All type variables created in the current function. - /// This map is used to default any integer type variables at the end of - /// a function (before checking trait constraints) if a type wasn't already chosen. - type_variables: Vec, - - /// Trait constraints are collected during type checking until they are - /// verified at the end of a function. This is because constraints arise - /// on each variable, but it is only until function calls when the types - /// needed for the trait constraint may become known. - trait_constraints: Vec<(TraitConstraint, ExprId)>, + /// This is a stack of function contexts. Most of the time, for each function we + /// expect this to be of length one, containing each type variable and trait constraint + /// used in the function. This is also pushed to when a `comptime {}` block is used within + /// the function. Since it can force us to resolve that block's trait constraints earlier + /// so that they are resolved when the interpreter is run before the enclosing function + /// is finished elaborating. When this happens, we need to resolve any type variables + /// that were made within this block as well so that we can solve these traits. + function_context: Vec, /// The current module this elaborator is in. /// Initially empty, it is set whenever a new top-level item is resolved. @@ -166,6 +164,20 @@ pub struct Elaborator<'context> { unresolved_globals: BTreeMap, } +#[derive(Default)] +struct FunctionContext { + /// All type variables created in the current function. + /// This map is used to default any integer type variables at the end of + /// a function (before checking trait constraints) if a type wasn't already chosen. + type_variables: Vec, + + /// Trait constraints are collected during type checking until they are + /// verified at the end of a function. This is because constraints arise + /// on each variable, but it is only until function calls when the types + /// needed for the trait constraint may become known. + trait_constraints: Vec<(TraitConstraint, ExprId)>, +} + impl<'context> Elaborator<'context> { pub fn new(context: &'context mut Context, crate_id: CrateId) -> Self { Self { @@ -185,8 +197,7 @@ impl<'context> Elaborator<'context> { resolving_ids: BTreeSet::new(), trait_bounds: Vec::new(), current_function: None, - type_variables: Vec::new(), - trait_constraints: Vec::new(), + function_context: vec![FunctionContext::default()], current_trait_impl: None, comptime_scopes: vec![HashMap::default()], unresolved_globals: BTreeMap::new(), @@ -326,6 +337,7 @@ impl<'context> Elaborator<'context> { let func_meta = func_meta.clone(); self.trait_bounds = func_meta.trait_constraints.clone(); + self.function_context.push(FunctionContext::default()); // Introduce all numeric generics into scope for generic in &func_meta.all_generics { @@ -367,34 +379,11 @@ impl<'context> Elaborator<'context> { self.type_check_function_body(body_type, &func_meta, hir_func.as_expr()); } - // Default any type variables that still need defaulting. + // Default any type variables that still need defaulting and + // verify any remaining trait constraints arising from the function body. // This is done before trait impl search since leaving them bindable can lead to errors // when multiple impls are available. Instead we default first to choose the Field or u64 impl. - for typ in &self.type_variables { - if let Type::TypeVariable(variable, kind) = typ.follow_bindings() { - let msg = "TypeChecker should only track defaultable type vars"; - variable.bind(kind.default_type().expect(msg)); - } - } - - // Verify any remaining trait constraints arising from the function body - for (mut constraint, expr_id) in std::mem::take(&mut self.trait_constraints) { - let span = self.interner.expr_span(&expr_id); - - if matches!(&constraint.typ, Type::MutableReference(_)) { - let (_, dereferenced_typ) = - self.insert_auto_dereferences(expr_id, constraint.typ.clone()); - constraint.typ = dereferenced_typ; - } - - self.verify_trait_constraint( - &constraint.typ, - constraint.trait_id, - &constraint.trait_generics, - expr_id, - span, - ); - } + self.check_and_pop_function_context(); // Now remove all the `where` clause constraints we added for constraint in &func_meta.trait_constraints { @@ -417,12 +406,42 @@ impl<'context> Elaborator<'context> { meta.function_body = FunctionBody::Resolved; self.trait_bounds.clear(); - self.type_variables.clear(); self.interner.update_fn(id, hir_func); self.current_function = old_function; self.current_item = old_item; } + /// Defaults all type variables used in this function context then solves + /// all still-unsolved trait constraints in this context. + fn check_and_pop_function_context(&mut self) { + let context = self.function_context.pop().expect("Imbalanced function_context pushes"); + + for typ in context.type_variables { + if let Type::TypeVariable(variable, kind) = typ.follow_bindings() { + let msg = "TypeChecker should only track defaultable type vars"; + variable.bind(kind.default_type().expect(msg)); + } + } + + for (mut constraint, expr_id) in context.trait_constraints { + let span = self.interner.expr_span(&expr_id); + + if matches!(&constraint.typ, Type::MutableReference(_)) { + let (_, dereferenced_typ) = + self.insert_auto_dereferences(expr_id, constraint.typ.clone()); + constraint.typ = dereferenced_typ; + } + + self.verify_trait_constraint( + &constraint.typ, + constraint.trait_id, + &constraint.trait_generics, + expr_id, + span, + ); + } + } + /// This turns function parameters of the form: /// `fn foo(x: impl Bar)` /// @@ -1339,10 +1358,6 @@ impl<'context> Elaborator<'context> { self.elaborate_comptime_global(global_id); } - // Avoid defaulting the types of globals here since they may be used in any function. - // Otherwise we may prematurely default to a Field inside the next function if this - // global was unused there, even if it is consistently used as a u8 everywhere else. - self.type_variables.clear(); self.local_module = old_module; self.file = old_file; self.current_item = old_item; diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 58a7a57bb3f..8a2f305d8f6 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -396,6 +396,7 @@ impl<'context> Elaborator<'context> { let expr = self.resolve_variable(variable); let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone())); + self.interner.push_expr_location(id, span, self.file); let typ = self.type_check_variable(expr, id, generics); self.interner.push_expr_type(id, typ.clone()); @@ -524,7 +525,7 @@ impl<'context> Elaborator<'context> { for mut constraint in function.trait_constraints.clone() { constraint.apply_bindings(&bindings); - self.trait_constraints.push((constraint, expr_id)); + self.push_trait_constraint(constraint, expr_id); } } } @@ -541,7 +542,7 @@ impl<'context> Elaborator<'context> { // Currently only one impl can be selected per expr_id, so this // constraint needs to be pushed after any other constraints so // that monomorphization can resolve this trait method to the correct impl. - self.trait_constraints.push((constraint, expr_id)); + self.push_trait_constraint(constraint, expr_id); } } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 0d67c9ed3e3..e2d44919c5e 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -433,8 +433,15 @@ impl<'context> Elaborator<'context> { } fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) { + // We have to push a new FunctionContext so that we can resolve any constraints + // in this comptime block early before the function as a whole finishes elaborating. + // Otherwise the interpreter below may find expressions for which the underlying trait + // call is not yet solved for. + self.function_context.push(Default::default()); let span = statement.span; let (hir_statement, _typ) = self.elaborate_statement(statement); + self.check_and_pop_function_context(); + let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id); let value = interpreter.evaluate_statement(hir_statement); diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 63cab40f9d3..7f07e2a9538 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -16,7 +16,7 @@ use crate::{ errors::ResolverError, resolver::{verify_mutable_reference, SELF_TYPE_NAME, WILDCARD_TYPE}, }, - type_check::{Source, TypeCheckError}, + type_check::{NoMatchingImplFoundError, Source, TypeCheckError}, }, hir_def::{ expr::{ @@ -615,7 +615,7 @@ impl<'context> Elaborator<'context> { /// in self.type_variables to default it later. pub(super) fn polymorphic_integer_or_field(&mut self) -> Type { let typ = Type::polymorphic_integer_or_field(self.interner); - self.type_variables.push(typ.clone()); + self.push_type_variable(typ.clone()); typ } @@ -623,7 +623,7 @@ impl<'context> Elaborator<'context> { /// in self.type_variables to default it later. pub(super) fn polymorphic_integer(&mut self) -> Type { let typ = Type::polymorphic_integer(self.interner); - self.type_variables.push(typ.clone()); + self.push_type_variable(typ.clone()); typ } @@ -1410,26 +1410,10 @@ impl<'context> Elaborator<'context> { Err(erroring_constraints) => { if erroring_constraints.is_empty() { self.push_err(TypeCheckError::TypeAnnotationsNeeded { span }); - } else { - // Don't show any errors where try_get_trait returns None. - // This can happen if a trait is used that was never declared. - let constraints = erroring_constraints - .into_iter() - .map(|constraint| { - let r#trait = self.interner.try_get_trait(constraint.trait_id)?; - let mut name = r#trait.name.to_string(); - if !constraint.trait_generics.is_empty() { - let generics = - vecmap(&constraint.trait_generics, ToString::to_string); - name += &format!("<{}>", generics.join(", ")); - } - Some((constraint.typ, name)) - }) - .collect::>>(); - - if let Some(constraints) = constraints { - self.push_err(TypeCheckError::NoMatchingImplFound { constraints, span }); - } + } else if let Some(error) = + NoMatchingImplFoundError::new(self.interner, erroring_constraints, span) + { + self.push_err(TypeCheckError::NoMatchingImplFound(error)); } } } @@ -1557,4 +1541,20 @@ impl<'context> Elaborator<'context> { } } } + + /// Push a type variable into the current FunctionContext to be defaulted if needed + /// at the end of the earlier of either the current function or the current comptime scope. + fn push_type_variable(&mut self, typ: Type) { + let context = self.function_context.last_mut(); + let context = context.expect("The function_context stack should always be non-empty"); + context.type_variables.push(typ); + } + + /// Push a trait constraint into the current FunctionContext to be solved if needed + /// at the end of the earlier of either the current function or the current comptime scope. + pub fn push_trait_constraint(&mut self, constraint: TraitConstraint, expr_id: ExprId) { + let context = self.function_context.last_mut(); + let context = context.expect("The function_context stack should always be non-empty"); + context.trait_constraints.push((constraint, expr_id)); + } } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index d2c7acee2a3..9bc54cf0e04 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -1,7 +1,10 @@ use std::rc::Rc; use crate::{ - hir::def_collector::dc_crate::CompilationError, parser::ParserError, token::Tokens, Type, + hir::{def_collector::dc_crate::CompilationError, type_check::NoMatchingImplFoundError}, + parser::ParserError, + token::Tokens, + Type, }; use acvm::{acir::AcirField, FieldElement}; use fm::FileId; @@ -44,6 +47,8 @@ pub enum InterpreterError { FailedToParseMacro { error: ParserError, tokens: Rc, rule: &'static str, file: FileId }, UnsupportedTopLevelItemUnquote { item: String, location: Location }, NonComptimeFnCallInSameCrate { function: String, location: Location }, + NoImpl { location: Location }, + NoMatchingImplFound { error: NoMatchingImplFoundError, file: FileId }, Unimplemented { item: String, location: Location }, @@ -106,11 +111,15 @@ impl InterpreterError { | InterpreterError::UnsupportedTopLevelItemUnquote { location, .. } | InterpreterError::NonComptimeFnCallInSameCrate { location, .. } | InterpreterError::Unimplemented { location, .. } + | InterpreterError::NoImpl { location, .. } | InterpreterError::BreakNotInLoop { location, .. } | InterpreterError::ContinueNotInLoop { location, .. } => *location, InterpreterError::FailedToParseMacro { error, file, .. } => { Location::new(error.span(), *file) } + InterpreterError::NoMatchingImplFound { error, file } => { + Location::new(error.span, *file) + } InterpreterError::Break | InterpreterError::Continue => { panic!("Tried to get the location of Break/Continue error!") } @@ -324,6 +333,11 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let msg = "There is no loop to continue!".into(); CustomDiagnostic::simple_error(msg, String::new(), location.span) } + InterpreterError::NoImpl { location } => { + let msg = "No impl found due to prior type error".into(); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NoMatchingImplFound { error, .. } => error.into(), InterpreterError::Break => unreachable!("Uncaught InterpreterError::Break"), InterpreterError::Continue => unreachable!("Uncaught InterpreterError::Continue"), } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index d8f17e40f08..1b168e40043 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -8,7 +8,12 @@ use rustc_hash::FxHashMap as HashMap; use crate::ast::{BinaryOpKind, FunctionKind, IntegerBitSize, Signedness}; use crate::graph::CrateId; -use crate::monomorphization::{perform_instantiation_bindings, undo_instantiation_bindings}; +use crate::hir_def::expr::ImplKind; +use crate::macros_api::UnaryOp; +use crate::monomorphization::{ + perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method, + undo_instantiation_bindings, +}; use crate::token::Tokens; use crate::{ hir_def::{ @@ -66,8 +71,12 @@ impl<'a> Interpreter<'a> { instantiation_bindings: TypeBindings, location: Location, ) -> IResult { + let trait_method = self.interner.get_trait_method_id(function); + perform_instantiation_bindings(&instantiation_bindings); + let impl_bindings = perform_impl_bindings(self.interner, trait_method, function); let result = self.call_function_inner(function, arguments, location); + undo_instantiation_bindings(impl_bindings); undo_instantiation_bindings(instantiation_bindings); result } @@ -347,6 +356,13 @@ impl<'a> Interpreter<'a> { pub(super) fn evaluate_ident(&mut self, ident: HirIdent, id: ExprId) -> IResult { let definition = self.interner.definition(ident.id); + if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind { + let method_id = resolve_trait_method(self.interner, method, id)?; + let typ = self.interner.id_type(id).follow_bindings(); + let bindings = self.interner.get_instantiation_bindings(id).clone(); + return Ok(Value::Function(method_id, typ, Rc::new(bindings))); + } + match &definition.kind { DefinitionKind::Function(function_id) => { let typ = self.interner.id_type(id).follow_bindings(); @@ -556,8 +572,17 @@ impl<'a> Interpreter<'a> { fn evaluate_prefix(&mut self, prefix: HirPrefixExpression, id: ExprId) -> IResult { let rhs = self.evaluate(prefix.rhs)?; - match prefix.operator { - crate::ast::UnaryOp::Minus => match rhs { + self.evaluate_prefix_with_value(rhs, prefix.operator, id) + } + + fn evaluate_prefix_with_value( + &mut self, + rhs: Value, + operator: UnaryOp, + id: ExprId, + ) -> IResult { + match operator { + UnaryOp::Minus => match rhs { Value::Field(value) => Ok(Value::Field(FieldElement::zero() - value)), Value::I8(value) => Ok(Value::I8(-value)), Value::I16(value) => Ok(Value::I16(-value)), @@ -573,7 +598,7 @@ impl<'a> Interpreter<'a> { Err(InterpreterError::InvalidValueForUnary { value, location, operator }) } }, - crate::ast::UnaryOp::Not => match rhs { + UnaryOp::Not => match rhs { Value::Bool(value) => Ok(Value::Bool(!value)), Value::I8(value) => Ok(Value::I8(!value)), Value::I16(value) => Ok(Value::I16(!value)), @@ -588,8 +613,8 @@ impl<'a> Interpreter<'a> { Err(InterpreterError::InvalidValueForUnary { value, location, operator: "not" }) } }, - crate::ast::UnaryOp::MutableReference => Ok(Value::Pointer(Shared::new(rhs))), - crate::ast::UnaryOp::Dereference { implicitly_added: _ } => match rhs { + UnaryOp::MutableReference => Ok(Value::Pointer(Shared::new(rhs))), + UnaryOp::Dereference { implicitly_added: _ } => match rhs { Value::Pointer(element) => Ok(element.borrow().clone()), value => { let location = self.interner.expr_location(&id); @@ -603,13 +628,8 @@ impl<'a> Interpreter<'a> { let lhs = self.evaluate(infix.lhs)?; let rhs = self.evaluate(infix.rhs)?; - // TODO: Need to account for operator overloading - // See https://github.com/noir-lang/noir/issues/4925 if self.interner.get_selected_impl_for_expression(id).is_some() { - return Err(InterpreterError::Unimplemented { - item: "Operator overloading in the interpreter".to_string(), - location: infix.operator.location, - }); + return self.evaluate_overloaded_infix(infix, lhs, rhs, id); } use InterpreterError::InvalidValuesForBinary; @@ -854,6 +874,64 @@ impl<'a> Interpreter<'a> { } } + fn evaluate_overloaded_infix( + &mut self, + infix: HirInfixExpression, + lhs: Value, + rhs: Value, + id: ExprId, + ) -> IResult { + let method = infix.trait_method_id; + let operator = infix.operator.kind; + + let method_id = resolve_trait_method(self.interner, method, id)?; + let type_bindings = self.interner.get_instantiation_bindings(id).clone(); + + let lhs = (lhs, self.interner.expr_location(&infix.lhs)); + let rhs = (rhs, self.interner.expr_location(&infix.rhs)); + + let location = self.interner.expr_location(&id); + let value = self.call_function(method_id, vec![lhs, rhs], type_bindings, location)?; + + // Certain operators add additional operations after the trait call: + // - `!=`: Reverse the result of Eq + // - Comparator operators: Convert the returned `Ordering` to a boolean. + use BinaryOpKind::*; + match operator { + NotEqual => self.evaluate_prefix_with_value(value, UnaryOp::Not, id), + Less | LessEqual | Greater | GreaterEqual => self.evaluate_ordering(value, operator), + _ => Ok(value), + } + } + + /// Given the result of a `cmp` operation, convert it into the boolean result of the given operator. + /// - `<`: `ordering == Ordering::Less` + /// - `<=`: `ordering != Ordering::Greater` + /// - `>`: `ordering == Ordering::Greater` + /// - `<=`: `ordering != Ordering::Less` + fn evaluate_ordering(&self, ordering: Value, operator: BinaryOpKind) -> IResult { + let ordering = match ordering { + Value::Struct(fields, _) => match fields.into_iter().next().unwrap().1 { + Value::Field(ordering) => ordering, + _ => unreachable!("`cmp` should always return an Ordering value"), + }, + _ => unreachable!("`cmp` should always return an Ordering value"), + }; + + use BinaryOpKind::*; + let less_or_greater = if matches!(operator, Less | GreaterEqual) { + FieldElement::zero() // Ordering::Less + } else { + 2u128.into() // Ordering::Greater + }; + + if matches!(operator, Less | Greater) { + Ok(Value::Bool(ordering == less_or_greater)) + } else { + Ok(Value::Bool(ordering != less_or_greater)) + } + } + fn evaluate_index(&mut self, index: HirIndexExpression, id: ExprId) -> IResult { let array = self.evaluate(index.collection)?; let index = self.evaluate(index.index)?; diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index f18e8a9e843..9c6a39d1940 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -1,4 +1,5 @@ use acvm::FieldElement; +use iter_extended::vecmap; use noirc_errors::CustomDiagnostic as Diagnostic; use noirc_errors::Span; use thiserror::Error; @@ -6,7 +7,9 @@ use thiserror::Error; use crate::ast::{BinaryOpKind, FunctionReturnType, IntegerBitSize, Signedness}; use crate::hir::resolution::errors::ResolverError; use crate::hir_def::expr::HirBinaryOp; +use crate::hir_def::traits::TraitConstraint; use crate::hir_def::types::Type; +use crate::macros_api::NodeInterner; #[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum Source { @@ -114,7 +117,7 @@ pub enum TypeCheckError { parameter_index: usize, }, #[error("No matching impl found")] - NoMatchingImplFound { constraints: Vec<(Type, String)>, span: Span }, + NoMatchingImplFound(NoMatchingImplFoundError), #[error("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope")] UnneededTraitConstraint { trait_name: String, typ: Type, span: Span }, #[error( @@ -149,6 +152,12 @@ pub enum TypeCheckError { MacroReturningNonExpr { typ: Type, span: Span }, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NoMatchingImplFoundError { + constraints: Vec<(Type, String)>, + pub span: Span, +} + impl TypeCheckError { pub fn add_context(self, ctx: &'static str) -> Self { TypeCheckError::Context { err: Box::new(self), ctx } @@ -307,20 +316,7 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { let msg = format!("Unused expression result of type {expr_type}"); Diagnostic::simple_warning(msg, String::new(), *expr_span) } - TypeCheckError::NoMatchingImplFound { constraints, span } => { - assert!(!constraints.is_empty()); - let msg = format!("No matching impl found for `{}: {}`", constraints[0].0, constraints[0].1); - let mut diagnostic = Diagnostic::from_message(&msg); - - diagnostic.add_secondary(format!("No impl for `{}: {}`", constraints[0].0, constraints[0].1), *span); - - // These must be notes since secondaries are unordered - for (typ, trait_name) in &constraints[1..] { - diagnostic.add_note(format!("Required by `{typ}: {trait_name}`")); - } - - diagnostic - } + TypeCheckError::NoMatchingImplFound(error) => error.into(), TypeCheckError::UnneededTraitConstraint { trait_name, typ, span } => { let msg = format!("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope"); Diagnostic::simple_warning(msg, "Unnecessary trait constraint in where clause".into(), *span) @@ -350,7 +346,54 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { format!("Expected macro call to return a `Quoted` but found a(n) `{typ}`"), "Macro calls must return quoted values, otherwise there is no code to insert".into(), *span, - ), + ), + } + } +} + +impl<'a> From<&'a NoMatchingImplFoundError> for Diagnostic { + fn from(error: &'a NoMatchingImplFoundError) -> Self { + let constraints = &error.constraints; + let span = error.span; + + assert!(!constraints.is_empty()); + let msg = + format!("No matching impl found for `{}: {}`", constraints[0].0, constraints[0].1); + let mut diagnostic = Diagnostic::from_message(&msg); + + let secondary = format!("No impl for `{}: {}`", constraints[0].0, constraints[0].1); + diagnostic.add_secondary(secondary, span); + + // These must be notes since secondaries are unordered + for (typ, trait_name) in &constraints[1..] { + diagnostic.add_note(format!("Required by `{typ}: {trait_name}`")); } + + diagnostic + } +} + +impl NoMatchingImplFoundError { + pub fn new( + interner: &NodeInterner, + failing_constraints: Vec, + span: Span, + ) -> Option { + // Don't show any errors where try_get_trait returns None. + // This can happen if a trait is used that was never declared. + let constraints = failing_constraints + .into_iter() + .map(|constraint| { + let r#trait = interner.try_get_trait(constraint.trait_id)?; + let mut name = r#trait.name.to_string(); + if !constraint.trait_generics.is_empty() { + let generics = vecmap(&constraint.trait_generics, ToString::to_string); + name += &format!("<{}>", generics.join(", ")); + } + Some((constraint.typ, name)) + }) + .collect::>>()?; + + Some(Self { constraints, span }) } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 77861a6d8f8..1f3e9103cde 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -17,6 +17,7 @@ use crate::{ TypeBinding, TypeBindings, TypeVariableKind, }; +use super::NoMatchingImplFoundError; use super::{errors::TypeCheckError, TypeChecker}; impl<'interner> TypeChecker<'interner> { @@ -518,26 +519,10 @@ impl<'interner> TypeChecker<'interner> { Err(erroring_constraints) => { if erroring_constraints.is_empty() { self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span }); - } else { - // Don't show any errors where try_get_trait returns None. - // This can happen if a trait is used that was never declared. - let constraints = erroring_constraints - .into_iter() - .map(|constraint| { - let r#trait = self.interner.try_get_trait(constraint.trait_id)?; - let mut name = r#trait.name.to_string(); - if !constraint.trait_generics.is_empty() { - let generics = - vecmap(&constraint.trait_generics, ToString::to_string); - name += &format!("<{}>", generics.join(", ")); - } - Some((constraint.typ, name)) - }) - .collect::>>(); - - if let Some(constraints) = constraints { - self.errors.push(TypeCheckError::NoMatchingImplFound { constraints, span }); - } + } else if let Some(error) = + NoMatchingImplFoundError::new(self.interner, erroring_constraints, span) + { + self.errors.push(TypeCheckError::NoMatchingImplFound(error)); } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 1d3c7fcda9b..b8fd59e015b 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -11,7 +11,7 @@ mod errors; mod expr; mod stmt; -pub use errors::TypeCheckError; +pub use errors::{NoMatchingImplFoundError, TypeCheckError}; use noirc_errors::Span; use crate::{ diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index ebf0503963e..071ca95b114 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -9,6 +9,9 @@ //! The entry point to this pass is the `monomorphize` function which, starting from a given //! function, will monomorphize the entire reachable program. 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::{ debug::DebugInstrumenter, hir_def::{ @@ -125,7 +128,7 @@ pub fn monomorphize_debug( monomorphizer.locals.clear(); perform_instantiation_bindings(&bindings); - let impl_bindings = monomorphizer.perform_impl_bindings(trait_method, next_fn_id); + let impl_bindings = perform_impl_bindings(monomorphizer.interner, trait_method, next_fn_id); monomorphizer.function(next_fn_id, new_id)?; undo_instantiation_bindings(impl_bindings); undo_instantiation_bindings(bindings); @@ -467,23 +470,11 @@ impl<'interner> Monomorphizer<'interner> { if self.interner.get_selected_impl_for_expression(expr).is_some() { // If an impl was selected for this infix operator, replace it // with a method call to the appropriate trait impl method. - let lhs_type = self.interner.id_type(infix.lhs); - let args = vec![lhs_type.clone(), lhs_type]; - - // If this is a comparison operator, the result is a boolean but - // the actual method call returns an Ordering - use crate::ast::BinaryOpKind::*; - let ret = if matches!(operator, Less | LessEqual | Greater | GreaterEqual) { - self.interner.ordering_type() - } else { - self.interner.id_type(expr) - }; - - let env = Box::new(Type::Unit); - let function_type = Type::Function(args, Box::new(ret.clone()), env); + let (function_type, ret) = + self.interner.get_operator_type(infix.lhs, operator, expr); let method = infix.trait_method_id; - let func = self.resolve_trait_method_reference(expr, function_type, method)?; + let func = self.resolve_trait_method_expr(expr, function_type, method)?; self.create_operator_impl_call(func, lhs, infix.operator, rhs, ret, location)? } else { let lhs = Box::new(lhs); @@ -843,7 +834,7 @@ impl<'interner> Monomorphizer<'interner> { let typ = self.interner.id_type(expr_id); if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind { - return self.resolve_trait_method_reference(expr_id, typ, method); + return self.resolve_trait_method_expr(expr_id, typ, method); } let definition = self.interner.definition(ident.id); @@ -1046,53 +1037,14 @@ impl<'interner> Monomorphizer<'interner> { } } - fn resolve_trait_method_reference( + fn resolve_trait_method_expr( &mut self, expr_id: node_interner::ExprId, function_type: HirType, method: TraitMethodId, ) -> Result { - let trait_impl = self - .interner - .get_selected_impl_for_expression(expr_id) - .expect("ICE: missing trait impl - should be caught during type checking"); - - let func_id = match trait_impl { - node_interner::TraitImplKind::Normal(impl_id) => { - self.interner.get_trait_implementation(impl_id).borrow().methods - [method.method_index] - } - node_interner::TraitImplKind::Assumed { object_type, trait_generics } => { - match self.interner.lookup_trait_implementation( - &object_type, - method.trait_id, - &trait_generics, - ) { - Ok(TraitImplKind::Normal(impl_id)) => { - self.interner.get_trait_implementation(impl_id).borrow().methods - [method.method_index] - } - Ok(TraitImplKind::Assumed { .. }) => unreachable!( - "There should be no remaining Assumed impls during monomorphization" - ), - Err(constraints) => { - let failed_constraints = vecmap(constraints, |constraint| { - let id = constraint.trait_id; - let mut name = self.interner.get_trait(id).name.to_string(); - if !constraint.trait_generics.is_empty() { - let types = - vecmap(&constraint.trait_generics, |t| format!("{t:?}")); - name += &format!("<{}>", types.join(", ")); - } - format!(" {}: {name}", constraint.typ) - }) - .join("\n"); - - unreachable!("Failed to find trait impl during monomorphization. The failed constraint(s) are:\n{failed_constraints}") - } - } - } - }; + let func_id = resolve_trait_method(self.interner, method, expr_id) + .map_err(MonomorphizationError::InterpreterError)?; let func_id = match self.lookup_function(func_id, expr_id, &function_type, vec![], Some(method)) { @@ -1763,46 +1715,6 @@ impl<'interner> Monomorphizer<'interner> { Ok(result) } - - /// Call sites are instantiated against the trait method, but when an impl is later selected, - /// the corresponding method in the impl will have a different set of generics. `perform_impl_bindings` - /// is needed to apply the generics from the trait method to the impl method. Without this, - /// static method references to generic impls (e.g. `Eq::eq` for `[T; N]`) will fail to re-apply - /// the correct type bindings during monomorphization. - fn perform_impl_bindings( - &self, - trait_method: Option, - impl_method: node_interner::FuncId, - ) -> TypeBindings { - let mut bindings = TypeBindings::new(); - - if let Some(trait_method) = trait_method { - let the_trait = self.interner.get_trait(trait_method.trait_id); - - let trait_method_type = the_trait.methods[trait_method.method_index].typ.as_monotype(); - - // Make each NamedGeneric in this type bindable by replacing it with a TypeVariable - // with the same internal id and binding. - let (generics, impl_method_type) = - self.interner.function_meta(&impl_method).typ.unwrap_forall(); - - let replace_type_variable = |var: &TypeVariable| { - (var.id(), (var.clone(), Type::TypeVariable(var.clone(), TypeVariableKind::Normal))) - }; - - // Replace each NamedGeneric with a TypeVariable containing the same internal type variable - let type_bindings = generics.iter().map(replace_type_variable).collect(); - let impl_method_type = impl_method_type.force_substitute(&type_bindings); - - trait_method_type.try_unify(&impl_method_type, &mut bindings).unwrap_or_else(|_| { - unreachable!("Impl method type {} does not unify with trait method type {} during monomorphization", impl_method_type, trait_method_type) - }); - - perform_instantiation_bindings(&bindings); - } - - bindings - } } fn unwrap_tuple_type(typ: &HirType) -> Vec { @@ -1830,3 +1742,84 @@ pub fn undo_instantiation_bindings(bindings: TypeBindings) { var.unbind(id); } } + +/// Call sites are instantiated against the trait method, but when an impl is later selected, +/// the corresponding method in the impl will have a different set of generics. `perform_impl_bindings` +/// is needed to apply the generics from the trait method to the impl method. Without this, +/// static method references to generic impls (e.g. `Eq::eq` for `[T; N]`) will fail to re-apply +/// the correct type bindings during monomorphization. +pub fn perform_impl_bindings( + interner: &NodeInterner, + trait_method: Option, + impl_method: node_interner::FuncId, +) -> TypeBindings { + let mut bindings = TypeBindings::new(); + + if let Some(trait_method) = trait_method { + let the_trait = interner.get_trait(trait_method.trait_id); + + let trait_method_type = the_trait.methods[trait_method.method_index].typ.as_monotype(); + + // Make each NamedGeneric in this type bindable by replacing it with a TypeVariable + // with the same internal id and binding. + let (generics, impl_method_type) = interner.function_meta(&impl_method).typ.unwrap_forall(); + + let replace_type_variable = |var: &TypeVariable| { + (var.id(), (var.clone(), Type::TypeVariable(var.clone(), TypeVariableKind::Normal))) + }; + + // Replace each NamedGeneric with a TypeVariable containing the same internal type variable + let type_bindings = generics.iter().map(replace_type_variable).collect(); + let impl_method_type = impl_method_type.force_substitute(&type_bindings); + + trait_method_type.try_unify(&impl_method_type, &mut bindings).unwrap_or_else(|_| { + unreachable!("Impl method type {} does not unify with trait method type {} during monomorphization", impl_method_type, trait_method_type) + }); + + perform_instantiation_bindings(&bindings); + } + + bindings +} + +pub fn resolve_trait_method( + interner: &NodeInterner, + method: TraitMethodId, + expr_id: ExprId, +) -> Result { + let trait_impl = interner.get_selected_impl_for_expression(expr_id).ok_or_else(|| { + let location = interner.expr_location(&expr_id); + InterpreterError::NoImpl { location } + })?; + + let impl_id = match trait_impl { + TraitImplKind::Normal(impl_id) => impl_id, + TraitImplKind::Assumed { object_type, trait_generics } => { + match interner.lookup_trait_implementation( + &object_type, + method.trait_id, + &trait_generics, + ) { + 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); + 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 }); + } + } + } + } + }; + + Ok(interner.get_trait_implementation(impl_id).borrow().methods[method.method_index]) +} diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 2a4b4962e74..2cb18a71d7a 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1218,6 +1218,17 @@ impl NodeInterner { self.trait_implementations[&id].clone() } + /// If the given function belongs to a trait impl, return its trait method id. + /// Otherwise, return None. + pub fn get_trait_method_id(&self, function: FuncId) -> Option { + let impl_id = self.function_meta(&function).trait_impl?; + let trait_impl = self.get_trait_implementation(impl_id); + let trait_impl = trait_impl.borrow(); + + let method_index = trait_impl.methods.iter().position(|id| *id == function)?; + Some(TraitMethodId { trait_id: trait_impl.trait_id, method_index }) + } + /// Given a `ObjectType: TraitId` pair, try to find an existing impl that satisfies the /// constraint. If an impl cannot be found, this will return a vector of each constraint /// in the path to get to the failing constraint. Usually this is just the single failing @@ -1794,6 +1805,29 @@ impl NodeInterner { pub fn get_quoted_type(&self, id: QuotedTypeId) -> &Type { &self.quoted_types[id.0] } + + /// Returns the type of an operator (which is always a function), along with its return type. + pub fn get_operator_type( + &self, + lhs: ExprId, + operator: BinaryOpKind, + operator_expr: ExprId, + ) -> (Type, Type) { + let lhs_type = self.id_type(lhs); + let args = vec![lhs_type.clone(), lhs_type]; + + // If this is a comparison operator, the result is a boolean but + // the actual method call returns an Ordering + use crate::ast::BinaryOpKind::*; + let ret = if matches!(operator, Less | LessEqual | Greater | GreaterEqual) { + self.ordering_type() + } else { + self.id_type(operator_expr) + }; + + let env = Box::new(Type::Unit); + (Type::Function(args, Box::new(ret.clone()), env), ret) + } } impl Methods { diff --git a/test_programs/compile_success_empty/comptime_traits/Nargo.toml b/test_programs/compile_success_empty/comptime_traits/Nargo.toml new file mode 100644 index 00000000000..75df4dc5c20 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_traits/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_traits" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_success_empty/comptime_traits/src/main.nr b/test_programs/compile_success_empty/comptime_traits/src/main.nr new file mode 100644 index 00000000000..143c9cda274 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_traits/src/main.nr @@ -0,0 +1,15 @@ +fn main() { + comptime + { + // impl Eq for Field + assert(3 == 3); + + // impl Default for [T; N] where T: Default + // impl Default for Field + let array = Default::default(); + + // impl Eq for [T; N] where T: Eq + // impl Eq for Field + assert([1, 2] != array); + } +} diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index a6873910524..0fbdaaba0b4 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -61,12 +61,13 @@ const IGNORED_BRILLIG_TESTS: [&str; 11] = [ /// Certain features are only available in the elaborator. /// We skip these tests for non-elaborator code since they are not /// expected to work there. This can be removed once the old code is removed. -const IGNORED_NEW_FEATURE_TESTS: [&str; 5] = [ +const IGNORED_NEW_FEATURE_TESTS: [&str; 6] = [ "macros", "wildcard_type", "type_definition_annotation", "numeric_generics_explicit", "derive_impl", + "comptime_traits", ]; fn read_test_cases(