Quick code review

[anhnamtran]

Format:

filename: [start line, end line] ...

or

filename: method_name

• [start line, end line]

Overall a few places are undocumented.

• [x] main.py: [68, 69], [73], [151, 159]
  • Commented code should either be removed or documented.
• [x] main.py: [88, 131]
  • I'm assuming that this generates the .synthesized files used for load results. If that's true then maybe a little bit of documentation to clarify.
  • I think this entire block would benefit from a bit more documentation.
• [x] invariant_preservation.py: [42, 44]
  • Commented code should either be removed or documented.
• [x] pools.py: [11]
  • There's a TODO.
• [x] syntax_tools.py: free_vars
  • [503, 576]: An explanation on how this loop works would be really useful.
• [x] syntax_tools.py: [1158]
  • There's a TODO.
• [x] Some method def's have type restrictions when some others don't. Intentional?
• [x] cost_model.py: [59]
  • Variable named "a". Assuming that it stands for all?
• [x] cost_model.py: [88, 91]
  • Why chose TWO, FOUR, EIGHT, TWENTY?
• [x] cost_model.py: [141, 148]
  • Commented code should either be removed or documented.
• [x] cost_model.py: debug_comparison
  • If this is a debug method then it should probably be documented as such.
• [x] desugar.py: [1]
  • Minor typo in comment.
• [x] evaluation.py: [123]
  • There's a TODO.
• [x] evaluation.py: [829, 830]
  • Commented code should either be removed or documented.
• [x] solver.py: [164, 166], [178, 180], [723, 729], [776, 779], [810, 837], [1021, 1029], [1126, 1145], [1209, 1223]
  • Commented code should either be removed or documented.
• [x] solver.py: [373], [992], [995]
  • There are TODO's.
• [x] state_maintenance.py: [32, 40], [56], [155, 182]
  • TODO with comments.
• [x] state_maintenance.py: [100, 109]
  • Commented code should either be removed or documented.

[Calvin-L]

There are a few things I am going to leave as-is.

Some method def's have type restrictions when some others don't. Intentional?

The type restrictions are a form of documentation, and are generally only present on the most important functions. Note that they are not enforced unless the procedure is annotated with @common.typechecked, and even then, the type annotations are only enforced dynamically.

Why chose TWO, FOUR, EIGHT, TWENTY?

The cost model is sort of a black art. There are boatload of regression tests in tests/cost_model.py that describe "expected behavior" from the cost model, and these parameters are tuned to make it behave appropriately.