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 4 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
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,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, type_var_kind) = typ.follow_bindings() {
let msg = "TypeChecker should only track defaultable type vars";
variable.bind(kind.default_type().expect(msg));
variable.bind(type_var_kind.default_type().expect(msg), &type_var_kind.kind());
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/elaborator/trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,12 @@ impl<'context> Elaborator<'context> {
let override_meta = self.interner.function_meta(func_id);
// Substitute each generic on the trait function with the corresponding generic on the impl function
for (
ResolvedGeneric { type_var: trait_fn_generic, .. },
ResolvedGeneric { type_var: trait_fn_generic, kind: trait_fn_kind, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, kind, .. },
) 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));
bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), trait_fn_kind.clone(), arg));
}

let mut substituted_method_ids = HashSet::default();
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/elaborator/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,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 { type_var: trait_fn_generic, kind: trait_fn_kind, .. },
ResolvedGeneric { name, type_var: impl_fn_generic, kind, .. },
) 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));
bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), trait_fn_kind.clone(), arg));
}

let (declaration_type, _) = trait_fn_meta.typ.instantiate_with_bindings(bindings, interner);
Expand Down
45 changes: 21 additions & 24 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,12 @@ impl<'context> Elaborator<'context> {
_ => (),
}

if !kind.matches_opt(resolved_type.kind()) {
if !kind.unifies(&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()),
.to_string(),
expr_span: span,
});
self.errors.push((expected_typ_err, self.file));
Expand Down Expand Up @@ -452,7 +450,7 @@ impl<'context> Elaborator<'context> {
});
return Type::Error;
}
if let Some(result) = op.function(lhs, rhs) {
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 });
Expand All @@ -470,15 +468,13 @@ impl<'context> Elaborator<'context> {
}

