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!: Syncing TypeVariableKind with Kind #6094

Merged
merged 36 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
4c012d3
wip: syncing TypeVariableKind with Kind, removing the temporary 'None…
michaeljklein Sep 18, 2024
93aee54
wip validating that op.function results fit in their kinds
michaeljklein Sep 19, 2024
a790553
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Sep 19, 2024
1002bf4
wip(todo!(..)'s remain): adding kinds checks to (try_)bind_to and Typ…
michaeljklein Sep 19, 2024
10a9bf1
wip debugging tests, implemented value size check, add assertions whe…
michaeljklein Sep 20, 2024
1081c50
wip debugging, add max_value method for FieldElement, include kind in…
michaeljklein Sep 20, 2024
a9aba8c
wip: debugging
michaeljklein Sep 23, 2024
236cd61
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Sep 23, 2024
f75fcd7
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Sep 23, 2024
2f497bd
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Sep 23, 2024
ca62c45
wip debugging missing kinds, splitting out kind size checks, add kind…
michaeljklein Sep 24, 2024
b59241e
wip implementing Jake's suggestions, incl. moving TypeVariableKind in…
michaeljklein Sep 24, 2024
953f285
wip: add UnresolvedGeneric::type_variable_kind, propagating changes t…
michaeljklein Sep 24, 2024
2a11bbe
wip debugging, move UnsupportedNumericGenericType into its own struct…
michaeljklein Sep 25, 2024
9cb1aab
wip debugging and add Kind::Normal
michaeljklein Sep 25, 2024
1ba8e44
wip debugging failing tests, updating kinds in noirc_driver and lsp, …
michaeljklein Sep 26, 2024
9e9fde0
wip debugging (most frontend tests passing or only failing with sligh…
michaeljklein Sep 26, 2024
3204c21
wip debugging: disable substitution kind check when overwiting bound …
michaeljklein Sep 26, 2024
ad7ed34
all tests passing! use resolve_generic_kind when possible, use Kind::…
michaeljklein Sep 27, 2024
b81611b
cleanup, remove unused kind arguments from (force_)bind, simplify nex…
michaeljklein Sep 27, 2024
7790be2
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Sep 27, 2024
306a5d7
cargo clippy / fmt
michaeljklein Sep 27, 2024
2581427
Update compiler/noirc_frontend/src/node_interner.rs
michaeljklein Sep 30, 2024
d838173
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Sep 30, 2024
ceac4b0
Update compiler/noirc_frontend/src/hir_def/types.rs
michaeljklein Sep 30, 2024
0792566
Update compiler/noirc_frontend/src/hir/def_collector/errors.rs
michaeljklein Sep 30, 2024
e218ec2
Merge branch 'master' into michaeljklein/sync-type-var-kinds
TomAFrench Sep 30, 2024
a70a97b
wip debugging parameter type regression, cleanup, note issue for kind…
michaeljklein Oct 1, 2024
029f1c3
fixed numeric generic as param type
michaeljklein Oct 1, 2024
0831c61
cargo clippy / fmt
michaeljklein Oct 1, 2024
334dd1d
move tests to test sub-folders
michaeljklein Oct 1, 2024
67f6058
note issue to follow up on Kind::Any
michaeljklein Oct 1, 2024
0bcc7ee
Merge branch 'master' into michaeljklein/sync-type-var-kinds
michaeljklein Oct 1, 2024
00dd2fc
patching after merge
michaeljklein Oct 1, 2024
0c31e98
Update compiler/noirc_frontend/src/tests.rs
michaeljklein Oct 1, 2024
bd37c34
Update compiler/noirc_frontend/src/elaborator/types.rs
TomAFrench Oct 1, 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
20 changes: 12 additions & 8 deletions compiler/noirc_driver/src/abi_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use noirc_abi::{
Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign,
};
use noirc_frontend::ast::{Signedness, Visibility};
use noirc_frontend::TypeBinding;
use noirc_frontend::{
hir::Context,
hir_def::{
Expand All @@ -17,7 +18,6 @@ use noirc_frontend::{
},
node_interner::{FuncId, NodeInterner},
};
use noirc_frontend::{TypeBinding, TypeVariableKind};

/// Arranges a function signature and a generated circuit's return witnesses into a
/// `noirc_abi::Abi`.
Expand Down Expand Up @@ -72,13 +72,18 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {

AbiType::Integer { sign, width: (*bit_width).into() }
}
Type::TypeVariable(binding, TypeVariableKind::IntegerOrField)
| Type::TypeVariable(binding, TypeVariableKind::Integer) => match &*binding.borrow() {
TypeBinding::Bound(typ) => abi_type_from_hir_type(context, typ),
TypeBinding::Unbound(_) => {
abi_type_from_hir_type(context, &Type::default_int_or_field_type())
Type::TypeVariable(binding) => {
if binding.is_integer() || binding.is_integer_or_field() {
match &*binding.borrow() {
TypeBinding::Bound(typ) => abi_type_from_hir_type(context, typ),
TypeBinding::Unbound(_id, _kind) => {
abi_type_from_hir_type(context, &Type::default_int_or_field_type())
}
}
} else {
unreachable!("{typ} cannot be used in the abi")
}
},
}
Type::Bool => AbiType::Boolean,
Type::String(size) => {
let size = size
Expand Down Expand Up @@ -106,7 +111,6 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
| Type::Constant(..)
| Type::InfixExpr(..)
| Type::TraitAsType(..)
| Type::TypeVariable(_, _)
| Type::NamedGeneric(..)
| Type::Forall(..)
| Type::Quoted(_)
Expand Down
16 changes: 12 additions & 4 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::borrow::Cow;
use std::fmt::Display;

use thiserror::Error;

use crate::ast::{
Ident, ItemVisibility, Path, Pattern, Recoverable, Statement, StatementKind,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
};
use crate::hir::def_collector::errors::DefCollectorErrorKind;
use crate::node_interner::{
ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId, StructId,
};
Expand Down Expand Up @@ -76,6 +77,13 @@ pub enum UnresolvedGeneric {
Resolved(QuotedTypeId, Span),
}

#[derive(Error, PartialEq, Eq, Debug, Clone)]
#[error("The only supported types of numeric generics are integers, fields, and booleans")]
pub struct UnsupportedNumericGenericType {
pub ident: Ident,
pub typ: UnresolvedTypeData,
}

impl UnresolvedGeneric {
pub fn span(&self) -> Span {
match self {
Expand All @@ -85,7 +93,7 @@ impl UnresolvedGeneric {
}
}

pub fn kind(&self) -> Result<Kind, DefCollectorErrorKind> {
pub fn kind(&self) -> Result<Kind, UnsupportedNumericGenericType> {
match self {
UnresolvedGeneric::Variable(_) => Ok(Kind::Normal),
UnresolvedGeneric::Numeric { typ, .. } => {
Expand All @@ -101,14 +109,14 @@ impl UnresolvedGeneric {
fn resolve_numeric_kind_type(
&self,
typ: &UnresolvedType,
) -> Result<Type, DefCollectorErrorKind> {
) -> Result<Type, UnsupportedNumericGenericType> {
use crate::ast::UnresolvedTypeData::{FieldElement, Integer};

match typ.typ {
FieldElement => Ok(Type::FieldElement),
Integer(sign, bits) => Ok(Type::Integer(sign, bits)),
// Only fields and integers are supported for numeric kinds
_ => Err(DefCollectorErrorKind::UnsupportedNumericGenericType {
_ => Err(UnsupportedNumericGenericType {
ident: self.ident().clone(),
typ: typ.typ.clone(),
}),
Expand Down
57 changes: 31 additions & 26 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
ast::{
BlockExpression, FunctionKind, GenericTypeArgs, Ident, NoirFunction, NoirStruct, Param,
Path, Pattern, TraitBound, UnresolvedGeneric, UnresolvedGenerics,
UnresolvedTraitConstraint, UnresolvedTypeData,
UnresolvedTraitConstraint, UnresolvedTypeData, UnsupportedNumericGenericType,
},
graph::CrateId,
hir::{
Expand Down Expand Up @@ -345,8 +345,9 @@ impl<'context> Elaborator<'context> {
fn introduce_generics_into_scope(&mut self, all_generics: Vec<ResolvedGeneric>) {
// Introduce all numeric generics into scope
for generic in &all_generics {
if let Kind::Numeric(typ) = &generic.kind {
let definition = DefinitionKind::NumericGeneric(generic.type_var.clone());
if let Kind::Numeric(typ) = &generic.kind() {
let definition =
DefinitionKind::NumericGeneric(generic.type_var.clone(), typ.clone());
let ident = Ident::new(generic.name.to_string(), generic.span);
let hir_ident = self.add_variable_decl(
ident, false, // mutable
Expand Down Expand Up @@ -461,9 +462,9 @@ impl<'context> Elaborator<'context> {
let context = self.function_context.pop().expect("Imbalanced function_context pushes");

for typ in context.type_variables {
if let Type::TypeVariable(variable, kind) = typ.follow_bindings() {
if let Type::TypeVariable(variable) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";
variable.bind(kind.default_type().expect(msg));
variable.bind(variable.kind().default_type().expect(msg));
}
}

Expand Down Expand Up @@ -501,11 +502,12 @@ impl<'context> Elaborator<'context> {
trait_constraints: &mut Vec<TraitConstraint>,
) -> Type {
let new_generic_id = self.interner.next_type_variable_id();
let new_generic = TypeVariable::unbound(new_generic_id);

let new_generic = TypeVariable::unbound(new_generic_id, Kind::Normal);
generics.push(new_generic.clone());

let name = format!("impl {trait_path}");
let generic_type = Type::NamedGeneric(new_generic, Rc::new(name), Kind::Normal);
let generic_type = Type::NamedGeneric(new_generic, Rc::new(name));
let trait_bound = TraitBound { trait_path, trait_id: None, trait_generics };

if let Some(new_constraint) = self.resolve_trait_bound(&trait_bound, generic_type.clone()) {
Expand All @@ -520,19 +522,20 @@ impl<'context> Elaborator<'context> {
pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics {
vecmap(generics, |generic| {
let mut is_error = false;
let (type_var, name, kind) = match self.resolve_generic(generic) {
let (type_var, name) = match self.resolve_generic(generic) {
Ok(values) => values,
Err(error) => {
self.push_err(error);
is_error = true;
let id = self.interner.next_type_variable_id();
(TypeVariable::unbound(id), Rc::new("(error)".into()), Kind::Normal)
let kind = self.resolve_generic_kind(generic);
(TypeVariable::unbound(id, kind), Rc::new("(error)".into()))
}
};

let span = generic.span();
let name_owned = name.as_ref().clone();
let resolved_generic = ResolvedGeneric { name, type_var, kind, span };
let resolved_generic = ResolvedGeneric { name, type_var, span };

// Check for name collisions of this generic
// Checking `is_error` here prevents DuplicateDefinition errors when
Expand All @@ -557,25 +560,22 @@ impl<'context> Elaborator<'context> {
fn resolve_generic(
&mut self,
generic: &UnresolvedGeneric,
) -> Result<(TypeVariable, Rc<String>, Kind), ResolverError> {
) -> Result<(TypeVariable, Rc<String>), ResolverError> {
// Map the generic to a fresh type variable
match generic {
UnresolvedGeneric::Variable(_) | UnresolvedGeneric::Numeric { .. } => {
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
let ident = generic.ident();

let kind = self.resolve_generic_kind(generic);
let typevar = TypeVariable::unbound(id, kind);
let ident = generic.ident();
let name = Rc::new(ident.0.contents.clone());
Ok((typevar, name, kind))
Ok((typevar, name))
}
// An already-resolved generic is only possible if it is the result of a
// previous macro call being inserted into a generics list.
UnresolvedGeneric::Resolved(id, span) => {
match self.interner.get_quoted_type(*id).follow_bindings() {
Type::NamedGeneric(type_variable, name, kind) => {
Ok((type_variable, name, kind))
}
Type::NamedGeneric(type_variable, name) => Ok((type_variable.clone(), name)),
other => Err(ResolverError::MacroResultInGenericsListNotAGeneric {
span: *span,
typ: other.clone(),
Expand All @@ -590,17 +590,21 @@ impl<'context> Elaborator<'context> {
/// sure only primitive numeric types are being used.
pub(super) fn resolve_generic_kind(&mut self, generic: &UnresolvedGeneric) -> Kind {
if let UnresolvedGeneric::Numeric { ident, typ } = generic {
let typ = typ.clone();
let typ = if typ.is_type_expression() {
self.resolve_type_inner(typ, &Kind::Numeric(Box::new(Type::default_int_type())))
let unresolved_typ = typ.clone();
let typ = if unresolved_typ.is_type_expression() {
self.resolve_type_inner(
unresolved_typ.clone(),
&Kind::Numeric(Box::new(Type::default_int_type())),
)
} else {
self.resolve_type(typ.clone())
self.resolve_type(unresolved_typ.clone())
};
if !matches!(typ, Type::FieldElement | Type::Integer(_, _)) {
let unsupported_typ_err = ResolverError::UnsupportedNumericGenericType {
ident: ident.clone(),
typ: typ.clone(),
};
let unsupported_typ_err =
ResolverError::UnsupportedNumericGenericType(UnsupportedNumericGenericType {
ident: ident.clone(),
typ: unresolved_typ.typ.clone(),
});
self.push_err(unsupported_typ_err);
}
Kind::Numeric(Box::new(typ))
Expand Down Expand Up @@ -735,6 +739,7 @@ impl<'context> Elaborator<'context> {
UnresolvedTypeData::TraitAsType(path, args) => {
self.desugar_impl_trait_arg(path, args, &mut generics, &mut trait_constraints)
}
// Function parameters have Kind::Normal
_ => self.resolve_type_inner(typ, &Kind::Normal),
};

Expand Down
9 changes: 5 additions & 4 deletions compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
stmt::HirPattern,
},
node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind},
ResolvedGeneric, Shared, StructType, Type, TypeBindings,
Kind, ResolvedGeneric, Shared, StructType, Type, TypeBindings,
};

use super::{Elaborator, ResolverMeta};
Expand Down Expand Up @@ -489,7 +489,7 @@ impl<'context> Elaborator<'context> {
) -> Vec<Type> {
let generics_with_types = generics.iter().zip(turbofish_generics);
vecmap(generics_with_types, |(generic, unresolved_type)| {
self.resolve_type_inner(unresolved_type, &generic.kind)
self.resolve_type_inner(unresolved_type, &generic.kind())
})
}

Expand Down Expand Up @@ -558,13 +558,14 @@ impl<'context> Elaborator<'context> {

self.interner.add_global_reference(global_id, hir_ident.location);
}
DefinitionKind::NumericGeneric(_) => {
DefinitionKind::NumericGeneric(_, ref numeric_typ) => {
// Initialize numeric generics to a polymorphic integer type in case
// they're used in expressions. We must do this here since type_check_variable
// does not check definition kinds and otherwise expects parameters to
// already be typed.
if self.interner.definition_type(hir_ident.id) == Type::Error {
let typ = Type::polymorphic_integer_or_field(self.interner);
let type_var_kind = Kind::Numeric(numeric_typ.clone());
let typ = self.type_variable_with_kind(type_var_kind);
self.interner.push_definition_type(hir_ident.id, typ);
}
}
Expand Down
10 changes: 7 additions & 3 deletions compiler/noirc_frontend/src/elaborator/trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,15 @@ impl<'context> Elaborator<'context> {
// Substitute each generic on the trait function with the corresponding generic on the impl function
for (
ResolvedGeneric { type_var: trait_fn_generic, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, kind, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, .. },
) in method.direct_generics.iter().zip(&override_meta.direct_generics)
{
let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), kind.clone());
bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg));
let trait_fn_kind = trait_fn_generic.kind();
let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone());
bindings.insert(
trait_fn_generic.id(),
(trait_fn_generic.clone(), trait_fn_kind.clone(), arg),
);
}

let mut substituted_method_ids = HashSet::default();
Expand Down
13 changes: 6 additions & 7 deletions compiler/noirc_frontend/src/elaborator/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
hir::{def_collector::dc_crate::UnresolvedTrait, type_check::TypeCheckError},
hir_def::{function::Parameters, traits::TraitFunction},
node_interner::{FuncId, NodeInterner, ReferenceId, TraitId},
Kind, ResolvedGeneric, Type, TypeBindings, TypeVariableKind,
ResolvedGeneric, Type, TypeBindings,
};

use super::Elaborator;
Expand Down Expand Up @@ -79,8 +79,7 @@ impl<'context> Elaborator<'context> {
self.recover_generics(|this| {
let the_trait = this.interner.get_trait(trait_id);
let self_typevar = the_trait.self_type_typevar.clone();
let self_type =
Type::TypeVariable(self_typevar.clone(), TypeVariableKind::Normal);
let self_type = Type::TypeVariable(self_typevar.clone());
let name_span = the_trait.name.span();

this.add_existing_generic(
Expand All @@ -90,7 +89,6 @@ impl<'context> Elaborator<'context> {
name: Rc::new("Self".to_owned()),
type_var: self_typevar,
span: name_span,
kind: Kind::Normal,
},
);
this.self_type = Some(self_type.clone());
Expand Down Expand Up @@ -279,11 +277,12 @@ pub(crate) fn check_trait_impl_method_matches_declaration(
// Substitute each generic on the trait function with the corresponding generic on the impl function
for (
ResolvedGeneric { type_var: trait_fn_generic, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, kind, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, .. },
) in trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics)
{
let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), kind.clone());
bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg));
let trait_fn_kind = trait_fn_generic.kind();
let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone());
bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), trait_fn_kind, arg));
}

let (declaration_type, _) = trait_fn_meta.typ.instantiate_with_bindings(bindings, interner);
Expand Down
Loading
Loading