Refactor using backend trait #35
Conversation
- move finalized field to backend
There was a problem hiding this comment.
Just some preliminary comment: it's easier for the reviewer for large PRs like this one to chunk it into smaller commits where we can clearly see code being moved around.
I think a simple git mv should be used in a commit if possible (before another commit that changes that code, for example), or it should be clear in each commit what code was removed from a file and added in another.
Also the addition of the type parameter Backend or Field everywhere could be a commit of its own also. It doesn't matter to me if a commit doesn't build, but at least it's easier to review commits within a PR.
I know it's annoying to do these changes though, so I propose that you don't and that I just take the time to go through this PR file by file. Will take me some time this week tho! And the next large PR please cut it down nicely for the reviewer :D
| @@ -0,0 +1,188 @@ | |||
| //! ASM-like language: | |||
There was a problem hiding this comment.
do you know how I can see the diff of asm.rs here? It would have been good to see a mv commit first and then a commit changing it :o not sure if there's a command I can run now to see a diff
| @@ -0,0 +1,183 @@ | |||
| use std::sync::Arc; | |||
There was a problem hiding this comment.
the same is true here, I think for these kind of changes it's easier if I see commit that move things around properly :o
There was a problem hiding this comment.
maybe this commit helps refactor: add backend interface for poseidon builtin
|
close this one for #36 |
Updated backend trait refactor PR based on the review comments from the previous PR.
For any backend related implementations, they are moved to the folder
backends/kimchi/, such as thebuiltin,asm,prover.For the comment: #30 (comment)
FnHandlein generic form.Boxin this branch, it throwscannot borrow *self as mutable because it is also borrowed as immutablein thecompute_exprfunction.Arc.