fn check_kind(&mut self, typ: Type, expected_kind: &Kind, span: Span) -> Type {
if let Some(kind) = typ.kind() {
if !kind.unifies(expected_kind) {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: expected_kind.to_string(),
expr_kind: kind.to_string(),
expr_span: span,
});
return Type::Error;
}
if !typ.kind().unifies(expected_kind) {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: expected_kind.to_string(),
expr_kind: typ.kind().to_string(),
expr_span: span,
});
return Type::Error;
}
typ
}
Expand Down Expand Up @@ -785,7 +781,7 @@ impl<'context> Elaborator<'context> {
let expected =
Type::Function(args, Box::new(ret.clone()), Box::new(env_type), false);

if let Err(error) = binding.try_bind(expected, span) {
if let Err(error) = binding.try_bind(expected, &Kind::Normal, span) {
self.push_err(error);
}
ret
Expand Down Expand Up @@ -927,13 +923,13 @@ impl<'context> Elaborator<'context> {
span,
});

let use_impl = !lhs_type.is_numeric();
let use_impl = !lhs_type.is_numeric_value();

// If this operator isn't valid for fields we have to possibly narrow
// TypeVariableKind::IntegerOrField to TypeVariableKind::Integer.
// Doing so also ensures a type error if Field is used.
// The is_numeric check is to allow impls for custom types to bypass this.
if !op.kind.is_valid_for_field_type() && lhs_type.is_numeric() {
if !op.kind.is_valid_for_field_type() && lhs_type.is_numeric_value() {
let target = Type::polymorphic_integer(self.interner);

use crate::ast::BinaryOpKind::*;
Expand Down Expand Up @@ -982,7 +978,7 @@ impl<'context> Elaborator<'context> {
&Type::Integer(Signedness::Unsigned, IntegerBitSize::Eight),
|| TypeCheckError::InvalidShiftSize { span },
);
let use_impl = if lhs_type.is_numeric() {
let use_impl = if lhs_type.is_numeric_value() {
let integer_type = Type::polymorphic_integer(self.interner);
self.bind_type_variables_for_infix(lhs_type, op, &integer_type, span)
} else {
Expand Down Expand Up @@ -1082,14 +1078,14 @@ impl<'context> Elaborator<'context> {

// The `!` prefix operator is not valid for Field, so if this is a numeric
// type we constrain it to just (non-Field) integer types.
if matches!(op, crate::ast::UnaryOp::Not) && rhs_type.is_numeric() {
if matches!(op, crate::ast::UnaryOp::Not) && rhs_type.is_numeric_value() {
let integer_type = Type::polymorphic_integer(self.interner);
self.unify(rhs_type, &integer_type, || {
TypeCheckError::InvalidUnaryOp { kind: rhs_type.to_string(), span }
});
}

Ok((rhs_type.clone(), !rhs_type.is_numeric()))
Ok((rhs_type.clone(), !rhs_type.is_numeric_value()))
}
Integer(sign_x, bit_width_x) => {
if *op == UnaryOp::Minus && *sign_x == Signedness::Unsigned {
Expand Down Expand Up @@ -1171,7 +1167,7 @@ impl<'context> Elaborator<'context> {
let object_type = object_type.substitute(&bindings);
bindings.insert(
the_trait.self_type_typevar.id(),
(the_trait.self_type_typevar.clone(), object_type.clone()),
(the_trait.self_type_typevar.clone(), Kind::Normal, object_type.clone()),
);
self.interner.select_impl_for_expression(
expr_id,
Expand Down Expand Up @@ -1736,7 +1732,7 @@ impl<'context> Elaborator<'context> {
for (param, arg) in the_trait.generics.iter().zip(&constraint.trait_generics.ordered) {
// Avoid binding t = t
if !arg.occurs(param.type_var.id()) {
bindings.insert(param.type_var.id(), (param.type_var.clone(), arg.clone()));
bindings.insert(param.type_var.id(), (param.type_var.clone(), param.kind.clone(), arg.clone()));
}
}

Expand All @@ -1755,7 +1751,7 @@ impl<'context> Elaborator<'context> {

// Avoid binding t = t
if !arg.typ.occurs(param.type_var.id()) {
bindings.insert(param.type_var.id(), (param.type_var.clone(), arg.typ.clone()));
bindings.insert(param.type_var.id(), (param.type_var.clone(), param.kind.clone(), arg.typ.clone()));
}
}

Expand All @@ -1764,7 +1760,8 @@ impl<'context> Elaborator<'context> {
// to specify a redundant type annotation.
if assumed {
let self_type = the_trait.self_type_typevar.clone();
bindings.insert(self_type.id(), (self_type, constraint.typ.clone()));
// self_type has Kind::Normal
bindings.insert(self_type.id(), (self_type, Kind::Normal, constraint.typ.clone()));
}
}
}
Expand Down
11 changes: 6 additions & 5 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
HirAssignStatement, HirConstrainStatement, HirForStatement, HirLValue, HirLetStatement,
HirPattern,
},
types::Kind,
},
macros_api::{HirLiteral, HirStatement, NodeInterner},
node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, StmtId},
Expand Down Expand Up @@ -89,7 +90,7 @@
// To match the monomorphizer, we need to call follow_bindings on each of
// the instantiation bindings before we unbind the generics from the previous function.
// This is because the instantiation bindings refer to variables from the call site.
for (_, binding) in instantiation_bindings.values_mut() {
for (_, _kind, binding) in instantiation_bindings.values_mut() {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
*binding = binding.follow_bindings();
}

Expand All @@ -98,7 +99,7 @@
let mut impl_bindings =
perform_impl_bindings(self.elaborator.interner, trait_method, function, location)?;

for (_, binding) in impl_bindings.values_mut() {
for (_, _kind, binding) in impl_bindings.values_mut() {
*binding = binding.follow_bindings();
}

Expand Down Expand Up @@ -243,7 +244,7 @@
}
} else {
let name = self.elaborator.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

Check warning on line 247 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down Expand Up @@ -349,7 +350,7 @@

if let Some(bindings) = self.bound_generics.last() {
for (var, binding) in bindings {
var.force_bind(binding.clone());
var.force_bind(binding.clone(), &Kind::Normal);
}
}
}
Expand All @@ -360,11 +361,11 @@
.last_mut()
.expect("remember_bindings called with no bound_generics on the stack");

for (var, binding) in main_bindings.values() {
for (var, _kind, binding) in main_bindings.values() {
bound_generics.insert(var.clone(), binding.follow_bindings());
}

for (var, binding) in impl_bindings.values() {
for (var, _kind, binding) in impl_bindings.values() {
bound_generics.insert(var.clone(), binding.follow_bindings());
}
}
Expand Down
Loading
Loading