Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Always Check Arithmetic Generics at Monomorphization #6329

Merged
merged 18 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
87bae02
wip: add Type::Txm(typ, from, to) as type-level checked_transmute(fro…
michaeljklein Oct 22, 2024
bf06178
wip debugging stack overflow, remove 'x' parameter from Txm since it …
michaeljklein Oct 23, 2024
24c9f1d
fixed type-level checked_transmute stack overflow, only run simplific…
michaeljklein Oct 24, 2024
bca5a85
ensure simplifications are only run on the 'to' side of Txm, cleanup,…
michaeljklein Oct 24, 2024
202e43b
rename Txm -> CheckedCast, address several TODO's
michaeljklein Oct 25, 2024
4bbb827
Merge branch 'master' into michaeljklein/arith-generics-transmute-type
michaeljklein Oct 25, 2024
fb325ec
.
michaeljklein Oct 25, 2024
cf88a9e
Merge branch 'master' into michaeljklein/arith-generics-transmute-type
michaeljklein Oct 28, 2024
ffba14a
Merge branch 'master' into michaeljklein/arith-generics-transmute-type
michaeljklein Oct 29, 2024
1922206
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Oct 29, 2024
9158d7b
adding errors to evaluate_to_field_element, added more detail to seve…
michaeljklein Oct 29, 2024
8e897aa
CheckedCast(to, from) -> CheckedCast { from, to }
michaeljklein Oct 31, 2024
6491c4a
propagate error instead of unwrapping evaluate_to_field_element, add …
michaeljklein Oct 31, 2024
478c850
wip TODO's
michaeljklein Oct 31, 2024
9a75192
debugging failing arithmetic generics test, adding span to field_coun…
michaeljklein Nov 1, 2024
b81d4ae
Merge branch 'master' into michaeljklein/arith-generics-transmute-type
michaeljklein Nov 1, 2024
36198d3
rename ResolverError::OverflowInType -> BinaryOpError, checking remai…
michaeljklein Nov 1, 2024
aa4c4bc
Merge branch 'master' into michaeljklein/arith-generics-transmute-type
TomAFrench Nov 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/noirc_driver/src/abi_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
AbiType::Struct { fields, path }
}
Type::Alias(def, args) => abi_type_from_hir_type(context, &def.borrow().get_type(args)),
Type::CheckedCast(to, _from) => abi_type_from_hir_type(context, to),
Type::Tuple(fields) => {
let fields = vecmap(fields, |typ| abi_type_from_hir_type(context, typ));
AbiType::Tuple { fields }
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,10 @@ impl<'context> Elaborator<'context> {
Type::Alias(alias_type, generics) => {
self.mark_type_as_used(&alias_type.borrow().get_type(generics));
}
Type::CheckedCast(to, from) => {
self.mark_type_as_used(to);
self.mark_type_as_used(from);
}
Type::MutableReference(typ) => {
self.mark_type_as_used(typ);
}
Expand Down Expand Up @@ -1496,6 +1500,10 @@ impl<'context> Elaborator<'context> {
span,
);
}
Type::CheckedCast(to, from) => {
self.check_type_is_not_more_private_then_item(name, visibility, to, span);
self.check_type_is_not_more_private_then_item(name, visibility, from, span);
}
Type::Function(args, return_type, env, _) => {
for arg in args {
self.check_type_is_not_more_private_then_item(name, visibility, arg, span);
Expand Down
10 changes: 9 additions & 1 deletion compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,10 @@ impl<'context> Elaborator<'context> {
Type::Error
}
}
(lhs, rhs) => Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)).canonicalize(),
(lhs, rhs) => {
let infix = Type::InfixExpr(Box::new(lhs), op, Box::new(rhs));
Type::CheckedCast(Box::new(infix.clone()), Box::new(infix)).canonicalize()
}
}
}
UnresolvedTypeExpression::AsTraitPath(path) => {
Expand Down Expand Up @@ -1736,6 +1739,11 @@ impl<'context> Elaborator<'context> {
| Type::Quoted(_)
| Type::Forall(_, _) => (),

Type::CheckedCast(to, from) => {
Self::find_numeric_generics_in_type(to, found);
Self::find_numeric_generics_in_type(from, found);
}

Type::TraitAsType(_, _, args) => {
for arg in &args.ordered {
Self::find_numeric_generics_in_type(arg, found);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ impl Type {
let name = Path::from_single(name.as_ref().clone(), Span::default());
UnresolvedTypeData::Named(name, GenericTypeArgs::default(), true)
}
Type::CheckedCast(to, _from) => to.to_display_ast().typ,
Type::Function(args, ret, env, unconstrained) => {
let args = vecmap(args, |arg| arg.to_display_ast());
let ret = Box::new(ret.to_display_ast());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,7 @@ fn zeroed(return_type: Type) -> IResult<Value> {
Ok(Value::Struct(values, typ))
}
Type::Alias(alias, generics) => zeroed(alias.borrow().get_type(&generics)),
Type::CheckedCast(to, _from) => zeroed(*to),
typ @ Type::Function(..) => {
// Using Value::Zeroed here is probably safer than using FuncId::dummy_id() or similar
Ok(Value::Zeroed(typ))
Expand Down
82 changes: 79 additions & 3 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ pub enum Type {
/// like `fn foo<T, U>(...) {}`. Unlike TypeVariables, they cannot be bound over.
NamedGeneric(TypeVariable, Rc<String>),

/// A cast (to, from) that's checked at monomorphization.
///
/// Simplifications on arithmetic generics are only allowed on the LHS.
CheckedCast(Box<Type>, Box<Type>),

/// A functions with arguments, a return type and environment.
/// the environment should be `Unit` by default,
/// for closures it should contain a `Tuple` type with the captured
Expand Down Expand Up @@ -865,6 +870,7 @@ impl std::fmt::Display for Type {
TypeBinding::Unbound(_, _) if name.is_empty() => write!(f, "_"),
TypeBinding::Unbound(_, _) => write!(f, "{name}"),
},
Type::CheckedCast(to, _) => write!(f, "{to}"),
Type::Constant(x, _kind) => write!(f, "{x}"),
Type::Forall(typevars, typ) => {
let typevars = vecmap(typevars, |var| var.id().to_string());
Expand Down Expand Up @@ -1066,6 +1072,7 @@ impl Type {
| Type::TypeVariable(..)
| Type::TraitAsType(..)
| Type::NamedGeneric(..)
| Type::CheckedCast(..)
| Type::Forall(..)
| Type::Constant(..)
| Type::Quoted(..)
Expand Down Expand Up @@ -1108,6 +1115,10 @@ impl Type {
Type::NamedGeneric(_, _) => {
named_generic_is_numeric(self, found_names);
}
Type::CheckedCast(to, from) => {
to.find_numeric_type_vars(found_names);
from.find_numeric_type_vars(found_names);
}

Type::TraitAsType(_, _, args) => {
for arg in args.ordered.iter() {
Expand Down Expand Up @@ -1192,6 +1203,8 @@ impl Type {
| Type::InfixExpr(_, _, _)
| Type::TraitAsType(..) => false,

Type::CheckedCast(to, _) => to.is_valid_for_program_input(),

Type::Alias(alias, generics) => {
let alias = alias.borrow();
alias.get_type(generics).is_valid_for_program_input()
Expand Down Expand Up @@ -1241,6 +1254,8 @@ impl Type {
| Type::Quoted(_)
| Type::TraitAsType(..) => false,

Type::CheckedCast(to, _) => to.is_valid_non_inlined_function_input(),

Type::Alias(alias, generics) => {
let alias = alias.borrow();
alias.get_type(generics).is_valid_non_inlined_function_input()
Expand Down Expand Up @@ -1282,6 +1297,8 @@ impl Type {
}
}

Type::CheckedCast(to, _) => to.is_valid_for_unconstrained_boundary(),

// Quoted objects only exist at compile-time where the only execution
// environment is the interpreter. In this environment, they are valid.
Type::Quoted(_) => true,
Expand Down Expand Up @@ -1314,6 +1331,7 @@ impl Type {
pub fn generic_count(&self) -> usize {
match self {
Type::Forall(generics, _) => generics.len(),
Type::CheckedCast(to, _) => to.generic_count(),
Type::TypeVariable(type_variable) | Type::NamedGeneric(type_variable, _) => {
match &*type_variable.borrow() {
TypeBinding::Bound(binding) => binding.generic_count(),
Expand Down Expand Up @@ -1353,6 +1371,7 @@ impl Type {

pub(crate) fn kind(&self) -> Kind {
match self {
Type::CheckedCast(to, _) => to.kind(),
Type::NamedGeneric(var, _) => var.kind(),
Type::Constant(_, kind) => kind.clone(),
Type::TypeVariable(var) => match &*var.borrow() {
Expand Down Expand Up @@ -1407,6 +1426,7 @@ impl Type {
let fields = struct_type.get_fields(args);
fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count())
}
Type::CheckedCast(to, _) => to.field_count(),
Type::Alias(def, generics) => def.borrow().get_type(generics).field_count(),
Type::Tuple(fields) => {
fields.iter().fold(0, |acc, field_typ| acc + field_typ.field_count())
Expand Down Expand Up @@ -1583,6 +1603,7 @@ impl Type {
match self {
Type::TypeVariable(var) => Some((var.1.clone(), var.kind())),
Type::NamedGeneric(var, _) => Some((var.1.clone(), var.kind())),
Type::CheckedCast(to, _from) => to.get_inner_type_variable(),
_ => None,
}
}
Expand Down Expand Up @@ -1697,6 +1718,10 @@ impl Type {
}
}

(CheckedCast(to, _from), other) | (other, CheckedCast(to, _from)) => {
to.try_unify(other, bindings)
}

(NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _))
if !binding.borrow().is_unbound() =>
{
Expand Down Expand Up @@ -1935,6 +1960,16 @@ impl Type {
// TODO(https://github.com/noir-lang/noir/issues/6260): remove
// the unifies checks once all kinds checks are implemented?
pub(crate) fn evaluate_to_field_element(&self, kind: &Kind) -> Option<acvm::FieldElement> {
let run_simplifications = true;
self.evaluate_to_field_element_helper(kind, run_simplifications)
}

/// evaluate_to_field_element with optional generic arithmetic simplifications
pub(crate) fn evaluate_to_field_element_helper(
&self,
kind: &Kind,
run_simplifications: bool,
) -> Option<acvm::FieldElement> {
if let Some((binding, binding_kind)) = self.get_inner_type_variable() {
if let TypeBinding::Bound(binding) = &*binding.borrow() {
if kind.unifies(&binding_kind) {
Expand All @@ -1943,7 +1978,8 @@ impl Type {
}
}

match self.canonicalize() {
let could_be_checked_cast = false;
match self.canonicalize_helper(could_be_checked_cast, run_simplifications) {
Type::Constant(x, constant_kind) => {
if kind.unifies(&constant_kind) {
kind.ensure_value_fits(x)
Expand All @@ -1954,13 +1990,33 @@ impl Type {
Type::InfixExpr(lhs, op, rhs) => {
let infix_kind = lhs.infix_kind(&rhs);
if kind.unifies(&infix_kind) {
let lhs_value = lhs.evaluate_to_field_element(&infix_kind)?;
let rhs_value = rhs.evaluate_to_field_element(&infix_kind)?;
let lhs_value =
lhs.evaluate_to_field_element_helper(&infix_kind, run_simplifications)?;
let rhs_value =
rhs.evaluate_to_field_element_helper(&infix_kind, run_simplifications)?;
op.function(lhs_value, rhs_value, &infix_kind)
} else {
None
}
}
Type::CheckedCast(to, from) => {
let to_value = to.evaluate_to_field_element(kind)?;

// if both 'to' and 'from' evaluate to a constant,
// return None unless they match
let skip_simplifications = false;
if let Some(from_value) =
from.evaluate_to_field_element_helper(kind, skip_simplifications)
{
if to_value == from_value {
Some(to_value)
} else {
None
}
} else {
Some(to_value)
}
}
_ => None,
}
}
Expand Down Expand Up @@ -2183,6 +2239,11 @@ impl Type {
let fields = fields.substitute_helper(type_bindings, substitute_bound_typevars);
Type::FmtString(Box::new(size), Box::new(fields))
}
Type::CheckedCast(to, from) => {
let to = to.substitute_helper(type_bindings, substitute_bound_typevars);
let from = from.substitute_helper(type_bindings, substitute_bound_typevars);
Type::CheckedCast(Box::new(to), Box::new(from))
}
Type::NamedGeneric(binding, _) | Type::TypeVariable(binding) => {
substitute_binding(binding)
}
Expand Down Expand Up @@ -2272,6 +2333,7 @@ impl Type {
|| args.named.iter().any(|arg| arg.typ.occurs(target_id))
}
Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)),
Type::CheckedCast(to, from) => to.occurs(target_id) || from.occurs(target_id),
Type::NamedGeneric(type_var, _) | Type::TypeVariable(type_var) => {
match &*type_var.borrow() {
TypeBinding::Bound(binding) => {
Expand Down Expand Up @@ -2330,6 +2392,11 @@ impl Type {
def.borrow().get_type(args).follow_bindings()
}
Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())),
CheckedCast(to, from) => {
let to = Box::new(to.follow_bindings());
let from = Box::new(from.follow_bindings());
CheckedCast(to, from)
}
TypeVariable(var) | NamedGeneric(var, _) => {
if let TypeBinding::Bound(typ) = &*var.borrow() {
return typ.follow_bindings();
Expand Down Expand Up @@ -2426,6 +2493,10 @@ impl Type {
generic.typ.replace_named_generics_with_type_variables();
}
}
Type::CheckedCast(to, from) => {
to.replace_named_generics_with_type_variables();
from.replace_named_generics_with_type_variables();
}
Type::NamedGeneric(var, _) => {
let type_binding = var.borrow();
if let TypeBinding::Bound(binding) = &*type_binding {
Expand Down Expand Up @@ -2483,6 +2554,7 @@ impl Type {
}
}
Type::Alias(alias, args) => alias.borrow().get_type(args).integral_maximum_size(),
Type::CheckedCast(to, _) => to.integral_maximum_size(),
Type::NamedGeneric(binding, _name) => match &*binding.borrow() {
TypeBinding::Bound(typ) => typ.integral_maximum_size(),
TypeBinding::Unbound(_, kind) => kind.integral_maximum_size(),
Expand Down Expand Up @@ -2649,6 +2721,7 @@ impl From<&Type> for PrintableType {
Type::Alias(alias, args) => alias.borrow().get_type(args).into(),
Type::TraitAsType(..) => unreachable!(),
Type::Tuple(types) => PrintableType::Tuple { types: vecmap(types, |typ| typ.into()) },
Type::CheckedCast(to, _) => (*to.clone()).into(),
Type::NamedGeneric(..) => unreachable!(),
Type::Forall(..) => unreachable!(),
Type::Function(arguments, return_type, env, unconstrained) => PrintableType::Function {
Expand Down Expand Up @@ -2723,6 +2796,7 @@ impl std::fmt::Debug for Type {
}
Type::Unit => write!(f, "()"),
Type::Error => write!(f, "error"),
Type::CheckedCast(to, _) => write!(f, "{:?}", to),
Type::NamedGeneric(binding, name) => match binding.kind() {
Kind::Any | Kind::Normal | Kind::Integer | Kind::IntegerOrField => {
write!(f, "{}{:?}", name, binding)
Expand Down Expand Up @@ -2837,6 +2911,7 @@ impl std::hash::Hash for Type {
vars.hash(state);
typ.hash(state);
}
Type::CheckedCast(to, _from) => to.hash(state),
Type::Constant(value, _) => value.hash(state),
Type::Quoted(typ) => typ.hash(state),
Type::InfixExpr(lhs, op, rhs) => {
Expand Down Expand Up @@ -2903,6 +2978,7 @@ impl PartialEq for Type {
(Forall(lhs_vars, lhs_type), Forall(rhs_vars, rhs_type)) => {
lhs_vars == rhs_vars && lhs_type == rhs_type
}
(CheckedCast(typ, _), other) | (other, CheckedCast(typ, _)) => **typ == *other,
(Constant(lhs, lhs_kind), Constant(rhs, rhs_kind)) => {
lhs == rhs && lhs_kind == rhs_kind
}
Expand Down
Loading
Loading