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

chore!: removing implicit numeric generics #5837

Merged
merged 64 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
19cf0c3
chore: wip removing implicit numeric generics
michaeljklein Aug 27, 2024
136ef8d
wip removing support for implicit generics, updating tests
michaeljklein Aug 28, 2024
2fb72f0
updated tests to use explicit numeric generics, added tests to preven…
michaeljklein Aug 28, 2024
0ec783c
removing TODO's and wip comments, removing implicit numeric generics …
michaeljklein Aug 28, 2024
bdb7729
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Aug 28, 2024
776d194
debug noirc_frontend tests
michaeljklein Aug 28, 2024
d35baf1
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Aug 28, 2024
b331952
Update compiler/noirc_frontend/src/elaborator/mod.rs
michaeljklein Aug 29, 2024
484694e
Update compiler/noirc_frontend/src/elaborator/mod.rs
michaeljklein Aug 29, 2024
f339e56
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Aug 29, 2024
8d9ae0b
Update compiler/noirc_frontend/src/elaborator/types.rs
michaeljklein Aug 29, 2024
a1538e5
remove generic_is_numeric methods
michaeljklein Aug 29, 2024
7011bca
move generic kind check to convert_expression_type, update test for a…
michaeljklein Aug 29, 2024
197f037
update array/slice/generics docs with explicit numeric generics, move…
michaeljklein Aug 29, 2024
93e33b8
use TypeKindMismatch instead of UseExplicitNumericGeneric
michaeljklein Aug 29, 2024
007610d
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Aug 29, 2024
686360d
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Aug 29, 2024
a75631b
cargo clippy/fmt
michaeljklein Aug 29, 2024
6ee01d2
./test_programs/format.sh
michaeljklein Aug 29, 2024
8db0ee7
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 3, 2024
f1a9b95
add test for #5447
michaeljklein Sep 3, 2024
ea8a61b
use explicit numeric generics in noir_codegen test
michaeljklein Sep 3, 2024
89ea894
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 3, 2024
5b546bc
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 4, 2024
6ec95cc
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 9, 2024
543c713
move type_kind_mismatch and implicit_numeric_generics_type compile_fa…
michaeljklein Sep 9, 2024
dae8ce7
Update compiler/noirc_frontend/src/tests.rs
michaeljklein Sep 9, 2024
b8690f9
Update compiler/noirc_frontend/src/tests.rs
michaeljklein Sep 9, 2024
be8f235
Update compiler/noirc_frontend/src/tests.rs
michaeljklein Sep 9, 2024
3d28fed
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 10, 2024
84c6e38
remove redundant test, restore regression unit test, clarify error in…
michaeljklein Sep 11, 2024
164beb4
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 11, 2024
ff7eb1c
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 11, 2024
ee458aa
deduplicating docs
michaeljklein Sep 11, 2024
43ee2cc
remove redundant test
michaeljklein Sep 11, 2024
4726267
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
TomAFrench Sep 11, 2024
626fe19
wip debugging failing kind errors, remove unused constant_variable fu…
michaeljklein Sep 12, 2024
25be3a0
cargo clippy/fmt
michaeljklein Sep 12, 2024
2ea6063
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 12, 2024
bd4d515
wip debugging kind mismatch errors: Type::Constant needs kind informa…
michaeljklein Sep 12, 2024
dca751c
wip: adding kind info for infix and constant types
michaeljklein Sep 16, 2024
d334fd4
wip debugging tests
michaeljklein Sep 16, 2024
19abd2d
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 16, 2024
6a6a826
updating Type::Constant cases, debugging failing stdlib and test_prog…
michaeljklein Sep 16, 2024
91092e6
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 16, 2024
6c41e48
cleanup, add kind checks in infix_kind instead of defaulting, check f…
michaeljklein Sep 16, 2024
618e053
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 16, 2024
b7dce00
add issue to remove TypeVariableKind::Constant and cleanup
michaeljklein Sep 16, 2024
78478ea
fix test kinds
michaeljklein Sep 16, 2024
b5e53e2
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 17, 2024
7160258
Merge branch 'master' into michaeljklein/no-implicit-numeric-generics
michaeljklein Sep 17, 2024
2bf7048
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Sep 17, 2024
9fb9341
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Sep 17, 2024
b783767
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Sep 17, 2024
fec1bcf
Update compiler/noirc_frontend/src/hir_def/types/arithmetic.rs
michaeljklein Sep 17, 2024
ee8ee18
Update compiler/noirc_frontend/src/tests.rs
michaeljklein Sep 17, 2024
f5fde5d
Update compiler/noirc_frontend/src/hir_def/types/arithmetic.rs
michaeljklein Sep 17, 2024
80da93d
Update test_programs/execution_success/global_consts/src/main.nr
michaeljklein Sep 17, 2024
9159ef6
Update test_programs/execution_success/global_consts/src/main.nr
michaeljklein Sep 17, 2024
18f89fe
Update tooling/lsp/src/requests/hover.rs
michaeljklein Sep 17, 2024
155e896
Update compiler/noirc_frontend/src/hir_def/types/arithmetic.rs
michaeljklein Sep 17, 2024
db7d9f6
Update compiler/noirc_frontend/src/hir_def/types/arithmetic.rs
michaeljklein Sep 17, 2024
075af22
support for bound type variables in Type::kind
michaeljklein Sep 17, 2024
7db75aa
fix displaying '(8: numeric u32)' -> '8'
michaeljklein Sep 17, 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
62 changes: 1 addition & 61 deletions compiler/noirc_frontend/src/elaborator/mod.rs
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 @@ -420,7 +420,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 @@ -892,44 +891,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 @@ -1184,27 +1145,6 @@ impl<'context> Elaborator<'context> {
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
71 changes: 31 additions & 40 deletions compiler/noirc_frontend/src/elaborator/types.rs
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,14 +77,13 @@ 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);
if let Type::NamedGeneric(ref _type_var, ref name, ref kind) = size {
if !kind.is_numeric() {
let ident = Ident::new(name.to_string(), span);
self.push_err(ResolverError::UseExplicitNumericGeneric { ident });
return Type::Error;
}
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}
Type::Array(Box::new(size), elem)
}
Expand All @@ -95,25 +95,24 @@ impl<'context> Elaborator<'context> {
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);
if let Type::NamedGeneric(ref _type_var, ref name, ref kind) = resolved_size {
if !kind.is_numeric() {
let ident = Ident::new(name.to_string(), span);
self.push_err(ResolverError::UseExplicitNumericGeneric { ident });
return Type::Error;
}
}
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);
if let Type::NamedGeneric(ref _type_var, ref name, ref kind) = resolved_size {
if !kind.is_numeric() {
let ident = Ident::new(name.to_string(), span);
self.push_err(ResolverError::UseExplicitNumericGeneric { ident });
return Type::Error;
}
}
let fields = self.resolve_type_inner(*fields, kind);
Type::FmtString(Box::new(resolved_size), Box::new(fields))
Expand Down Expand Up @@ -183,24 +182,16 @@ 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) {
if let Type::NamedGeneric(_type_var, _name, resolved_kind) = &resolved_type {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(&resolved_type.kind(), resolved_kind);
if resolved_type.kind() != kind.clone() {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
let expected_typ_err =
ResolverError::NumericGenericUsedForType { name: name.to_string(), span };
self.push_err(expected_typ_err);
CompilationError::TypeError(TypeCheckError::TypeKindMismatch {
expected_kind: kind.to_string(),
expr_kind: resolved_type.kind().to_string(),
expr_span: span,
});
self.errors.push((expected_typ_err, self.file));
return Type::Error;
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
ResolverError::UseExplicitNumericGeneric { ident } => {
let name = &ident.0.contents;

Diagnostic::simple_warning(
String::from("Noir now supports explicit numeric generics. Support for implicit numeric generics will be removed in the following release."),
Diagnostic::simple_error(
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
String::from("Noir requires explicit numeric generics."),
format!("Numeric generic `{name}` should now be specified with `let {name}: <annotated type>`"),
ident.0.span(),
)
Expand Down
149 changes: 41 additions & 108 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ pub enum Kind {
Numeric(Box<Type>),
}

impl Kind {
pub(crate) fn is_numeric(&self) -> bool {
matches!(self, Self::Numeric { .. })
}
}

impl std::fmt::Display for Kind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down Expand Up @@ -209,6 +215,10 @@ impl ResolvedGeneric {
pub fn as_named_generic(self) -> Type {
Type::NamedGeneric(self.type_var, self.name, self.kind)
}

fn is_numeric(&self) -> bool {
self.kind.is_numeric()
}
}

enum FunctionCoercionResult {
Expand Down Expand Up @@ -330,10 +340,8 @@ impl StructType {
/// True if the given index is the same index as a generic type of this struct
/// which is expected to be a numeric generic.
/// This is needed because we infer type kinds in Noir and don't have extensive kind checking.
/// TODO(https://github.com/noir-lang/noir/issues/5156): This is outdated and we should remove this implicit searching for numeric generics
pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
let target_id = self.generics[index_of_generic].type_var.id();
self.fields.iter().any(|(_, field)| field.contains_numeric_typevar(target_id))
self.generics[index_of_generic].is_numeric()
}

/// Instantiate this struct type, returning a Vec of the new generic args (in
Expand Down Expand Up @@ -420,11 +428,9 @@ impl TypeAlias {
}

/// True if the given index is the same index as a generic type of this alias
/// which is expected to be a numeric generic.
/// This is needed because we infer type kinds in Noir and don't have extensive kind checking.
/// which is a numeric generic.
pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool {
let target_id = self.generics[index_of_generic].type_var.id();
self.typ.contains_numeric_typevar(target_id)
self.generics[index_of_generic].is_numeric()
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -835,78 +841,6 @@ impl Type {
)
}

fn contains_numeric_typevar(&self, target_id: TypeVariableId) -> bool {
// True if the given type is a NamedGeneric with the target_id
let named_generic_id_matches_target = |typ: &Type| {
if let Type::NamedGeneric(type_variable, _, _) = typ {
match &*type_variable.borrow() {
TypeBinding::Bound(_) => {
unreachable!("Named generics should not be bound until monomorphization")
}
TypeBinding::Unbound(id) => target_id == *id,
}
} else {
false
}
};

match self {
Type::FieldElement
| Type::Integer(_, _)
| Type::Bool
| Type::Unit
| Type::Error
| Type::TypeVariable(_, _)
| Type::Constant(_)
| Type::NamedGeneric(_, _, _)
| Type::Forall(_, _)
| Type::Quoted(_) => false,

Type::TraitAsType(_, _, generics) => {
generics.ordered.iter().any(|generic| generic.contains_numeric_typevar(target_id))
|| generics.named.iter().any(|typ| typ.typ.contains_numeric_typevar(target_id))
}
Type::Array(length, elem) => {
elem.contains_numeric_typevar(target_id) || named_generic_id_matches_target(length)
}
Type::Slice(elem) => elem.contains_numeric_typevar(target_id),
Type::Tuple(fields) => {
fields.iter().any(|field| field.contains_numeric_typevar(target_id))
}
Type::Function(parameters, return_type, env, _unconstrained) => {
parameters.iter().any(|parameter| parameter.contains_numeric_typevar(target_id))
|| return_type.contains_numeric_typevar(target_id)
|| env.contains_numeric_typevar(target_id)
}
Type::Struct(struct_type, generics) => {
generics.iter().enumerate().any(|(i, generic)| {
if named_generic_id_matches_target(generic) {
struct_type.borrow().generic_is_numeric(i)
} else {
generic.contains_numeric_typevar(target_id)
}
})
}
Type::Alias(alias, generics) => generics.iter().enumerate().any(|(i, generic)| {
if named_generic_id_matches_target(generic) {
alias.borrow().generic_is_numeric(i)
} else {
generic.contains_numeric_typevar(target_id)
}
}),
Type::MutableReference(element) => element.contains_numeric_typevar(target_id),
Type::String(length) => named_generic_id_matches_target(length),
Type::FmtString(length, elements) => {
elements.contains_numeric_typevar(target_id)
|| named_generic_id_matches_target(length)
}
Type::InfixExpr(lhs, _op, rhs) => {
lhs.contains_numeric_typevar(target_id) || rhs.contains_numeric_typevar(target_id)
}
}
}

/// TODO(https://github.com/noir-lang/noir/issues/5156): Remove with explicit numeric generics
pub fn find_numeric_type_vars(&self, found_names: &mut Vec<String>) {
// Return whether the named generic has a TypeKind::Numeric and save its name
let named_generic_is_numeric = |typ: &Type, found_names: &mut Vec<String>| {
Expand Down Expand Up @@ -1173,35 +1107,34 @@ impl Type {
}
}

// TODO(https://github.com/noir-lang/noir/issues/5156): Bring back this method when we remove implicit numeric generics
// It has been commented out as to not trigger clippy for an unused method
// pub(crate) fn kind(&self) -> Kind {
// match self {
// Type::NamedGeneric(_, _, kind) => kind.clone(),
// Type::Constant(_) => Kind::Numeric(Box::new(Type::Integer(
// Signedness::Unsigned,
// IntegerBitSize::ThirtyTwo,
// ))),
// Type::FieldElement
// | Type::Array(_, _)
// | Type::Slice(_)
// | Type::Integer(_, _)
// | Type::Bool
// | Type::String(_)
// | Type::FmtString(_, _)
// | Type::Unit
// | Type::Tuple(_)
// | Type::Struct(_, _)
// | Type::Alias(_, _)
// | Type::TypeVariable(_, _)
// | Type::TraitAsType(_, _, _)
// | Type::Function(_, _, _)
// | Type::MutableReference(_)
// | Type::Forall(_, _)
// | Type::Quoted(_)
// | Type::Error => Kind::Normal,
// }
// }
pub(crate) fn kind(&self) -> Kind {
match self {
Type::NamedGeneric(_, _, kind) => kind.clone(),
Type::Constant(..) => Kind::Numeric(Box::new(Type::Integer(
Signedness::Unsigned,
IntegerBitSize::ThirtyTwo,
))),
Type::FieldElement
| Type::Array(..)
| Type::Slice(..)
| Type::Integer(..)
| Type::Bool
| Type::String(..)
| Type::FmtString(..)
| Type::Unit
| Type::Tuple(..)
| Type::Struct(..)
| Type::Alias(..)
| Type::TypeVariable(..)
| Type::TraitAsType(..)
| Type::Function(..)
| Type::MutableReference(..)
| Type::Forall(..)
| Type::Quoted(..)
| Type::InfixExpr(..)
| Type::Error => Kind::Normal,
}
}

/// Returns the number of field elements required to represent the type once encoded.
pub fn field_count(&self) -> u32 {
Expand Down
Loading
Loading