-
Notifications
You must be signed in to change notification settings - Fork 97
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
Remove constraints blocks #652
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,22 +32,38 @@ impl<T: FieldElement> Folder<T> for MacroExpander<T> { | |
type Error = Infallible; | ||
|
||
fn fold_machine(&mut self, mut machine: Machine<T>) -> Result<Machine<T>, Self::Error> { | ||
machine.statements.iter_mut().for_each(|s| match s { | ||
MachineStatement::InstructionDeclaration(_, _, Instruction { body, .. }) => { | ||
match body { | ||
InstructionBody::Local(body) => { | ||
*body = self.expand_macros(std::mem::take(body)) | ||
} | ||
InstructionBody::CallableRef(..) => { | ||
machine.statements = machine | ||
.statements | ||
.into_iter() | ||
.flat_map(|s| match s { | ||
MachineStatement::InstructionDeclaration( | ||
start, | ||
name, | ||
Instruction { body, params }, | ||
) => { | ||
let body = match body { | ||
InstructionBody::Local(body) => { | ||
InstructionBody::Local(self.expand_macros(body)) | ||
} | ||
// there is nothing to expand in a callable ref | ||
} | ||
InstructionBody::CallableRef(r) => InstructionBody::CallableRef(r), | ||
}; | ||
vec![MachineStatement::InstructionDeclaration( | ||
start, | ||
name, | ||
Instruction { body, params }, | ||
)] | ||
} | ||
} | ||
MachineStatement::InlinePil(_, statements) => { | ||
*statements = self.expand_macros(std::mem::take(statements)); | ||
} | ||
_ => {} | ||
}); | ||
MachineStatement::Pil(start, statement) => self | ||
.expand_macros(vec![statement]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would make more sense to me for |
||
.into_iter() | ||
.map(|s| MachineStatement::Pil(start, s)) | ||
.collect(), | ||
s => { | ||
vec![s] | ||
} | ||
}) | ||
.collect(); | ||
|
||
Ok(machine) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ use super::{ | |
AnalysisASMFile, AssignmentStatement, CallableSymbol, CallableSymbolDefinitionRef, | ||
DebugDirective, DegreeStatement, FunctionBody, FunctionStatement, FunctionStatements, | ||
Incompatible, IncompatibleSet, Instruction, InstructionDefinitionStatement, | ||
InstructionStatement, LabelStatement, LinkDefinitionStatement, Machine, PilBlock, | ||
InstructionStatement, LabelStatement, LinkDefinitionStatement, Machine, | ||
RegisterDeclarationStatement, RegisterTy, Return, Rom, SubmachineDeclaration, | ||
}; | ||
|
||
|
@@ -40,7 +40,7 @@ impl<T: Display> Display for Machine<T> { | |
write_items_indented(f, &self.registers)?; | ||
write_items_indented(f, &self.instructions)?; | ||
write_items_indented(f, &self.callable)?; | ||
write_items_indented(f, &self.constraints)?; | ||
write_items_indented(f, &self.pil)?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it a problem that these might be ordered differently? I mean can we always re-parse stuff that is printed here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate here? Within constraints, the order is preserved throughout the compiler, but eventually this should not be a requirement, right? There are no tests which parse outputs of this display afaik, this could make sense to add but it seems unrelated to this change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just wondering if it might make sense to move the pil a bit further up or what happens if you mix constraints and instruction declarations and such, but yes, unrelated to this PR. |
||
write_items_indented(f, &self.links)?; | ||
|
||
writeln!(f, "}}") | ||
|
@@ -144,16 +144,6 @@ impl Display for LabelStatement { | |
} | ||
} | ||
|
||
impl<T: Display> Display for PilBlock<T> { | ||
fn fmt(&self, f: &mut Formatter<'_>) -> Result { | ||
writeln!(f, "constraints {{")?; | ||
for statement in &self.statements { | ||
writeln!(f, "{}", indent(statement, 1))?; | ||
} | ||
writeln!(f, "}}") | ||
} | ||
} | ||
|
||
impl Display for RegisterDeclarationStatement { | ||
fn fmt(&self, f: &mut Formatter<'_>) -> Result { | ||
write!(f, "reg {}{};", self.name, self.ty,) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1 @@ | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
# PIL | ||
|
||
powdr PIL is the lower level of abstraction in powdr. It is strongly inspired by and compatible with [Polygon zkEVM PIL](https://github.com/0xPolygonHermez/pilcom/). We refer to the [Polygon zkEVM PIL documentation](https://wiki.polygon.technology/docs/category/polynomial-identity-language/) and document deviations from the original design here. | ||
powdr-pil is the lower level of abstraction in powdr. It is strongly inspired by [Polygon zkEVM PIL](https://github.com/0xPolygonHermez/pilcom/). We refer to the [Polygon zkEVM PIL documentation](https://wiki.polygon.technology/docs/category/polynomial-identity-language/) and document deviations from the original design here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was changed to a functional approach, as a statement can yield many, and
iter_mut
does not allow that. Hence the usage of flat_map here.