mal
mal copied to clipboard
[RFC] Add Hack implementation
Hi, I've made a type-based implementation for Hack.
It should be idiomatic Hack.
I haven't created the Docker image, as that's not a fun step, so I'd like to get a go-ahead on this PR before I invest my time in that.
I also plan to add a non-type-based implementation, which would be less educational but more performant (it wouldn't wrap Hack values in objects, it would likely be close to the PHP implementation).
Also I have tried to keep a clear history of commits, so that someone could follow step by step (not just via files), but I can squash them if that's preferred.
Heya, let me know if there's a will to merge this and I'll work on the docker image. Thanks
@xixixao Yes, I would be interested in merging this.
There is some refactoring/cleanup that will need to happen first. The commit history is fine, but structure of the files needs to show the progression. Somebody wanting to learn from the implementation should be able to look at step3 and easily be able to identify the code that contributes directly to implementing step3 functionality. The use of autoload.hack
and the common eval.hack file obscure this. The clear delineation is less important for some of the lower-level target implementation specific details (the type of stuff in exceptions.hack
and types.hack
). However, the code in eval.hack
has a direct link to the implementation steps so it needs to be captured with its implementation. This does feel counter to normal develop practices (to not repeat identical code) but the goal of mal is educational for future reference and not just the implementer. More rationale is given in the FAQ https://github.com/kanaka/mal/blob/master/docs/FAQ.md#will-you-add-my-new-implementation You done this to some degree in the commit history, but that gets messy over time as fixes are applied and there aren't good processes/tools for running regressions tests and/or comparing differently named files across a git history. For similar reasons I think mal files should depend on other mal files explicitly so the code dependency are explicit to somebody trying to understand the code instead of automatic (again, a bit contrary to what you would do for a production non-teaching project).
Does the implementation self-host successfully?
Let me know when you have a Dockerfile that is ready for me to be build and pushed to docker hub. Once that happens then the test for this can be activated in Travis. With the changes I described above and passing tests in Travis then I'll do a bit deeper code review to see if it's ready for merging.
Thanks!