Better column query in MPT macros #8
Conversation
| /// Enables column query without knowing the Column type | ||
| pub trait Querier<F: Field> { | ||
| /// Query a column at a relative position | ||
| fn query(self, meta: &mut VirtualCells<F>, at: Rotation) -> Expression<F>; | ||
| } |
There was a problem hiding this comment.
Nice! Makes the API similar to what privacy-scaling-explorations/halo2#154.
There was a problem hiding this comment.
Awesome, I left a comment, I think the expr method name is confusing and perhaps cur would be better for the method name.
| #[allow(unused_macros)] | ||
| macro_rules! cur { | ||
| ($column:expr) => {{ | ||
| $column.query($meta, Rotation::cur()) | ||
| }}; | ||
| } | ||
|
|
||
| #[allow(unused_macros)] | ||
| macro_rules! next { | ||
| ($column:expr) => {{ | ||
| $column.query($meta, Rotation::next()) | ||
| }}; | ||
| } | ||
|
|
||
| #[allow(unused_macros)] | ||
| macro_rules! prev { | ||
| ($column:expr) => {{ | ||
| $column.query($meta, Rotation::prev()) | ||
| }}; | ||
| } | ||
|
|
||
| #[allow(unused_macros)] | ||
| macro_rules! query { | ||
| ($column:expr, $rot:expr) => {{ | ||
| $column.query($meta, Rotation($rot as i32)) | ||
| }}; | ||
| } |
There was a problem hiding this comment.
If the meta dependency is gone for querying cells, do you still think we need these macro's or just using the Querier syntax directly also works for you?
There was a problem hiding this comment.
Do you mean that column.cur() would work without the meta? In that case the macros would be unnecessary I believe.
|
@Brechtpd Is it ok if I review some more before I merge? There's been quite a lot of changes since I last reviewed, so I would like to spend some more time on it to properly understand :). In fact, I started rewriting the specs (pretty much everything needs to be rewritten) and would like to do it in parallel. But if you need some features from master I can just merge it to my branch (to enable sync) and review later? Might be even better this way? For the witness generator - no worries, I can implement |
Of course, no problem.
Ah yes, that also works, thanks! |
No description provided.