Closed rkrishnasanka closed 1 year ago
@burakemir Let me know if this sounds good ? I can start on the PR this weekend
I don't currently see the advantages of either of these. Let's not do this.
In case a new antlr version generated code that breaks someone, and we check that in, how is this preventing breakage? Is the idea that it wouldn't even get checked in? In any case, there is actually a CI system in place where code it regularly generated, built, tested. I'd expect changes of minor version to be tested carefully and not cause breakages, and a major upgrade is an altogether different story. I don't see the point of generated sources checked in, and there are actually conditions in place that would prevent me from doing so.
for switching to listeners instead of visitors, I think it is very nice that right now there is explicit control over what to visit when. I also think it is valuable to be able to change the structure of the result from the parse tree, and I'd imagine a listener would require manually keeping track of stack. How does the use of listeners make things better/easier?
Without including the antlr generate code, it becomes really hard to use this package as a dependency. Go modules assume that all Go code you need to build them is included when you go get
the code and it is standard practice for a Go module to include their generated sources. Lots of things break when this is not the case, for example, go mod tidy
is unable to run:
example.com/mypackage imports
github.com/google/mangle/parse imports
github.com/google/mangle/parse/gen: module github.com/google/mangle@latest found (v0.0.0-20230707132936-395177d4b5ed), but does not contain package github.com/google/mangle/parse/gen```
Makes sense. I will check in the sources.
I've been sneaking peeks at the source code and here are a couple of things we need to the repo:
These were some of the lesson's I've learnt when using ANTLR for my Ph.D. projects since I heavily relied on ANTLR for generating the parsers. If we don't port it to visitors right now, we will have a lot more trouble in the future as the execution logic will become more complicated and become quite a pain in the ass to rewrite.