Closed j-hui closed 3 years ago
Sounds like a good idea, especially as the project is expected to grow more.
I don't think we can put Trace.hs
under test/
though, as the interpreter depends on it (the output of the interpreter is Output
from Trace.hs
).
A point along the same line is the naming of the modules. A lot of them were initially placeholders. E.g we had another code generator before, but when I redesigned it such that we generate code from the LowCore.hs
statements, I just created a LowCodeGen.hs
. Now that is the only code generator, so a more suitable name is perhaps CGen
or something.
I was going to suggest we move the interpreter to the test lib too, because I don't think it belongs in the edsl directory either. At least for the moment, it isn't used for anything other than testing, and even if it were to do something outside of testing, it shouldn't return Output
either.
I think that if I want to run a program quickly on the interpreter to try out some semantics, I don't necessarily want to load the testing stuff into my REPL. In this case, it perhaps makes more sense to change the output of the interpreter instead and then leaving it outside of test/
, while we can move Trace.hs
to test/
.
Point well taken. When we called and spoke about this earlier, I pointed out that the interpreter as it is currently written is hightly specific to testing, since its return type is just Output
. But having a user-facing interpreter is certainly handy. Perhaps the interpreter could be parametrized by some SsmDebug
typeclass to define exactly what information is emitted during evaluation.
Returning to the matter of module organization: another strategy could be to keep everything under edsl
, but to place library helpers under a Test/Ssm
hiearchy so that their purpose is clear, and they stay out of the way of the core compiler pipeline. Then the test suites will be just a series of Spec.hs
entry points and individual test cases that call into Test.Ssm.*
. This roughly corresponds to the "God" library strategy discussed in the haskellforall blog post.
PR #29 implements a fix for the module structure itself.
Closed by #29
This is more of a nit than an actual technical issue, but I think it would help us better navigate this codebase if we were to adopt a hierarchical module organization typical of other Haskell libraries (e.g., putting
Core.hs
's AST underSsm.Core.Ast
). There are also several modules underedsl
that are rather specific to testing, such asTrace.hs
and the Quickcheck instances; it would be great to move these under the hierarchy starting attest/lib
. This can also help people browsing this code what the pipeline of the compiler looks like, without having to grep around.The only written opinion on this issue that I could find is this fairly recent blogpost: https://www.haskellforall.com/2021/05/module-organization-guidelines-for.html .
For reference,
graphmod
reports that the repo currently looks something like this: