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

Allow the type checker to accept empty values #1393

Merged
merged 72 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
9d18617
Empty value in type checker
gzanitti May 21, 2024
d4253d5
WIP
gzanitti May 22, 2024
3ed99e5
Empty tuple & tests
gzanitti May 22, 2024
0d12e02
Removed line in test
gzanitti May 22, 2024
28ba122
Tests fixed
gzanitti May 22, 2024
612acef
Extra test
gzanitti May 22, 2024
fb72711
std with ()
gzanitti May 22, 2024
9c95cb0
std with ()
gzanitti May 22, 2024
7321790
New unification
gzanitti May 22, 2024
f497b6c
Hacky unification. To be checked
gzanitti May 23, 2024
1569fa7
clippy
gzanitti May 23, 2024
6b2e85a
More tests fixed
gzanitti May 23, 2024
aedec5c
Fix types
gzanitti May 23, 2024
73efe49
Error removed
gzanitti May 23, 2024
425b70b
Back to std::prelude::Constr
gzanitti May 23, 2024
3f46e9d
expect_type seems too deep
gzanitti May 24, 2024
63e4a5e
Minor issues & types
gzanitti May 24, 2024
94420a2
Merge branch 'type_checker_empty' of https://github.com/gzanitti/powd…
gzanitti May 24, 2024
a7863f9
Conflict solved & merged with main
gzanitti May 24, 2024
8cf085f
rollback
gzanitti May 24, 2024
ad712e8
New approach
gzanitti May 24, 2024
6649c5e
New approach
gzanitti May 24, 2024
f6b1ff6
Checking statements
gzanitti May 27, 2024
f3b4d8b
Main merged
gzanitti May 27, 2024
4555b1f
format!
gzanitti May 27, 2024
a4e19ba
format!
gzanitti May 27, 2024
8a8f1a8
Better error reporting and fixed tests
gzanitti May 27, 2024
872ab59
Lambda_kind in TypeChecker
gzanitti May 27, 2024
a948b50
Comments removed
gzanitti May 27, 2024
40c2e1f
Check statements inside Constr functions against Constr and ()
gzanitti May 27, 2024
aaae2a1
assert always failing
gzanitti May 27, 2024
1f6dec7
Issue with sustitutions
gzanitti May 27, 2024
06cb0d1
asserts weirdly failing, need to investigate
gzanitti May 28, 2024
46f6006
Merge remote-tracking branch 'upstream/main' into type_checker_empty
gzanitti May 28, 2024
ae72155
minor changes
gzanitti May 28, 2024
6454efb
Allow empty values in ExpectedType. Issue in the condenser
gzanitti May 29, 2024
6758149
Minor fixes and better error reporting
gzanitti May 29, 2024
0a3f9f6
removed return
gzanitti May 29, 2024
79b002a
Issue in simple_sum: Expected std::prelude::Constr or () but got int …
gzanitti May 29, 2024
561e7c5
Need to check how to try exp_ty variant when can't unify
gzanitti May 30, 2024
fd884df
Cleaner code but missing documentation
gzanitti May 30, 2024
80c8ab0
TODO
gzanitti May 30, 2024
3a40232
Simplified check
gzanitti Jun 4, 2024
41d5f10
Main merged
gzanitti Jun 4, 2024
c0446e0
Even more simplified version
gzanitti Jun 4, 2024
80fb590
Even more simplified version (minor fix)
gzanitti Jun 4, 2024
b204d31
Type corrected
gzanitti Jun 4, 2024
1f5e47b
Test data fixed
gzanitti Jun 4, 2024
98baf75
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 4, 2024
30542a5
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 4, 2024
1fdb7fb
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 4, 2024
3250066
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 4, 2024
bb07873
chriseth comments addressed
gzanitti Jun 4, 2024
2ca3d81
chriseth comments addresed
gzanitti Jun 4, 2024
89d12b6
chriseth comments addresed
gzanitti Jun 4, 2024
31256f7
main merged
gzanitti Jun 4, 2024
0292955
Change expected type
gzanitti Jun 4, 2024
0cbae87
Fixed expected error (should_panic)
gzanitti Jun 4, 2024
bb2232a
Fixed expected error (should_panic)
gzanitti Jun 4, 2024
37e1315
Update pil-analyzer/src/condenser.rs
gzanitti Jun 6, 2024
6fa7c4c
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 6, 2024
fa13fc7
Update pil-analyzer/src/type_inference.rs
gzanitti Jun 6, 2024
8a8e7e4
Minor fix
gzanitti Jun 6, 2024
7204b46
Empty tuple method
gzanitti Jun 6, 2024
ac4b91a
Minor fix
gzanitti Jun 6, 2024
5eaa699
Merge remote-tracking branch 'upstream/main' into type_checker_empty
gzanitti Jun 6, 2024
fa8357a
Test
gzanitti Jun 7, 2024
6354d20
Rolling back quick test
gzanitti Jun 7, 2024
b082e8b
opt number in type inferece
gzanitti Jun 10, 2024
bd2fd85
opt number in type inferece
gzanitti Jun 10, 2024
18fc8b3
type_if_literal
gzanitti Jun 10, 2024
6361dea
better name and correct types
gzanitti Jun 10, 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: 4 additions & 0 deletions ast/src/parsed/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ impl<E> Type<E> {
pub fn contained_type_vars(&self) -> impl Iterator<Item = &String> {
self.contained_type_vars_with_repetitions().unique()
}

pub fn empty_tuple() -> Type<E> {
Type::Tuple(TupleType { items: vec![] })
}
}
impl<E: Clone> Type<E> {
/// Substitutes all occurrences of the given type variables with the given types.
Expand Down
9 changes: 8 additions & 1 deletion pil-analyzer/src/condenser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,14 @@ impl<'a, T: FieldElement> Condenser<'a, T> {
if identity.kind == IdentityKind::Polynomial {
let expr = identity.expression_for_poly_id();
evaluator::evaluate(expr, self)
.and_then(|expr| self.add_constraints(expr, identity.source.clone()))
.and_then(|expr| {
if let Value::Tuple(items) = expr.as_ref() {
assert!(items.is_empty());
Ok(())
} else {
self.add_constraints(expr, identity.source.clone())
}
})
.unwrap_or_else(|err| {
panic!(
"Error reducing expression to constraint:\nExpression: {expr}\nError: {err:?}"
Expand Down
33 changes: 20 additions & 13 deletions pil-analyzer/src/pil_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,18 @@ impl PILAnalyzer {
Some((name.clone(), (type_scheme, expr)))
})
.collect();
// Collect all expressions in identities.
let statement_type = ExpectedType {
let constr_function_statement_type = ExpectedType {
ty: Type::NamedType(SymbolPath::from_str("std::prelude::Constr").unwrap(), None),
allow_array: true,
allow_empty: true,
};
for id in &mut self.identities {
if id.kind == IdentityKind::Polynomial {
// At statement level, we allow Constr or Constr[].
expressions.push((id.expression_for_poly_id_mut(), statement_type.clone()));
// At statement level, we allow Constr, Constr[] or ().
expressions.push((
id.expression_for_poly_id_mut(),
constr_function_statement_type.clone(),
));
} else {
for part in [&mut id.left, &mut id.right] {
if let Some(selector) = &mut part.selector {
Expand All @@ -290,15 +293,19 @@ impl PILAnalyzer {
}
}
}
let inferred_types = infer_types(definitions, &mut expressions, &statement_type)
.map_err(|mut errors| {
eprintln!("\nError during type inference:");
for e in &errors {
e.output_to_stderr();
}
errors.pop().unwrap()
})
.unwrap();
let inferred_types = infer_types(
definitions,
&mut expressions,
&constr_function_statement_type,
)
.map_err(|mut errors| {
eprintln!("\nError during type inference:");
for e in &errors {
e.output_to_stderr();
}
errors.pop().unwrap()
})
.unwrap();
// Store the inferred types.
for (name, ty) in inferred_types {
let Some(FunctionValueDefinition::Expression(TypedExpression {
Expand Down
22 changes: 6 additions & 16 deletions pil-analyzer/src/side_effect_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,12 @@ impl<'a> SideEffectChecker<'a> {
}
Expression::BlockExpression(_, BlockExpression { statements, .. }) => {
for s in statements {
match s {
StatementInsideBlock::LetStatement(s) => {
if s.value.is_none() && self.context != FunctionKind::Constr {
return Err(format!(
"Tried to create a witness column in a {} context: {s}",
self.context
));
}
}
StatementInsideBlock::Expression(expr) => {
if self.context != FunctionKind::Constr {
return Err(format!(
"Tried to add a constraint in a {} context: {expr}",
self.context
));
}
if let StatementInsideBlock::LetStatement(ls) = s {
if ls.value.is_none() && self.context != FunctionKind::Constr {
return Err(format!(
"Tried to create a witness column in a {} context: {ls}",
self.context
));
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions pil-analyzer/src/type_builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ lazy_static! {
("std::convert::fe", ("T: FromLiteral", "T -> fe")),
("std::convert::int", ("T: FromLiteral", "T -> int")),
("std::convert::expr", ("T: FromLiteral", "T -> expr")),
(
"std::debug::print",
("T: ToString", "T -> std::prelude::Constr[]")
),
("std::debug::print", ("T: ToString", "T -> ()")),
("std::field::modulus", ("", "-> int")),
("std::prover::challenge", ("", "int, int -> expr")),
("std::prover::degree", ("", "-> int")),
Expand Down
129 changes: 75 additions & 54 deletions pil-analyzer/src/type_inference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use powdr_ast::{
display::format_type_scheme_around_name,
types::{ArrayType, FunctionType, TupleType, Type, TypeBounds, TypeScheme},
visitor::ExpressionVisitable,
ArrayLiteral, BinaryOperation, BlockExpression, FunctionCall, IndexAccess,
ArrayLiteral, BinaryOperation, BlockExpression, FunctionCall, FunctionKind, IndexAccess,
LambdaExpression, LetStatementInsideBlock, MatchArm, MatchExpression, Number, Pattern,
SourceReference, StatementInsideBlock, UnaryOperation,
},
Expand Down Expand Up @@ -39,21 +39,25 @@ pub fn infer_types(
#[derive(Clone)]
pub struct ExpectedType {
pub ty: Type,
/// If true, arrays of `ty` are also allowed.
pub allow_array: bool,
/// If true, the empty tuple is also allowed.
pub allow_empty: bool,
}

impl From<Type> for ExpectedType {
fn from(ty: Type) -> Self {
ExpectedType {
ty,
allow_array: false,
allow_empty: false,
}
}
}

struct TypeChecker<'a> {
/// The expected type for expressions at statement level in block expressions.
statement_type: &'a ExpectedType,
/// The expected type for expressions at statement level in block expressions inside a constr function.
constr_function_statement_type: &'a ExpectedType,
/// Types for local variables, might contain type variables.
local_var_types: Vec<Type>,
/// Declared types for all symbols and their source references.
Expand All @@ -65,17 +69,20 @@ struct TypeChecker<'a> {
unifier: Unifier,
/// Last used type variable index.
last_type_var: usize,
/// Keeps track of the kind of lambda we are currently type-checking.
lambda_kind: FunctionKind,
}

impl<'a> TypeChecker<'a> {
pub fn new(statement_type: &'a ExpectedType) -> Self {
Self {
statement_type,
constr_function_statement_type: statement_type,
local_var_types: Default::default(),
declared_types: Default::default(),
declared_type_vars: Default::default(),
unifier: Default::default(),
last_type_var: Default::default(),
lambda_kind: FunctionKind::Constr,
}
}

Expand Down Expand Up @@ -441,31 +448,30 @@ impl<'a> TypeChecker<'a> {
expected_type: &ExpectedType,
expr: &mut Expression,
) -> Result<(), Error> {
if expected_type.allow_array {
self.infer_type_of_expression(expr).and_then(|ty| {
let ty = self.type_into_substituted(ty);
let expected_type = if matches!(ty, Type::Array(_)) {
Type::Array(ArrayType {
base: Box::new(expected_type.ty.clone()),
length: None,
})
} else {
expected_type.ty.clone()
};
update_type_if_literal(expr, &expected_type.ty);

self.unifier
.unify_types(ty.clone(), expected_type.clone())
.map_err(|err| {
expr.source_reference().with_error(format!(
"Expected type {} but got type {}.\n{err}",
self.format_type_with_bounds(expected_type),
self.format_type_with_bounds(ty)
))
})
let ty = self.infer_type_of_expression(expr)?;
let ty = self.type_into_substituted(ty);
let expected_type = if expected_type.allow_array && matches!(ty, Type::Array(_)) {
Type::Array(ArrayType {
base: Box::new(expected_type.ty.clone()),
length: None,
})
} else if expected_type.allow_empty && (ty == Type::empty_tuple()) {
Type::empty_tuple()
} else {
self.expect_type(&expected_type.ty, expr)
}
expected_type.ty.clone()
};

self.unifier
.unify_types(ty.clone(), expected_type.clone())
.map_err(|err| {
expr.source_reference().with_error(format!(
"Expected type {} but got type {}.\n{err}",
self.format_type_with_bounds(expected_type),
self.format_type_with_bounds(ty),
))
})
}

/// Process an expression and return the type of the expression.
Expand Down Expand Up @@ -532,14 +538,7 @@ impl<'a> TypeChecker<'a> {
.map(|item| self.infer_type_of_expression(item))
.collect::<Result<_, _>>()?,
}),
Expression::LambdaExpression(
source_ref,
LambdaExpression {
kind: _,
params,
body,
},
) => {
Expression::LambdaExpression(source_ref, LambdaExpression { kind, params, body }) => {
let old_len = self.local_var_types.len();
let result = params
.iter()
Expand All @@ -548,7 +547,11 @@ impl<'a> TypeChecker<'a> {
// TODO we need a better source reference
.map_err(|err| source_ref.with_error(err))
.and_then(|param_types| {
Ok((param_types, self.infer_type_of_expression(body)?))
let old_lambda_kind = self.lambda_kind;
self.lambda_kind = *kind;
let body_type = self.infer_type_of_expression(body);
self.lambda_kind = old_lambda_kind;
Ok((param_types, body_type?))
});
self.local_var_types.truncate(old_len);
let (param_types, body_type) = result?;
Expand Down Expand Up @@ -639,6 +642,7 @@ impl<'a> TypeChecker<'a> {
}
Expression::BlockExpression(source_ref, BlockExpression { statements, expr }) => {
let original_var_count = self.local_var_types.len();

for statement in statements {
match statement {
StatementInsideBlock::LetStatement(LetStatementInsideBlock {
Expand All @@ -655,7 +659,7 @@ impl<'a> TypeChecker<'a> {
.map_err(|err| source_ref.with_error(err))?;
}
StatementInsideBlock::Expression(expr) => {
self.expect_type_with_flexibility(self.statement_type, expr)?;
self.expect_type_with_flexibility(&self.statement_type(), expr)?;
}
}
}
Expand All @@ -666,6 +670,15 @@ impl<'a> TypeChecker<'a> {
})
}

/// Returns the type expected at statement level, given the current function context.
fn statement_type(&self) -> ExpectedType {
gzanitti marked this conversation as resolved.
Show resolved Hide resolved
if self.lambda_kind == FunctionKind::Constr {
self.constr_function_statement_type.clone()
} else {
Type::empty_tuple().into()
}
}

/// Process a function call and return the type of the expression.
/// The error message is used to clarify which kind of function call it is
/// (it might be an operator).
Expand Down Expand Up @@ -707,24 +720,8 @@ impl<'a> TypeChecker<'a> {
/// This function should be preferred over `infer_type_of_expression` if an expected type is known
/// because we can create better error messages.
fn expect_type(&mut self, expected_type: &Type, expr: &mut Expression) -> Result<(), Error> {
// For literals, we try to store the type here already.
// This avoids creating tons of type variables for large arrays.
if let Expression::Number(
_,
Number {
type_: annotated_type @ None,
..
},
) = expr
{
match expected_type {
Type::Int => *annotated_type = Some(Type::Int),
Type::Fe => *annotated_type = Some(Type::Fe),
Type::Expr => *annotated_type = Some(Type::Expr),
Type::TypeVar(tv) => *annotated_type = Some(Type::TypeVar(tv.clone())),
_ => {}
};
}
update_type_if_literal(expr, expected_type);

let inferred_type = self.infer_type_of_expression(expr)?;
self.unifier
.unify_types(inferred_type.clone(), expected_type.clone())
Expand Down Expand Up @@ -946,3 +943,27 @@ impl<'a> TypeChecker<'a> {
self.local_var_types[id as usize].clone()
}
}

fn update_type_if_literal(
expr: &mut powdr_ast::parsed::Expression<Reference>,
expected_type: &Type,
) {
// For literals, we try to store the type here already.
// This avoids creating tons of type variables for large arrays.
if let Expression::Number(
_,
Number {
type_: annotated_type @ None,
..
},
) = expr
{
match expected_type.clone() {
Type::Int => *annotated_type = Some(Type::Int),
Type::Fe => *annotated_type = Some(Type::Fe),
Type::Expr => *annotated_type = Some(Type::Expr),
Type::TypeVar(tv) => *annotated_type = Some(Type::TypeVar(tv.clone())),
_ => {}
};
}
}
1 change: 0 additions & 1 deletion pil-analyzer/src/type_unifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ impl Unifier {
.zip(args2)
.try_for_each(|(a1, a2)| self.unify_types(a1, a2))
}

(ty1, ty2) => Err(format!("Cannot unify types {ty1} and {ty2}")),
}
}
Expand Down
2 changes: 1 addition & 1 deletion pil-analyzer/tests/parse_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ fn expression_but_expected_constraint() {
}

#[test]
#[should_panic = "Expected type: expr\\nInferred type: std::prelude::Constr\\n"]
#[should_panic = "Expected type expr but got type std::prelude::Constr."]
fn constraint_but_expected_expression() {
let input = r#"namespace N(16);
col witness y;
Expand Down
4 changes: 2 additions & 2 deletions pil-analyzer/tests/side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn new_wit_in_pure() {
}

#[test]
#[should_panic = "Tried to add a constraint in a pure context: x = 7"]
#[should_panic = "Expected type () but got type std::prelude::Constr"]
fn constr_in_pure() {
let input = r#"namespace N(16);
let new_col = |x| { x = 7; [] };
Expand Down Expand Up @@ -77,7 +77,7 @@ fn constr_lambda_in_pure() {
}

#[test]
#[should_panic = "Tried to add a constraint in a pure context: x = 7"]
#[should_panic = "Expected type () but got type std::prelude::Constr"]
fn reset_context() {
let input = r#"namespace N(16);
let new_col = |x| { x = 7; [] };
Expand Down
2 changes: 1 addition & 1 deletion pil-analyzer/tests/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ fn enum_is_not_constr() {
}

#[test]
#[should_panic = "Expected type: int -> std::prover::Query"]
#[should_panic = "Expected type int -> std::prover::Query"]
fn query_with_wrong_type() {
let input = "col witness w(i) query i;";
type_check(input, &[]);
Expand Down
Loading
Loading