Defunctionalization with simple-sub Control-flow Analysis #222
Conversation
compiler/shared/main/scala/mlscript/compiler/simpledef/Simpledef.scala
Outdated
Show resolved
Hide resolved
compiler/shared/main/scala/mlscript/compiler/simpledef/Simpledef.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for the PR!
Please move all the test files from Simpledef back to Defunctionalize, so we get a more useful diff.
It seems you haven't carefully reviewed your tests. They contain lots of "Uncaught error" instances that don't seem justified.
File Simpledef.scala is completely missing any sort of documentation. At the very least give some vey high-level introduction at the beginning and explain what the information stored in each mutable map is.
compiler/shared/main/scala/mlscript/compiler/debug/MonomorphError.scala
Outdated
Show resolved
Hide resolved
compiler/shared/main/scala/mlscript/compiler/simpledef/Simpledef.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Not a fan of what you did to the class lifter tests – sweeping under the rug the fact they are ill-formed (produce runtime crashes) and that the lifter doesn't propagate type annotations when it should. Nevertheless, let's just get this merged ASAP after you address my last comments below.
compiler/shared/main/scala/mlscript/compiler/simpledef/Simpledef.scala
Outdated
Show resolved
Hide resolved
compiler/shared/main/scala/mlscript/compiler/simpledef/Simpledef.scala
Outdated
Show resolved
Hide resolved
|
I actually can't tell why you need |
|
After examining the code more, I've deemed the flag unnecessary. I've moved all defunctionalization to the (now separate) post-typing process. |
This PR adds an implementation of defunctionalization for selection on objects that uses simple-sub for control flow analysis.