From 7dd2c5ea7cf5f2e63ad41d691112062a5a546183 Mon Sep 17 00:00:00 2001 From: jfecher Date: Wed, 24 Apr 2024 15:00:49 -0500 Subject: [PATCH] chore: Add error conversion from `InterpreterError` (#4896) # Description ## Problem\* Resolves comment: https://github.com/noir-lang/noir/pull/4884#discussion_r1576646720 ## Summary\* Converts from `InterpreterError` to `CustomDiagnostic` and removes the previous `.unwrap()` placeholder so that interpreter errors are now properly displayed. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Maxim Vezenov --- .../noirc_frontend/src/hir/comptime/errors.rs | 226 +++++++++++++++++- .../src/hir/comptime/interpreter.rs | 9 +- .../noirc_frontend/src/hir/comptime/mod.rs | 1 + .../src/hir/def_collector/dc_crate.rs | 14 +- 4 files changed, 235 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 67d9a006b22..8de444252ab 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -1,11 +1,11 @@ -use crate::Type; +use crate::{hir::def_collector::dc_crate::CompilationError, Type}; use acvm::FieldElement; -use noirc_errors::Location; +use noirc_errors::{CustomDiagnostic, Location}; use super::value::Value; /// The possible errors that can halt the interpreter. -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum InterpreterError { ArgumentCountMismatch { expected: usize, actual: usize, call_location: Location }, TypeMismatch { expected: Type, value: Value, location: Location }, @@ -16,7 +16,7 @@ pub enum InterpreterError { NonBoolUsedInIf { value: Value, location: Location }, NonBoolUsedInConstrain { value: Value, location: Location }, FailingConstraint { message: Option, location: Location }, - NoMethodFound { object: Value, typ: Type, location: Location }, + NoMethodFound { name: String, typ: Type, location: Location }, NonIntegerUsedInLoop { value: Value, location: Location }, NonPointerDereferenced { value: Value, location: Location }, NonTupleOrStructInMemberAccess { value: Value, location: Location }, @@ -51,3 +51,221 @@ pub enum InterpreterError { #[allow(unused)] pub(super) type IResult = std::result::Result; + +impl InterpreterError { + pub fn into_compilation_error_pair(self) -> (CompilationError, fm::FileId) { + let location = self.get_location(); + (CompilationError::InterpreterError(self), location.file) + } + + pub fn get_location(&self) -> Location { + match self { + InterpreterError::ArgumentCountMismatch { call_location: location, .. } + | InterpreterError::TypeMismatch { location, .. } + | InterpreterError::NonComptimeVarReferenced { location, .. } + | InterpreterError::IntegerOutOfRangeForType { location, .. } + | InterpreterError::ErrorNodeEncountered { location, .. } + | InterpreterError::NonFunctionCalled { location, .. } + | InterpreterError::NonBoolUsedInIf { location, .. } + | InterpreterError::NonBoolUsedInConstrain { location, .. } + | InterpreterError::FailingConstraint { location, .. } + | InterpreterError::NoMethodFound { location, .. } + | InterpreterError::NonIntegerUsedInLoop { location, .. } + | InterpreterError::NonPointerDereferenced { location, .. } + | InterpreterError::NonTupleOrStructInMemberAccess { location, .. } + | InterpreterError::NonArrayIndexed { location, .. } + | InterpreterError::NonIntegerUsedAsIndex { location, .. } + | InterpreterError::NonIntegerIntegerLiteral { location, .. } + | InterpreterError::NonIntegerArrayLength { location, .. } + | InterpreterError::NonNumericCasted { location, .. } + | InterpreterError::IndexOutOfBounds { location, .. } + | InterpreterError::ExpectedStructToHaveField { location, .. } + | InterpreterError::TypeUnsupported { location, .. } + | InterpreterError::InvalidValueForUnary { location, .. } + | InterpreterError::InvalidValuesForBinary { location, .. } + | InterpreterError::CastToNonNumericType { location, .. } + | InterpreterError::QuoteInRuntimeCode { location, .. } + | InterpreterError::NonStructInConstructor { location, .. } + | InterpreterError::CannotInlineMacro { location, .. } + | InterpreterError::UnquoteFoundDuringEvaluation { location, .. } + | InterpreterError::Unimplemented { location, .. } + | InterpreterError::BreakNotInLoop { location, .. } + | InterpreterError::ContinueNotInLoop { location, .. } => *location, + InterpreterError::Break | InterpreterError::Continue => { + panic!("Tried to get the location of Break/Continue error!") + } + } + } +} + +impl From for CustomDiagnostic { + fn from(error: InterpreterError) -> Self { + match error { + InterpreterError::ArgumentCountMismatch { expected, actual, call_location } => { + let only = if expected > actual { "only " } else { "" }; + let plural = if expected == 1 { "" } else { "s" }; + let was_were = if actual == 1 { "was" } else { "were" }; + let msg = format!( + "Expected {expected} argument{plural}, but {only}{actual} {was_were} provided" + ); + + let few_many = if actual < expected { "few" } else { "many" }; + let secondary = format!("Too {few_many} arguments"); + CustomDiagnostic::simple_error(msg, secondary, call_location.span) + } + InterpreterError::TypeMismatch { expected, value, location } => { + let typ = value.get_type(); + let msg = format!("Expected `{expected}` but a value of type `{typ}` was given"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NonComptimeVarReferenced { name, location } => { + let msg = format!("Non-comptime variable `{name}` referenced in comptime code"); + let secondary = "Non-comptime variables can't be used in comptime code".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::IntegerOutOfRangeForType { value, typ, location } => { + let int = match value.try_into_u128() { + Some(int) => int.to_string(), + None => value.to_string(), + }; + let msg = format!("{int} is outside the range of the {typ} type"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::ErrorNodeEncountered { location } => { + let msg = "Internal Compiler Error: Error node encountered".to_string(); + let secondary = "This is a bug, please report this if found!".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonFunctionCalled { value, location } => { + let msg = "Only functions may be called".to_string(); + let secondary = format!("Expression has type {}", value.get_type()); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonBoolUsedInIf { value, location } => { + let msg = format!("Expected a `bool` but found `{}`", value.get_type()); + let secondary = "If conditions must be a boolean value".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonBoolUsedInConstrain { value, location } => { + let msg = format!("Expected a `bool` but found `{}`", value.get_type()); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::FailingConstraint { message, location } => { + let (primary, secondary) = match message { + Some(msg) => (format!("{msg:?}"), "Assertion failed".into()), + None => ("Assertion failed".into(), String::new()), + }; + CustomDiagnostic::simple_error(primary, secondary, location.span) + } + InterpreterError::NoMethodFound { name, typ, location } => { + let msg = format!("No method named `{name}` found for type `{typ}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NonIntegerUsedInLoop { value, location } => { + let typ = value.get_type(); + let msg = format!("Non-integer type `{typ}` used in for loop"); + let secondary = if matches!(typ.as_ref(), &Type::FieldElement) { + "`field` is not an integer type, try `u64` instead".to_string() + } else { + String::new() + }; + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonPointerDereferenced { value, location } => { + let typ = value.get_type(); + let msg = format!("Only references may be dereferenced, but found `{typ}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NonTupleOrStructInMemberAccess { value, location } => { + let msg = format!("The type `{}` has no fields to access", value.get_type()); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NonArrayIndexed { value, location } => { + let msg = format!("Expected an array or slice but found a(n) {}", value.get_type()); + let secondary = "Only arrays or slices may be indexed".into(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonIntegerUsedAsIndex { value, location } => { + let msg = format!("Expected an integer but found a(n) {}", value.get_type()); + let secondary = + "Only integers may be indexed. Note that this excludes `field`s".into(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonIntegerIntegerLiteral { typ, location } => { + let msg = format!("This integer literal somehow has the type `{typ}`"); + let secondary = "This is likely a bug".into(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonIntegerArrayLength { typ, location } => { + let msg = format!("Non-integer array length: `{typ}`"); + let secondary = "Array lengths must be integers".into(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::NonNumericCasted { value, location } => { + let msg = "Only numeric types may be casted".into(); + let secondary = format!("`{}` is non-numeric", value.get_type()); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::IndexOutOfBounds { index, length, location } => { + let msg = format!("{index} is out of bounds for the array of length {length}"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::ExpectedStructToHaveField { value, field_name, location } => { + let typ = value.get_type(); + let msg = format!("The type `{typ}` has no field named `{field_name}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::TypeUnsupported { typ, location } => { + let msg = + format!("The type `{typ}` is currently unsupported in comptime expressions"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::InvalidValueForUnary { value, operator, location } => { + let msg = format!("`{}` cannot be used with unary {operator}", value.get_type()); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::InvalidValuesForBinary { lhs, rhs, operator, location } => { + let lhs = lhs.get_type(); + let rhs = rhs.get_type(); + let msg = format!("No implementation for `{lhs}` {operator} `{rhs}`",); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::CastToNonNumericType { typ, location } => { + let msg = format!("Cannot cast to non-numeric type `{typ}`"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::QuoteInRuntimeCode { location } => { + let msg = "`quote` may only be used in comptime code".into(); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::NonStructInConstructor { typ, location } => { + let msg = format!("`{typ}` is not a struct type"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::CannotInlineMacro { value, location } => { + let msg = "Cannot inline value into runtime code if it contains references".into(); + let secondary = format!("Cannot inline value {value:?}"); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::UnquoteFoundDuringEvaluation { location } => { + let msg = "Unquote found during comptime evaluation".into(); + let secondary = "This is a bug".into(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } + InterpreterError::Unimplemented { item, location } => { + let msg = format!("{item} is currently unimplemented"); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::BreakNotInLoop { location } => { + let msg = "There is no loop to break out of!".into(); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + InterpreterError::ContinueNotInLoop { location } => { + let msg = "There is no loop to continue!".into(); + CustomDiagnostic::simple_error(msg, String::new(), location.span) + } + 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 c01c985a40c..4bdd4eaec9a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -506,11 +506,8 @@ impl<'a> Interpreter<'a> { Value::U64(value) => Ok(Value::U64(0 - value)), value => { let location = self.interner.expr_location(&id); - Err(InterpreterError::InvalidValueForUnary { - value, - location, - operator: "minus", - }) + let operator = "minus"; + Err(InterpreterError::InvalidValueForUnary { value, location, operator }) } }, crate::ast::UnaryOp::Not => match rhs { @@ -880,7 +877,7 @@ impl<'a> Interpreter<'a> { if let Some(method) = method { self.call_function(method, arguments, location) } else { - Err(InterpreterError::NoMethodFound { object, typ, location }) + Err(InterpreterError::NoMethodFound { name: method_name.clone(), typ, location }) } } diff --git a/compiler/noirc_frontend/src/hir/comptime/mod.rs b/compiler/noirc_frontend/src/hir/comptime/mod.rs index 26e05d675b3..148fa56b4cb 100644 --- a/compiler/noirc_frontend/src/hir/comptime/mod.rs +++ b/compiler/noirc_frontend/src/hir/comptime/mod.rs @@ -5,4 +5,5 @@ mod scan; mod tests; mod value; +pub use errors::InterpreterError; pub use interpreter::Interpreter; 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 7805f36cdb2..d8839b33ff4 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -1,7 +1,7 @@ use super::dc_mod::collect_defs; use super::errors::{DefCollectorErrorKind, DuplicateType}; use crate::graph::CrateId; -use crate::hir::comptime::Interpreter; +use crate::hir::comptime::{Interpreter, InterpreterError}; use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId}; use crate::hir::resolution::errors::ResolverError; @@ -155,6 +155,7 @@ pub enum CompilationError { DefinitionError(DefCollectorErrorKind), ResolverError(ResolverError), TypeError(TypeCheckError), + InterpreterError(InterpreterError), } impl From for CustomDiagnostic { @@ -164,6 +165,7 @@ impl From for CustomDiagnostic { CompilationError::DefinitionError(error) => error.into(), CompilationError::ResolverError(error) => error.into(), CompilationError::TypeError(error) => error.into(), + CompilationError::InterpreterError(error) => error.into(), } } } @@ -500,13 +502,15 @@ impl ResolvedModule { } /// Evaluate all `comptime` expressions in this module - fn evaluate_comptime(&self, interner: &mut NodeInterner) { + fn evaluate_comptime(&mut self, interner: &mut NodeInterner) { let mut interpreter = Interpreter::new(interner); for (_file, function) in &self.functions { - // .unwrap() is temporary here until we can convert - // from InterpreterError to (CompilationError, FileId) - interpreter.scan_function(*function).unwrap(); + // The file returned by the error may be different than the file the + // function is in so only use the error's file id. + if let Err(error) = interpreter.scan_function(*function) { + self.errors.push(error.into_compilation_error_pair()); + } } }