Strumenta / antlr-kotlin

Support for Kotlin as a target for ANTLR 4
Apache License 2.0
229 stars 47 forks source link

Predicate Reference To Token Generates Invalid Code #162

Closed piacenti closed 9 months ago

piacenti commented 10 months ago

I'm dealing with a complex piece of text to parse that requires usage of predicates. I ended up finding this when referencing a token inside a predicate image

I java it just creates a token variable which this attempts to do while also making it an extension property also which is invalid.

In addition to that, when you try fixing that problem in line you will be met with another one that affects that JS target image

piacenti commented 10 months ago

first issue seems to come from this method in the target code:

  override fun getTokenTypeAsTargetLabel(g: Grammar, ttype: Int): String {
    // All tokens are namespaced inside a Tokens object.
    // Here we simply force the qualification
    val label = super.getTokenTypeAsTargetLabel(g, ttype)
    return "Tokens.$label"
  }

looks like java uses this for both token reference as well as the local variable generation which works fine for java but not here since the way that data is accessed is a little different

lppedd commented 10 months ago

@piacenti this has been solved by @ftomassetti for ANTLR5.
@ftomassetti could you "backport" the change you did for the Tokens prefix? The one we discussed earlier on Slack.
Should be as straightforward as copying over the Kotlin.stg file (beware of the latest PRs tho), and removing the getTokenTypeAsTargetLabel from KotlinTarget.

lppedd commented 10 months ago

Basically my getTokenTypeAsTargetLabel change was a quick and dirty way of getting the namespaced tokens to work. We don't have yet a decent test coverage here, but on the ANTLR5 repo Federico has solved a bunch of template-related issues thanks to the official test suite (I want to emphasize the Kotlin code is exactly the same as of now).

piacenti commented 10 months ago

what is the antlr5 repo you are referring to? could you provide a link? I locally fixed the problem above and added some tests to make sure it works as expected with actions and predicates but I'm still working through something so if more problems come up I will add them up to a PR if not already solved by then.

ftomassetti commented 10 months ago

@piacenti

what is the antlr5 repo you are referring to? could you provide a link?

Here there is: https://github.com/antlr/antlr5

In this PR they are looking into merging the ANTLR Kotlin runtime in ANTLR 5: https://github.com/antlr/antlr5/pull/6

And in this PR on that PR I tried to fix the issues encountered in some tests: https://github.com/antlr/antlr5/pull/8

piacenti commented 10 months ago

Thanks, for some reason when I searched for that it didn't come up with that repo. Looks like it was recently created and is just a copy of ANTLR 4 at the moment with some updates, they even have the same number of commits and same tags. What are the plans around it? will it use a different algorithm like ANTLR4? From the open issues it sounds like they plan to possibly replace several runtimes with Kotlin but I don't see any other info around intentions for the project. And I'd assume that when things are integrated from this project, it will be archived, right?

ftomassetti commented 10 months ago

Answering from the phone but regarding ANTLR 5 you can look at the project antlr5-specs within the same organization to learn about its goals (in short: producing only webassembly and then wrappers around the single webassembly target). Development started very recently. I would expect it to be production ready in years. This means this project will be maintained for years to come. If anything we could get new ideas from ANTLR 5 and backport them to these projects when it makes sense (for example the fixes done for ANTLR 5)

lppedd commented 9 months ago

@piacenti let us know if it's all good on your side with 0.1.3.