From 763044694824d3f475bb5f194082a44d4db17aec 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 | 1 - executor/src/witgen/query_processor.rs | 2 +- parser/src/powdr.lalrpop | 24 ++++--- pil_analyzer/src/condenser.rs | 34 ++++++--- pil_analyzer/src/evaluator.rs | 4 +- pil_analyzer/src/pil_analyzer.rs | 14 +--- pilopt/src/lib.rs | 1 - 17 files changed, 95 insertions(+), 144 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 1397eb6cfb..1a731abcf1 100644 --- a/ast/src/analyzed/display.rs +++ b/ast/src/analyzed/display.rs @@ -223,13 +223,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 bb009b5948..74915ab316 100644 --- a/ast/src/analyzed/mod.rs +++ b/ast/src/analyzed/mod.rs @@ -630,7 +630,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 095f42a0f0..ee71930ab3 100644 --- a/backend/src/pilstark/json_exporter/mod.rs +++ b/backend/src/pilstark/json_exporter/mod.rs @@ -59,7 +59,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(), diff --git a/executor/src/witgen/query_processor.rs b/executor/src/witgen/query_processor.rs index 9fd97af1cc..8676832113 100644 --- a/executor/src/witgen/query_processor.rs +++ b/executor/src/witgen/query_processor.rs @@ -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) 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 01790c7c70..b9b7eb85a6 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(_, _)) => { @@ -164,14 +164,28 @@ impl Condenser { } Expression::UnaryOperation(op, inner) => { let inner = self.condense_expression(inner); - match inner { - AlgebraicExpression::Number(n) => { - AlgebraicExpression::Number(evaluate_unary_operation(*op, n)) + if *op == UnaryOperator::Next { + let AlgebraicExpression::Reference(reference) = inner else { + panic!( + "Can apply \"'\" operator only directly to columns in this context." + ); + }; + + assert!(!reference.next, "Double application of \"'\""); + AlgebraicExpression::Reference(AlgebraicReference { + next: true, + ..reference + }) + } else { + match inner { + AlgebraicExpression::Number(n) => { + AlgebraicExpression::Number(evaluate_unary_operation(*op, n)) + } + _ => AlgebraicExpression::UnaryOperation( + (*op).try_into().unwrap(), + Box::new(inner), + ), } - _ => AlgebraicExpression::UnaryOperation( - (*op).try_into().unwrap(), - Box::new(inner), - ), } } Expression::PublicReference(r) => AlgebraicExpression::PublicReference(r.clone()), 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 2b5ab8837e..d25f8d1ad3 100644 --- a/pilopt/src/lib.rs +++ b/pilopt/src/lib.rs @@ -260,7 +260,6 @@ fn substitute_polynomial_references( if let Expression::Reference(Reference::Poly(PolynomialReference { name: _, index, - next: _, poly_id: Some(poly_id), })) = e {