Skip to content

Commit

Permalink
chore!: removing implicit numeric generics (#5837)
Browse files Browse the repository at this point in the history 
# Description

## Problem\*

- Resolves #5156
- Resolves #5447

## Summary\*



## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] 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.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Maxim Vezenov <[email protected]>
Co-authored-by: jfecher <[email protected]>
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
@michaeljklein @vezenovm
@jfecher @TomAFrench
4 people authored Sep 17, 2024
1 parent 23ed74b commit eda9043
Show file tree
Hide file tree
Showing 54 changed files with 405 additions and 505 deletions.
6 changes: 3 additions & 3 deletions aztec_macros/src/utils/hir_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub fn signature_of_type(typ: &Type) -> String {
Type::FieldElement => "Field".to_owned(),
Type::Bool => "bool".to_owned(),
Type::Array(len, typ) => {
if let Type::Constant(len) = **len {
if let Type::Constant(len, _) = **len {
format!("[{};{len}]", signature_of_type(typ))
} else {
unimplemented!("Cannot generate signature for array with length type {:?}", typ)
Expand All @@ -90,7 +90,7 @@ pub fn signature_of_type(typ: &Type) -> String {
format!("({})", fields.join(","))
}
Type::String(len_typ) => {
if let Type::Constant(len) = **len_typ {
if let Type::Constant(len, _) = **len_typ {
format!("str<{len}>")
} else {
unimplemented!(
Expand Down Expand Up @@ -326,7 +326,7 @@ pub fn get_serialized_length(
let serialized_trait_impl = serialized_trait_impl_shared.borrow();

match serialized_trait_impl.trait_generics.first().unwrap() {
Type::Constant(value) => Ok(*value),
Type::Constant(value, _) => Ok(*value),
_ => Err(MacroError {
primary_message: format!("{} length for {} must be a constant", trait_name, typ),
secondary_message: None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/src/abi_gen.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
}
Type::Error
| Type::Unit
| Type::Constant(_)
| Type::Constant(..)
| Type::InfixExpr(..)
| Type::TraitAsType(..)
| Type::TypeVariable(_, _)
Expand Down
10 changes: 5 additions & 5 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
},
node_interner::{DefinitionKind, ExprId, FuncId, InternedStatementKind, TraitMethodId},
token::Tokens,
QuotedType, Shared, StructType, Type,
Kind, QuotedType, Shared, StructType, Type,
};

use super::{Elaborator, LambdaContext};
Expand Down Expand Up @@ -162,7 +162,7 @@ impl<'context> Elaborator<'context> {
(Lit(int), self.polymorphic_integer_or_field())
}
Literal::Str(str) | Literal::RawStr(str, _) => {
let len = Type::Constant(str.len() as u32);
let len = Type::Constant(str.len() as u32, Kind::u32());
(Lit(HirLiteral::Str(str)), Type::String(Box::new(len)))
}
Literal::FmtStr(str) => self.elaborate_fmt_string(str, span),
Expand Down Expand Up @@ -204,7 +204,7 @@ impl<'context> Elaborator<'context> {
elem_id
});

let length = Type::Constant(elements.len() as u32);
let length = Type::Constant(elements.len() as u32, Kind::u32());
(HirArrayLiteral::Standard(elements), first_elem_type, length)
}
ArrayLiteral::Repeated { repeated_element, length } => {
Expand All @@ -215,7 +215,7 @@ impl<'context> Elaborator<'context> {
UnresolvedTypeExpression::Constant(0, span)
});

let length = self.convert_expression_type(length);
let length = self.convert_expression_type(length, span);
let (repeated_element, elem_type) = self.elaborate_expression(*repeated_element);

let length_clone = length.clone();
Expand Down Expand Up @@ -268,7 +268,7 @@ impl<'context> Elaborator<'context> {
}
}

let len = Type::Constant(str.len() as u32);
let len = Type::Constant(str.len() as u32, Kind::u32());
let typ = Type::FmtString(Box::new(len), Box::new(Type::Tuple(capture_types)));
(HirExpression::Literal(HirLiteral::FmtStr(str, fmt_str_idents)), typ)
}
Expand Down
63 changes: 1 addition & 62 deletions compiler/noirc_frontend/src/elaborator/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
},
hir_def::{
expr::{HirCapturedVar, HirIdent},
function::{FunctionBody, Parameters},
function::FunctionBody,
traits::TraitConstraint,
types::{Generics, Kind, ResolvedGeneric},
},
Expand Down Expand Up @@ -415,7 +415,6 @@ impl<'context> Elaborator<'context> {
self.add_existing_variable_to_scope(name, parameter.clone(), true);
}

self.declare_numeric_generics(&func_meta.parameters, func_meta.return_type());
self.add_trait_constraints_to_scope(&func_meta);

let (hir_func, body_type) = match kind {
Expand Down Expand Up @@ -896,44 +895,6 @@ impl<'context> Elaborator<'context> {
}
}

// TODO(https://github.com/noir-lang/noir/issues/5156): Remove implicit numeric generics
fn declare_numeric_generics(&mut self, params: &Parameters, return_type: &Type) {
if self.generics.is_empty() {
return;
}

for (name_to_find, type_variable) in Self::find_numeric_generics(params, return_type) {
// Declare any generics to let users use numeric generics in scope.
// Don't issue a warning if these are unused
//
// We can fail to find the generic in self.generics if it is an implicit one created
// by the compiler. This can happen when, e.g. eliding array lengths using the slice
// syntax [T].
if let Some(ResolvedGeneric { name, span, kind, .. }) =
self.generics.iter_mut().find(|generic| generic.name.as_ref() == &name_to_find)
{
let scope = self.scopes.get_mut_scope();
let value = scope.find(&name_to_find);
if value.is_some() {
// With the addition of explicit numeric generics we do not want to introduce numeric generics in this manner
// However, this is going to be a big breaking change so for now we simply issue a warning while users have time
// to transition to the new syntax
// e.g. this code would break with a duplicate definition error:
// ```
// fn foo<let N: u8>(arr: [Field; N]) { }
// ```
continue;
}
*kind = Kind::Numeric(Box::new(Type::default_int_type()));
let ident = Ident::new(name.to_string(), *span);
let definition = DefinitionKind::GenericType(type_variable);
self.add_variable_decl_inner(ident.clone(), false, false, false, definition);

self.push_err(ResolverError::UseExplicitNumericGeneric { ident });
}
}
}

fn add_trait_constraints_to_scope(&mut self, func_meta: &FuncMeta) {
for constraint in &func_meta.trait_constraints {
let object = constraint.typ.clone();
Expand Down Expand Up @@ -1187,28 +1148,6 @@ impl<'context> Elaborator<'context> {
let fields_len = fields.len();
self.interner.update_struct(*type_id, |struct_def| {
struct_def.set_fields(fields);

// TODO(https://github.com/noir-lang/noir/issues/5156): Remove this with implicit numeric generics
// This is only necessary for resolving named types when implicit numeric generics are used.
let mut found_names = Vec::new();
struct_def.find_numeric_generics_in_fields(&mut found_names);
for generic in struct_def.generics.iter_mut() {
for found_generic in found_names.iter() {
if found_generic == generic.name.as_str() {
if matches!(generic.kind, Kind::Normal) {
let ident = Ident::new(generic.name.to_string(), generic.span);
self.errors.push((
CompilationError::ResolverError(
ResolverError::UseExplicitNumericGeneric { ident },
),
self.file,
));
generic.kind = Kind::Numeric(Box::new(Type::default_int_type()));
}
break;
}
}
}
});

for field_index in 0..fields_len {
Expand Down
123 changes: 62 additions & 61 deletions compiler/noirc_frontend/src/elaborator/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
},
hir::{
comptime::{Interpreter, Value},
def_collector::dc_crate::CompilationError,
def_map::ModuleDefId,
resolution::errors::ResolverError,
type_check::{
Expand Down Expand Up @@ -76,45 +77,22 @@ impl<'context> Elaborator<'context> {
FieldElement => Type::FieldElement,
Array(size, elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, kind));
let mut size = self.convert_expression_type(size);
// TODO(https://github.com/noir-lang/noir/issues/5156): Remove this once we only have explicit numeric generics
if let Type::NamedGeneric(type_var, name, _) = size {
size = Type::NamedGeneric(
type_var,
name,
Kind::Numeric(Box::new(Type::default_int_type())),
);
}
let size = self.convert_expression_type(size, span);
Type::Array(Box::new(size), elem)
}
Slice(elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, kind));
Type::Slice(elem)
}
Expression(expr) => self.convert_expression_type(expr),
Expression(expr) => self.convert_expression_type(expr, span),
Integer(sign, bits) => Type::Integer(sign, bits),
Bool => Type::Bool,
String(size) => {
let mut resolved_size = self.convert_expression_type(size);
// TODO(https://github.com/noir-lang/noir/issues/5156): Remove this once we only have explicit numeric generics
if let Type::NamedGeneric(type_var, name, _) = resolved_size {
resolved_size = Type::NamedGeneric(
type_var,
name,
Kind::Numeric(Box::new(Type::default_int_type())),
);
}
let resolved_size = self.convert_expression_type(size, span);
Type::String(Box::new(resolved_size))
}
FormatString(size, fields) => {
let mut resolved_size = self.convert_expression_type(size);
if let Type::NamedGeneric(type_var, name, _) = resolved_size {
resolved_size = Type::NamedGeneric(
type_var,
name,
Kind::Numeric(Box::new(Type::default_int_type())),
);
}
let resolved_size = self.convert_expression_type(size, span);
let fields = self.resolve_type_inner(*fields, kind);
Type::FmtString(Box::new(resolved_size), Box::new(fields))
}
Expand Down Expand Up @@ -191,26 +169,18 @@ impl<'context> Elaborator<'context> {
_ => (),
}

// Check that any types with a type kind match the expected type kind supplied to this function
// TODO(https://github.com/noir-lang/noir/issues/5156): make this named generic check more general with `*resolved_kind != kind`
// as implicit numeric generics still existing makes this check more challenging to enforce
// An example of a more general check that we should switch to:
// if resolved_type.kind() != kind.clone() {
// let expected_typ_err = CompilationError::TypeError(TypeCheckError::TypeKindMismatch {
// expected_kind: kind.to_string(),
// expr_kind: resolved_type.kind().to_string(),
// expr_span: span.expect("Type should have span"),
// });
// self.errors.push((expected_typ_err, self.file));
// return Type::Error;
// }
if let Type::NamedGeneric(_, name, resolved_kind) = &resolved_type {
if matches!(resolved_kind, Kind::Numeric { .. }) && matches!(kind, Kind::Normal) {
let expected_typ_err =
ResolverError::NumericGenericUsedForType { name: name.to_string(), span };
self.push_err(expected_typ_err);
return Type::Error;
}
if !kind.matches_opt(resolved_type.kind()) {
let expected_typ_err = CompilationError::TypeError(TypeCheckError::TypeKindMismatch {
expected_kind: kind.to_string(),
expr_kind: resolved_type
.kind()
.as_ref()
.map(Kind::to_string)
.unwrap_or("unknown".to_string()),
expr_span: span,
});
self.errors.push((expected_typ_err, self.file));
return Type::Error;
}

resolved_type
Expand Down Expand Up @@ -441,30 +411,61 @@ impl<'context> Elaborator<'context> {

let reference_location = Location::new(path.span(), self.file);
self.interner.add_global_reference(id, reference_location);
let kind = self
.interner
.get_global_let_statement(id)
.map(|let_statement| Kind::Numeric(Box::new(let_statement.r#type)))
.unwrap_or(Kind::u32());

Some(Type::Constant(self.eval_global_as_array_length(id, path)))
Some(Type::Constant(self.eval_global_as_array_length(id, path), kind))
}
_ => None,
}
}

pub(super) fn convert_expression_type(&mut self, length: UnresolvedTypeExpression) -> Type {
pub(super) fn convert_expression_type(
&mut self,
length: UnresolvedTypeExpression,
span: Span,
) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
self.lookup_generic_or_global_type(&path).unwrap_or_else(|| {
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
Type::Constant(0)
})
let resolved_length =
self.lookup_generic_or_global_type(&path).unwrap_or_else(|| {
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
Type::Constant(0, Kind::u32())
});

if let Type::NamedGeneric(ref _type_var, ref _name, ref kind) = resolved_length {
if !kind.is_numeric() {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: Kind::u32().to_string(),
expr_kind: kind.to_string(),
expr_span: span,
});
return Type::Error;
}
}
resolved_length
}
UnresolvedTypeExpression::Constant(int, _) => Type::Constant(int),
UnresolvedTypeExpression::Constant(int, _span) => Type::Constant(int, Kind::u32()),
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, span) => {
let lhs = self.convert_expression_type(*lhs);
let rhs = self.convert_expression_type(*rhs);
let (lhs_span, rhs_span) = (lhs.span(), rhs.span());
let lhs = self.convert_expression_type(*lhs, lhs_span);
let rhs = self.convert_expression_type(*rhs, rhs_span);

match (lhs, rhs) {
(Type::Constant(lhs), Type::Constant(rhs)) => {
(Type::Constant(lhs, lhs_kind), Type::Constant(rhs, rhs_kind)) => {
if lhs_kind != rhs_kind {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: lhs_kind.to_string(),
expr_kind: rhs_kind.to_string(),
expr_span: span,
});
return Type::Error;
}
if let Some(result) = op.function(lhs, rhs) {
Type::Constant(result)
Type::Constant(result, lhs_kind)
} else {
self.push_err(ResolverError::OverflowInType { lhs, op, rhs, span });
Type::Error
Expand Down Expand Up @@ -1686,7 +1687,7 @@ impl<'context> Elaborator<'context> {
| Type::Unit
| Type::Error
| Type::TypeVariable(_, _)
| Type::Constant(_)
| Type::Constant(..)
| Type::NamedGeneric(_, _, _)
| Type::Quoted(_)
| Type::Forall(_, _) => (),
Expand Down Expand Up @@ -1727,7 +1728,7 @@ impl<'context> Elaborator<'context> {
Type::Struct(struct_type, generics) => {
for (i, generic) in generics.iter().enumerate() {
if let Type::NamedGeneric(type_variable, name, _) = generic {
if struct_type.borrow().generic_is_numeric(i) {
if struct_type.borrow().generics[i].is_numeric() {
found.insert(name.to_string(), type_variable.clone());
}
} else {
Expand All @@ -1738,7 +1739,7 @@ impl<'context> Elaborator<'context> {
Type::Alias(alias, generics) => {
for (i, generic) in generics.iter().enumerate() {
if let Type::NamedGeneric(type_variable, name, _) = generic {
if alias.borrow().generic_is_numeric(i) {
if alias.borrow().generics[i].is_numeric() {
found.insert(name.to_string(), type_variable.clone());
}
} else {
Expand Down
Loading

0 comments on commit eda9043

Please sign in to comment.