chore: break big files into smaller ones

[guorong009]

Description

Break the big files(> 1k LOC) into smaller files

Related issues

• Resolve Break big files into smaller ones #297

Changes

• refactor the files in halo2_frontend/src/plonk/circuit dir
  • create new files - ../plonk/circuit/helpers.rs & ../plonk/circuit/constraint_system/helpers.rs
  • move the contents from expression.rs & constraint_system.rs to new files
• refactor the halo2_frontend/src/circuit.rs file
  • create new file witness_collector.rs inside halo2_frontend/src/circuit/ dir

NOTE

There are additional big files - halo2_frontend/src/dev.rs & halo2_backend/src/plonk/prover.rs.
I believe they don't need splitting, since they include only what's needed.

[ed255]

Most of the changes look good to me but I have two comments:

The first is that I see the new files column.rs, phase.rs and query.rs which have definitions that were previously in expression.rs. I notice that these new files are very small. Could they be merged in a single file?

The other comment is related to plonk/circuit.rs. To me it feels strange to move the types Constraint, Constraints, SelectorsToFixed, Gate and so on, which are types directly used by the ConstraintSystem at plonk/circuit/constraint_system.rs into plonk/circuits.rs which is a more general module. In my mind, the more general things are in less deep modules, and more specific things are in more deep modules; so it's strange to see that types that are used specifically by plonk/circuit/constraint_system.rs are found in plonk/circuit.rs. I think I would prefer moving those to plonk/circuit/constraint_system/foo.rs instead.

Please let me know your thoughts about these two points. To me the second one is more important to discuss and resolve; the first one could be more related to personal taste.

[guorong009]

Most of the changes look good to me but I have two comments:

The first is that I see the new files column.rs, phase.rs and query.rs which have definitions that were previously in expression.rs. I notice that these new files are very small. Could they be merged in a single file?

The other comment is related to plonk/circuit.rs. To me it feels strange to move the types Constraint, Constraints, SelectorsToFixed, Gate and so on, which are types directly used by the ConstraintSystem at plonk/circuit/constraint_system.rs into plonk/circuits.rs which is a more general module. In my mind, the more general things are in less deep modules, and more specific things are in more deep modules; so it's strange to see that types that are used specifically by plonk/circuit/constraint_system.rs are found in plonk/circuit.rs. I think I would prefer moving those to plonk/circuit/constraint_system/foo.rs instead.

Please let me know your thoughts about these two points. To me the second one is more important to discuss and resolve; the first one could be more related to personal taste.

Thanks for suggestions! @ed255

1. I believe it is right to create less new files.
   Hence, I remove 3 new files & create plonk/circuit/helpers.rs file for Expression -related contents.

2. I also think you are right.
   FYI, I originally moved out all of those structs to plonk/circuit.rs,
   because they are also exported & used in halo2_proofs crate.
   But, it seems that there is no need.
   I create a new file plonk/circuit/constraint_system/helpers.rs for ConstraintSystem-related contents.

Please check new updates & give more feedback.

[ed255]

I'm not 100% convinced about naming the new modules "helpers", as it looks like a generic placeholder, but I can't think of anything better 🤔
Apart from that, LGTM!

[guorong009]

Thanks for the review! @ed255

I think this PR is a bit awkward since it tries to split the code in unnatural way.
It just aims to reduce the LOC, and breaks the cohesion. Plus, a new structure is not that good.
How about closing the PR, without merge? (Maybe, close the issue also?)

[ed255]

I think this PR is a bit awkward since it tries to split the code in unnatural way. It just aims to reduce the LOC, and breaks the cohesion. Plus, a new structure is not that good. How about closing the PR, without merge? (Maybe, close the issue also?)

Closing the PR and the issue sounds good to me!

[guorong009]

Close the PR since it leads to unnatural, incoherent refactoring.
Plus, close #297 as resolved.