From c2fa193b0df446544f519495e1fa1879b5c9b1b0 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 30 Oct 2023 18:23:00 +0100 Subject: [PATCH] Parse next as operator. --- analysis/src/macro_expansion.rs | 3 - asm_to_pil/src/vm_to_constrained.rs | 18 +++-- ast/src/analyzed/display.rs | 3 +- ast/src/analyzed/mod.rs | 1 - ast/src/asm_analysis/mod.rs | 10 ++- ast/src/lib.rs | 1 + ast/src/parsed/build.rs | 15 +--- ast/src/parsed/display.rs | 15 ++-- ast/src/parsed/mod.rs | 87 +++++---------------- ast/src/parsed/visitor.rs | 6 +- backend/src/pilstark/json_exporter/mod.rs | 5 +- executor/src/witgen/expression_evaluator.rs | 1 + executor/src/witgen/query_processor.rs | 6 +- parser/src/powdr.lalrpop | 24 +++--- pil_analyzer/src/condenser.rs | 19 ++++- pil_analyzer/src/evaluator.rs | 4 +- pil_analyzer/src/pil_analyzer.rs | 14 +--- pilopt/src/lib.rs | 3 +- 18 files changed, 94 insertions(+), 141 deletions(-) diff --git a/analysis/src/macro_expansion.rs b/analysis/src/macro_expansion.rs index 6106947c81..940c99db59 100644 --- a/analysis/src/macro_expansion.rs +++ b/analysis/src/macro_expansion.rs @@ -173,9 +173,6 @@ where fn process_expression(&mut self, e: &mut Expression) { if let Expression::Reference(poly) = e { if poly.namespace().is_none() && self.parameter_names.contains_key(poly.name()) { - // TODO to make this work inside macros, "next" and "index" need to be - // their own ast nodes / operators. - assert!(!poly.shift()); assert!(poly.index().is_none()); *e = self.arguments[self.parameter_names[poly.name()]].clone() } diff --git a/asm_to_pil/src/vm_to_constrained.rs b/asm_to_pil/src/vm_to_constrained.rs index 84496a5e48..15998347e3 100644 --- a/asm_to_pil/src/vm_to_constrained.rs +++ b/asm_to_pil/src/vm_to_constrained.rs @@ -538,7 +538,6 @@ impl ASMPILConverter { .map(|(reg, a)| { // Output a value trough assignment register "reg" if let Expression::Reference(r) = a { - assert!(!r.shift()); assert!(r.index().is_none()); (reg.clone(), vec![r.name().into()]) } else { @@ -567,7 +566,6 @@ impl ASMPILConverter { Expression::Reference(reference) => { assert!(reference.namespace().is_none()); assert!(reference.index().is_none()); - assert!(!reference.shift()); // TODO check it actually is a register vec![( 1.into(), @@ -1029,11 +1027,17 @@ fn extract_update(expr: Expression) -> (Option, Expr // TODO check that there are no other "next" references in the expression if let Expression::BinaryOperation(left, BinaryOperator::Sub, right) = expr { match *left { - Expression::Reference(r) if r.shift() => { - assert!(r.namespace().is_none()); - assert!(r.index().is_none()); - (Some(r.name().into()), *right) - } + Expression::UnaryOperation(UnaryOperator::Next, column) => match *column { + Expression::Reference(column) => { + assert!(column.namespace().is_none()); + assert!(column.index().is_none()); + return (Some(column.name().into()), *right); + } + _ => ( + None, + Expression::UnaryOperation(UnaryOperator::Next, column) - *right, + ), + }, _ => (None, *left - *right), } } else { diff --git a/ast/src/analyzed/display.rs b/ast/src/analyzed/display.rs index 4602ecd349..2bcfd82337 100644 --- a/ast/src/analyzed/display.rs +++ b/ast/src/analyzed/display.rs @@ -208,13 +208,12 @@ impl Display for PolynomialReference { fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( f, - "{}{}{}", + "{}{}", self.name, self.index .as_ref() .map(|s| format!("[{s}]")) .unwrap_or_default(), - if self.next { "'" } else { "" }, ) } } diff --git a/ast/src/analyzed/mod.rs b/ast/src/analyzed/mod.rs index afe6422b1c..1d82647db4 100644 --- a/ast/src/analyzed/mod.rs +++ b/ast/src/analyzed/mod.rs @@ -564,7 +564,6 @@ pub struct PolynomialReference { /// TODO make this non-optional pub poly_id: Option, pub index: Option, - pub next: bool, } #[derive(Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)] diff --git a/ast/src/asm_analysis/mod.rs b/ast/src/asm_analysis/mod.rs index 1e55ad13a2..58a1fc159d 100644 --- a/ast/src/asm_analysis/mod.rs +++ b/ast/src/asm_analysis/mod.rs @@ -18,7 +18,7 @@ use crate::parsed::{ AbsoluteSymbolPath, AssignmentRegister, CallableRef, InstructionBody, OperationId, Params, }, visitor::{ExpressionVisitable, VisitOrder}, - PilStatement, ShiftedPolynomialReference, + NamespacedPolynomialReference, PilStatement, }; pub use crate::parsed::Expression; @@ -540,10 +540,12 @@ pub enum FunctionStatement { Return(Return), } -impl ExpressionVisitable>> for FunctionStatement { +impl ExpressionVisitable>> + for FunctionStatement +{ fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> std::ops::ControlFlow where - F: FnMut(&mut Expression>) -> std::ops::ControlFlow, + F: FnMut(&mut Expression>) -> std::ops::ControlFlow, { match self { FunctionStatement::Assignment(assignment) => { @@ -565,7 +567,7 @@ impl ExpressionVisitable>> for Fu fn visit_expressions(&self, f: &mut F, o: VisitOrder) -> std::ops::ControlFlow where - F: FnMut(&Expression>) -> std::ops::ControlFlow, + F: FnMut(&Expression>) -> std::ops::ControlFlow, { match self { FunctionStatement::Assignment(assignment) => { diff --git a/ast/src/lib.rs b/ast/src/lib.rs index 064c7222b1..d4d0569e74 100644 --- a/ast/src/lib.rs +++ b/ast/src/lib.rs @@ -72,6 +72,7 @@ pub fn evaluate_unary_operation(op: UnaryOperator, v: T) -> T { UnaryOperator::Plus => v, UnaryOperator::Minus => -v, UnaryOperator::LogicalNot => v.is_zero().into(), + UnaryOperator::Next => panic!("Cannot evaluate \"'\"."), } } diff --git a/ast/src/parsed/build.rs b/ast/src/parsed/build.rs index f06eba29b4..ee10b4e22f 100644 --- a/ast/src/parsed/build.rs +++ b/ast/src/parsed/build.rs @@ -1,27 +1,18 @@ use crate::parsed::Expression; -use super::PolynomialReference; +use super::{PolynomialReference, UnaryOperator}; pub fn direct_reference, T>(name: S) -> Expression { - PolynomialReference::new(name) - .single() - .local() - .current() - .into() + PolynomialReference::new(name).single().local().into() } pub fn namespaced_reference, T>(namespace: String, name: S) -> Expression { PolynomialReference::new(name) .single() .namespaced(namespace) - .current() .into() } pub fn next_reference(name: &str) -> Expression { - PolynomialReference::new(name) - .single() - .local() - .next() - .into() + Expression::UnaryOperation(UnaryOperator::Next, Box::new(direct_reference(name))) } diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index a5b1fb3b26..d52ef657c7 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -434,7 +434,13 @@ impl Display for Expression { Expression::LambdaExpression(lambda) => write!(f, "{}", lambda), Expression::ArrayLiteral(array) => write!(f, "{array}"), Expression::BinaryOperation(left, op, right) => write!(f, "({left} {op} {right})"), - Expression::UnaryOperation(op, exp) => write!(f, "{op}{exp}"), + Expression::UnaryOperation(op, exp) => { + if op.is_prefix() { + write!(f, "{op}{exp}") + } else { + write!(f, "{exp}{op}") + } + } Expression::FunctionCall(fun_call) => write!(f, "{fun_call}"), Expression::FreeInput(input) => write!(f, "${{ {input} }}"), Expression::MatchExpression(scrutinee, arms) => { @@ -458,12 +464,6 @@ impl Display for PolynomialName { } } -impl Display for ShiftedPolynomialReference { - fn fmt(&self, f: &mut Formatter<'_>) -> Result { - write!(f, "{}{}", self.pol, if self.is_next { "'" } else { "" }) - } -} - impl Display for NamespacedPolynomialReference { fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( @@ -549,6 +549,7 @@ impl Display for UnaryOperator { UnaryOperator::Minus => "-", UnaryOperator::Plus => "+", UnaryOperator::LogicalNot => "!", + UnaryOperator::Next => "'", } ) } diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index f9d69cbde1..043a45d217 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -68,7 +68,7 @@ impl Default for SelectedExpressions { } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub enum Expression> { +pub enum Expression> { Reference(Ref), PublicReference(String), Number(T), @@ -82,6 +82,7 @@ pub enum Expression> { Box>, ), UnaryOperation(UnaryOperator, Box>), + FunctionCall(FunctionCall), FreeInput(Box>), MatchExpression(Box>, Vec>), @@ -144,8 +145,8 @@ impl From for Expression { } } -impl From> for Expression { - fn from(value: ShiftedPolynomialReference) -> Self { +impl From> for Expression { + fn from(value: NamespacedPolynomialReference) -> Self { Self::Reference(value) } } @@ -156,47 +157,6 @@ pub struct PolynomialName { pub array_size: Option>, } -/// A polynomial with an optional shift -#[derive(Debug, PartialEq, Eq, Default, Clone, PartialOrd, Ord)] -pub struct ShiftedPolynomialReference { - /// Whether we shift or not - is_next: bool, - /// The underlying polynomial - pol: NamespacedPolynomialReference, -} - -impl ShiftedPolynomialReference { - /// Returns the underlying namespaced polynomial - pub fn into_namespaced(self) -> NamespacedPolynomialReference { - self.pol - } - - /// Returns the shift of this polynomial - pub fn shift(&self) -> bool { - self.is_next - } - - /// Returns the optional namespace of the underlying polynomial - pub fn namespace(&self) -> &Option { - self.pol.namespace() - } - - /// Returns the optional index of the underlying polynomial in its declaration array - pub fn index(&self) -> &Option>> { - self.pol.index() - } - - /// Returns the name of the declared polynomial or array of polynomials - pub fn name(&self) -> &str { - self.pol.name() - } - - /// Returns a mutable reference to the declared polynomial or array of polynomials - pub fn name_mut(&mut self) -> &mut String { - self.pol.name_mut() - } -} - #[derive(Debug, PartialEq, Eq, Default, Clone, PartialOrd, Ord)] /// A polynomial with an optional namespace pub struct NamespacedPolynomialReference { @@ -207,14 +167,6 @@ pub struct NamespacedPolynomialReference { } impl NamespacedPolynomialReference { - /// Return a shifted polynomial based on this namespaced polynomial and a boolean shift - pub fn with_shift(self, next: bool) -> ShiftedPolynomialReference { - ShiftedPolynomialReference { - is_next: next, - pol: self, - } - } - /// Returns the optional namespace of this polynomial pub fn namespace(&self) -> &Option { &self.namespace @@ -234,16 +186,6 @@ impl NamespacedPolynomialReference { pub fn name_mut(&mut self) -> &mut String { self.pol.name_mut() } - - /// Return a shifted polynomial based on this namespaced polynomial with a shift of 1 - pub fn next(self) -> ShiftedPolynomialReference { - self.with_shift(true) - } - - /// Return a shifted polynomial based on this namespaced polynomial with a shift of 0 - pub fn current(self) -> ShiftedPolynomialReference { - self.with_shift(false) - } } #[derive(Debug, PartialEq, Eq, Default, Clone, PartialOrd, Ord)] @@ -333,13 +275,13 @@ impl PolynomialReference { } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct LambdaExpression> { +pub struct LambdaExpression> { pub params: Vec, pub body: Box>, } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct ArrayLiteral> { +pub struct ArrayLiteral> { pub items: Vec>, } @@ -348,6 +290,17 @@ pub enum UnaryOperator { Plus, Minus, LogicalNot, + Next, +} + +impl UnaryOperator { + /// Returns true if the operator is a prefix-operator and false if it is a postfix operator. + pub fn is_prefix(&self) -> bool { + match self { + UnaryOperator::Plus | UnaryOperator::Minus | UnaryOperator::LogicalNot => true, + UnaryOperator::Next => false, + } + } } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] @@ -374,20 +327,20 @@ pub enum BinaryOperator { } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub struct FunctionCall> { +pub struct FunctionCall> { pub id: String, pub arguments: Vec>, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub struct MatchArm> { +pub struct MatchArm> { pub pattern: MatchPattern, pub value: Expression, } /// A pattern for a match arm. We could extend this in the future. #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub enum MatchPattern> { +pub enum MatchPattern> { CatchAll, Pattern(Expression), } diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index c28829617f..b6561622e6 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -2,7 +2,7 @@ use std::ops::ControlFlow; use super::{ ArrayExpression, ArrayLiteral, Expression, FunctionCall, FunctionDefinition, LambdaExpression, - MatchArm, MatchPattern, PilStatement, SelectedExpressions, ShiftedPolynomialReference, + MatchArm, MatchPattern, NamespacedPolynomialReference, PilStatement, SelectedExpressions, }; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -180,10 +180,10 @@ impl ExpressionVisitable> for Expression { } } -impl ExpressionVisitable>> for PilStatement { +impl ExpressionVisitable>> for PilStatement { fn visit_expressions_mut(&mut self, f: &mut F, o: VisitOrder) -> ControlFlow where - F: FnMut(&mut Expression>) -> ControlFlow, + F: FnMut(&mut Expression>) -> ControlFlow, { match self { PilStatement::FunctionCall(_, _, arguments) => arguments diff --git a/backend/src/pilstark/json_exporter/mod.rs b/backend/src/pilstark/json_exporter/mod.rs index cafbf3f733..a840839749 100644 --- a/backend/src/pilstark/json_exporter/mod.rs +++ b/backend/src/pilstark/json_exporter/mod.rs @@ -58,7 +58,6 @@ pub fn export(analyzed: &Analyzed) -> PIL { StatementIdentifier::PublicDeclaration(name) => { let pub_def = &analyzed.public_declarations[name]; let pub_ref = &pub_def.polynomial; - assert!(!pub_ref.next); let (_, expr) = exporter.polynomial_reference_to_json(&AlgebraicReference { name: pub_ref.name.clone(), poly_id: pub_ref.poly_id.unwrap(), @@ -336,7 +335,9 @@ impl<'a, T: FieldElement> Exporter<'a, T> { ..DEFAULT_EXPR }, ), - UnaryOperator::LogicalNot => panic!("Operator {op} not allowed here."), + UnaryOperator::LogicalNot | UnaryOperator::Next => { + panic!("Operator {op} not allowed here.") + } } } } diff --git a/executor/src/witgen/expression_evaluator.rs b/executor/src/witgen/expression_evaluator.rs index 85cf0fdd79..3c8dd4c6f0 100644 --- a/executor/src/witgen/expression_evaluator.rs +++ b/executor/src/witgen/expression_evaluator.rs @@ -146,6 +146,7 @@ where panic!() } } + UnaryOperator::Next => todo!(), // We could shift the affine result, but actually this should not occur. }) } } diff --git a/executor/src/witgen/query_processor.rs b/executor/src/witgen/query_processor.rs index 250f41e487..8676832113 100644 --- a/executor/src/witgen/query_processor.rs +++ b/executor/src/witgen/query_processor.rs @@ -116,7 +116,7 @@ where Ok(rows.current_row_index.into()) } Expression::Reference(Reference::Poly(poly)) => { - if !poly.next && poly.index.is_none() { + if poly.index.is_none() { let poly_id = poly.poly_id.unwrap(); match poly_id.ptype { PolynomialType::Committed | PolynomialType::Intermediate => { @@ -124,7 +124,7 @@ where name: poly.name.clone(), poly_id, index: poly.index, - next: poly.next, + next: false, }; Ok(rows .get_value(&poly_ref) @@ -135,7 +135,7 @@ where } } else { Err(IncompleteCause::ExpressionEvaluationUnimplemented( - "Cannot evaluate arrays or next references.".to_string(), + "Cannot evaluate arrays.".to_string(), )) } } diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 72ce5c1818..6599e22c7e 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -451,20 +451,29 @@ PowOp: BinaryOperator = { } Unary: Box> = { - UnaryOp Term => Box::new(Expression::UnaryOperation(<>)), - Term, + PrefixUnaryOp PostfixUnary => Box::new(Expression::UnaryOperation(<>)), + PostfixUnary, } -UnaryOp: UnaryOperator = { +PrefixUnaryOp: UnaryOperator = { "+" => UnaryOperator::Plus, "-" => UnaryOperator::Minus, "!" => UnaryOperator::LogicalNot, } +PostfixUnary: Box> = { + => Box::new(Expression::UnaryOperation(o, t)), + Term, +} + +PostfixUnaryOp: UnaryOperator = { + "'" => UnaryOperator::Next, +} + Term: Box> = { FunctionCall => Box::new(Expression::FunctionCall(<>)), - ConstantIdentifier => Box::new(Expression::Reference(PolynomialReference::new(<>).single().local().current())), - ShiftedPolynomialReference => Box::new(Expression::Reference(<>)), + ConstantIdentifier => Box::new(Expression::Reference(PolynomialReference::new(<>).single().local())), + NamespacedPolynomialReference => Box::new(Expression::Reference(<>)), PublicReference => Box::new(Expression::PublicReference(<>)), FieldElement => Box::new(Expression::Number(<>)), StringLiteral => Box::new(Expression::String(<>)), @@ -479,11 +488,6 @@ FunctionCall: FunctionCall = { "(" ")" => FunctionCall {<>}, } -ShiftedPolynomialReference: ShiftedPolynomialReference = { - - => pol.with_shift(next.is_some()), -} - NamespacedPolynomialReference: NamespacedPolynomialReference = { "." )?> => pol.with_namespace(namespace), diff --git a/pil_analyzer/src/condenser.rs b/pil_analyzer/src/condenser.rs index 47ac799d66..4244e86fb3 100644 --- a/pil_analyzer/src/condenser.rs +++ b/pil_analyzer/src/condenser.rs @@ -10,7 +10,7 @@ use ast::{ StatementIdentifier, Symbol, SymbolKind, }, evaluate_binary_operation, evaluate_unary_operation, - parsed::{visitor::ExpressionVisitable, SelectedExpressions}, + parsed::{visitor::ExpressionVisitable, SelectedExpressions, UnaryOperator}, }; use number::FieldElement; @@ -126,7 +126,7 @@ impl Condenser { pub fn condense_expression(&self, e: &Expression) -> AlgebraicExpression { match e { Expression::Reference(Reference::Poly(poly)) => { - if !poly.next && poly.index.is_none() { + if poly.index.is_none() { if let Some(value) = self.constants.get(&poly.name) { return AlgebraicExpression::Number(*value); } @@ -140,7 +140,7 @@ impl Condenser { name: poly.name.clone(), poly_id, index: poly.index, - next: poly.next, + next: false, }) } Expression::Reference(Reference::LocalVar(_, _)) => { @@ -160,6 +160,19 @@ impl Condenser { } Expression::UnaryOperation(op, inner) => { let inner = self.condense_expression(inner); + if *op == UnaryOperator::Next { + if let AlgebraicExpression::Reference(reference) = inner { + assert!(!reference.next, "Double application of \"'\""); + return AlgebraicExpression::Reference(AlgebraicReference { + next: true, + ..reference + }); + } else { + panic!( + "Can apply \"'\" operator only directly to columns in this context." + ); + } + } match inner { AlgebraicExpression::Number(n) => { AlgebraicExpression::Number(evaluate_unary_operation(*op, n)) diff --git a/pil_analyzer/src/evaluator.rs b/pil_analyzer/src/evaluator.rs index 67fc8b3b87..a7f97af923 100644 --- a/pil_analyzer/src/evaluator.rs +++ b/pil_analyzer/src/evaluator.rs @@ -52,7 +52,7 @@ impl<'a, T: FieldElement> Evaluator<'a, T> { match expr { Expression::Reference(Reference::LocalVar(i, _name)) => Ok(self.variables[*i as usize]), Expression::Reference(Reference::Poly(poly)) => { - if !poly.next && poly.index.is_none() { + if poly.index.is_none() { if let Some((_, value)) = self.definitions.get(&poly.name.to_string()) { match value { Some(FunctionValueDefinition::Expression(value)) => { @@ -64,7 +64,7 @@ impl<'a, T: FieldElement> Evaluator<'a, T> { panic!("Reference to {}, which is not a fixed column.", poly.name) } } else { - Err("Cannot evaluate arrays or next references.".to_string()) + Err("Cannot evaluate arrays.".to_string()) } } Expression::PublicReference(r) => Err(format!("Cannot evaluate public reference: {r}")), diff --git a/pil_analyzer/src/pil_analyzer.rs b/pil_analyzer/src/pil_analyzer.rs index 484616ca10..686a8d6774 100644 --- a/pil_analyzer/src/pil_analyzer.rs +++ b/pil_analyzer/src/pil_analyzer.rs @@ -539,12 +539,11 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { PExpression::Reference(poly) => { if poly.namespace().is_none() && self.local_variables.contains_key(poly.name()) { let id = self.local_variables[poly.name()]; - assert!(!poly.shift()); assert!(poly.index().is_none()); Expression::Reference(Reference::LocalVar(id, poly.name().to_string())) } else { Expression::Reference(Reference::Poly( - self.process_shifted_polynomial_reference(poly), + self.process_namespaced_polynomial_reference(poly), )) } } @@ -634,17 +633,6 @@ impl<'a, T: FieldElement> ExpressionProcessor<'a, T> { name, poly_id: None, index, - next: false, - } - } - - pub fn process_shifted_polynomial_reference( - &mut self, - poly: ::ast::parsed::ShiftedPolynomialReference, - ) -> PolynomialReference { - PolynomialReference { - next: poly.shift(), - ..self.process_namespaced_polynomial_reference(poly.into_namespaced()) } } } diff --git a/pilopt/src/lib.rs b/pilopt/src/lib.rs index d0c2bc4702..084c95a0e6 100644 --- a/pilopt/src/lib.rs +++ b/pilopt/src/lib.rs @@ -112,7 +112,7 @@ fn simplify_expression_single(e: &mut AlgebraicExpression) { if let AlgebraicExpression::UnaryOperation(op, inner) = e { if let AlgebraicExpression::Number(inner) = **inner { *e = AlgebraicExpression::Number(match op { - UnaryOperator::Plus => inner, + UnaryOperator::Plus | UnaryOperator::Next => inner, UnaryOperator::Minus => -inner, UnaryOperator::LogicalNot => inner.is_zero().into(), }); @@ -259,7 +259,6 @@ fn substitute_polynomial_references( if let Expression::Reference(Reference::Poly(PolynomialReference { name: _, index, - next: _, poly_id: Some(poly_id), })) = e {