Rewrite SubContext* rules and update tests

[jcjaskula-aws]

I added an additional argument to SubContextLocal and SubContextMember to avoid hardcoding indices in the test, i.e.

SubContextLocal({<Production("e")>(0)}, {<Result("v")>})>

To give more context, I have been able to create a runtime/generator for Julia. This is the only modification I need to push to the main Antlr4 repos to keep my implementation "standalone". The change will help me release my Julia runtime/generator eventually and I believe that it is transparent for all other target languages.

FYI, in Julia, I need to write SubContextLocal as:

SubContextLocal(ctx, subctx, local, arg) ::= "<subctx>(<ctx>, <arg>).<local>"

[jcjaskula-aws]

Hi @parrt, would it be possible to get someone to review this PR? I could not find a better way to ask for a review. Thank you!

[parrt]

Unfortunately I can't accept any more new targets to main repo; it's hard enough managing what I've got. especially since I just don't have time for this project at the moment. So, as it is on the way to accepting a new target, probably not worth me digging into this. Sorry!!

[jcjaskula-aws]

I have seen previous messages mentioning you don't want to add new target. I was not expecting the Julia target to be added to the main repo and I totally understand why.

However, these changes are minor (essentially 2-line modifications repeated across several targets) and they are eventually target-agnostic. It would help me keeping a sane repository that would contain only the Julia target and no antlr4 patch such that I can rebase with no conflict.

This could be reviewed by any co-maintainer that is in charge of targets so I'll leave this PR open. But I'm also ready to disagree and commit if this change is really a no-no.

[jimidle]

I took a look and it seems fairly innocuous, but before I look further, I would like to know a bit more about why you have to do this for Julia and cannot find any other way? Also, hard coding for tests isn't the end of the world. But can you add a comment here that explains exactly why you have to do this? It may be a quirk of Julia, but requiring changes in other target templates usually does not pass the sniff test. But right now, there is no context for me to see why this is the only solution in Julia.

[jcjaskula-aws]

Hey @jimidle, thanks for your reply. Let me take a different perspective on

It may be a quirk of Julia, but requiring changes in other target templates usually does not pass the sniff test.

AFAIU, the tests that I modified are supposed to be target- and implementation-agnostic but they are not eventually. More specifically, <Production("e")>(0) in

: e '*' e     {$v = <Cast("BinaryContext","$ctx"):SubContextLocal({<Production("e")>(0)}, {<Result("v")>})> * <Cast("BinaryContext","$ctx"):SubContextLocal({<Production("e")>(1)}, {<Result("v")>})>;}  # binary

implies that (i) <Production("e")> is callable/subscriptable, (ii) that 0 can be used as an index (and (iii) that parentheses are usable in this context). So to some extent, these tests are eventually forcing a certain implementation in other (current or future) targets. I believe that this PR making the tests more agnostic.

That said, Julia is not oriented object and it is not natural to attach functions to Struct. So taking an example for the Python runtime tests:

class AddSubContext(ExprContext):
    def expr(self, i:int=None):
        if i is None:
            return self.getTypedRuleContexts(ExprParser.ExprContext)
        else:
            return self.getTypedRuleContext(ExprParser.ExprContext,i)

# is called by AddSubContext(...).expr(i)

to

expr(ctx::AddSubContext, i::Nothing=nothing) =
    Antlr4Runtime.getTypedRuleContexts(ctx, AbstractExprContext)
expr(ctx::AddSubContext, i::Int) =
    Antlr4Runtime.getTypedRuleContext(ctx, AbstractExprContext, i)

# is called by expr(AddSubContext(...), i)

Ultimately, I could potentially find a way to make expr(::AddSubContext) returns a callable taking an argument of type Nothing or Int, which would look like expr(add_context)(i). I was probably not considering the option when I wrote the Julia target a year ago since I was trying to stay close to existing targets where expr takes an index argument. My feeling is also that the Julia JIT compiler will produce something better code but I didn't test it.

Worth mentioning I am not requesting changes of the other target generators but only test generators (I would have been much more uncomfortable to request changes on the <target>.stgs).

[jimidle]

I will need to look at why you need test changes for other targets though. It does not seem natural. I will try to give it some time and give Terence my recommendation.

[jcjaskula-aws]

Sure, feel free to take your time. There is no rush as I won't probably be able to release the Julia target within weeks.
Edit: also, I don't change the tests, simply the test templates. Eventually, what these templates print is the same, and the tests are consequently unchanged.