Skip to content

Commit

Permalink
adding errors to evaluate_to_field_element, added more detail to seve…
Browse files Browse the repository at this point in the history 
…ral errors, added errors: DivisionByZero, ModuloOnFields, OverflowingConstant, FailingBinaryOp, NonConstantEvaluated, TypeCanonicalizationMismatch, cargo clippy/fmt
  • Loading branch information
@michaeljklein
michaeljklein committed Oct 29, 2024
1 parent 1922206 commit 9158d7b
Show file tree
Hide file tree
Showing 11 changed files with 363 additions and 147 deletions.
20 changes: 17 additions & 3 deletions compiler/noirc_driver/src/abi_gen.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use iter_extended::vecmap;
use noirc_abi::{
Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign,
};
use noirc_errors::Span;
use noirc_frontend::ast::{Signedness, Visibility};
use noirc_frontend::TypeBinding;
use noirc_frontend::{
Expand Down Expand Up @@ -39,10 +40,21 @@ pub(super) fn gen_abi(
Abi { parameters, return_type, error_types }
}

// Get the Span of the root crate's main function, or else a dummy span if that fails
fn get_main_function_span(context: &Context) -> Span {
if let Some(func_id) = context.get_main_function(context.root_crate_id()) {
context.function_meta(&func_id).location.span
} else {
// TODO dummy span
Span::default()
}
}

fn build_abi_error_type(context: &Context, typ: &Type) -> AbiErrorType {
match typ {
Type::FmtString(len, item_types) => {
let length = len.evaluate_to_u32().expect("Cannot evaluate fmt length");
let span = get_main_function_span(context);
let length = len.evaluate_to_u32(span).expect("Cannot evaluate fmt length");
let Type::Tuple(item_types) = item_types.as_ref() else {
unreachable!("FmtString items must be a tuple")
};
Expand All @@ -58,8 +70,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
match typ {
Type::FieldElement => AbiType::Field,
Type::Array(size, typ) => {
let span = get_main_function_span(context);
let length = size
.evaluate_to_u32()
.evaluate_to_u32(span)
.expect("Cannot have variable sized arrays as a parameter to main");
let typ = typ.as_ref();
AbiType::Array { length, typ: Box::new(abi_type_from_hir_type(context, typ)) }
Expand All @@ -86,8 +99,9 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
}
Type::Bool => AbiType::Boolean,
Type::String(size) => {
let span = get_main_function_span(context);
let size = size
.evaluate_to_u32()
.evaluate_to_u32(span)
.expect("Cannot have variable sized strings as a parameter to main");
AbiType::String { length: size }
}
Expand Down
19 changes: 14 additions & 5 deletions compiler/noirc_frontend/src/elaborator/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,11 +462,20 @@ impl<'context> Elaborator<'context> {
});
return Type::Error;
}
if let Some(result) = op.function(lhs, rhs, &lhs_kind) {
Type::Constant(result, lhs_kind)
} else {
self.push_err(ResolverError::OverflowInType { lhs, op, rhs, span });
Type::Error
match op.function(lhs, rhs, &lhs_kind, span) {
Ok(result) => Type::Constant(result, lhs_kind),
Err(err) => {
// TODO improve error (e.g. 'err' could be an underflow)
let err = Box::new(err);
self.push_err(ResolverError::OverflowInType {
lhs,
op,
rhs,
err,
span,
});
Type::Error
}
}
}
(lhs, rhs) => {
Expand Down
20 changes: 15 additions & 5 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use std::rc::Rc;

use crate::{
ast::TraitBound,
hir::{def_collector::dc_crate::CompilationError, type_check::NoMatchingImplFoundError},
hir::{
def_collector::dc_crate::CompilationError,
type_check::{NoMatchingImplFoundError, TypeCheckError},
},
parser::ParserError,
Type,
};
Expand Down Expand Up @@ -87,6 +90,7 @@ pub enum InterpreterError {
},
NonIntegerArrayLength {
typ: Type,
err: Option<Box<TypeCheckError>>,
location: Location,
},
NonNumericCasted {
Expand Down Expand Up @@ -228,6 +232,7 @@ pub enum InterpreterError {
},
UnknownArrayLength {
length: Type,
err: Box<TypeCheckError>,
location: Location,
},

Expand Down Expand Up @@ -435,9 +440,13 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
let secondary = "This is likely a bug".into();
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::NonIntegerArrayLength { typ, location } => {
InterpreterError::NonIntegerArrayLength { typ, err, location } => {
let msg = format!("Non-integer array length: `{typ}`");
let secondary = "Array lengths must be integers".into();
let secondary = if let Some(err) = err {
format!("Array lengths must be integers, but evaluating `{typ}` resulted in `{err}`")
} else {
"Array lengths must be integers".to_string()
};
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::NonNumericCasted { typ, location } => {
Expand Down Expand Up @@ -640,9 +649,10 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
let msg = format!("`{expression}` is not a valid function body");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
}
InterpreterError::UnknownArrayLength { length, location } => {
InterpreterError::UnknownArrayLength { length, err, location } => {
let msg = format!("Could not determine array length `{length}`");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
let secondary = format!("Evaluating the length failed with: `{err}`");
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
}
}
Expand Down
44 changes: 27 additions & 17 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,20 +585,25 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
}
DefinitionKind::NumericGeneric(type_variable, numeric_typ) => {
let value = match &*type_variable.borrow() {
TypeBinding::Unbound(_, _) => None,
TypeBinding::Unbound(_, _) => {
let typ = self.elaborator.interner.id_type(id);
let location = self.elaborator.interner.expr_location(&id);
Err(InterpreterError::NonIntegerArrayLength { typ, err: None, location })
}
TypeBinding::Bound(binding) => {
binding.evaluate_to_field_element(&Kind::Numeric(numeric_typ.clone()))
let span = self.elaborator.interner.id_location(id).span;
binding
.evaluate_to_field_element(&Kind::Numeric(numeric_typ.clone()), span)
.map_err(|err| {
let typ = Type::TypeVariable(type_variable.clone());
let err = Some(Box::new(err));
let location = self.elaborator.interner.expr_location(&id);
InterpreterError::NonIntegerArrayLength { typ, err, location }
})
}
};
}?;

if let Some(value) = value {
let typ = self.elaborator.interner.id_type(id);
self.evaluate_integer(value, false, id)
} else {
let location = self.elaborator.interner.expr_location(&id);
let typ = Type::TypeVariable(type_variable.clone());
Err(InterpreterError::NonIntegerArrayLength { typ, location })
}
self.evaluate_integer(value, false, id)
}
}
}
Expand Down Expand Up @@ -805,12 +810,17 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
HirArrayLiteral::Repeated { repeated_element, length } => {
let element = self.evaluate(repeated_element)?;

if let Some(length) = length.evaluate_to_u32() {
let elements = (0..length).map(|_| element.clone()).collect();
Ok(Value::Array(elements, typ))
} else {
let location = self.elaborator.interner.expr_location(&id);
Err(InterpreterError::NonIntegerArrayLength { typ: length, location })
let span = self.elaborator.interner.id_location(id).span;
match length.evaluate_to_u32(span) {
Ok(length) => {
let elements = (0..length).map(|_| element.clone()).collect();
Ok(Value::Array(elements, typ))
}
Err(err) => {
let err = Some(Box::new(err));
let location = self.elaborator.interner.expr_location(&id);
Err(InterpreterError::NonIntegerArrayLength { typ: length, err, location })
}
}
}
}
Expand Down
Loading

0 comments on commit 9158d7b

Please sign in to comment.