Add "contextSuperClass" option to rule. Will overlap "contextSuperClass" option in grammar if both exist.

[XenoAmess]

trafficstars

as title.

[parrt]

Hi. this is hard to read because some whitespace changes makes it look like everything is different.

[XenoAmess]

Hi. this is hard to read because some whitespace changes makes it look like everything is different.

@parrt

Hi. any suggestions on this?

Because we really need this function in my company.

Now I made a 3rd-time maintaince in my group's nexus, but I guess that should not be a long time way.

It runs smothly these months(nearly 1 year) anyway...

[parrt]

Hmm..well you'd have to rebase on dev to bring up to date, but what problem are you solving? I.e., why do we need this? :)

[XenoAmess]

What problem are you solving? I.e., why do we need this? :)

Basically to inject some functions onto each context.

For example I injected a function String entitify(); onto each context, on the parent class layer, to make each context object have the ability to entitify themselfs, instead of 100+ additional util classes for each context type.

[XenoAmess]

What problem are you solving? I.e., why do we need this? :)

Basically to inject some functions onto each context.

For example I injected a function String entitify(); onto each context, on the parent class layer, to make each context object have the ability to entitify themselfs, instead of 100+ additional util classes for each context type.

and also fields.

[XenoAmess]

@parrt rebeased.

[parrt]

@ericvergnaud what are your thoughts on adding a super class for rule parts nodes? I can see how in some cases this would be useful, but this is the first time I think I've noticed anybody asking for it. We would also have to create tests for this extra functionality.

@XenoAmess wouldn't it be simpler just to have an extra function to do your entity() call? I guess the negative would be you couldn't add fields. Why not just to find your own Super class and stick that first in the CLASSPATH?

Basically what it comes down to is I'm managing so many open source projects that I can't even handle it working all weekend and on Fridays for my 20% project at work. I'm trying to avoid having to think too much about all new changes etc..

[ericvergnaud]

@parrt Here is my understanding of how things look like currently (expressed in Java)

class MyParserRuleContext extends RuleContext {
 
Object fieldA;
Object fieldB;

 void someUtilityMethodA() { ... }
 void someUtilityMethodB() { ... }
}

class RuleAContext extends MyParserRuleContext {
}

class RuleBContext extends MyParserRuleContext {
}

Here is how it would look if we merge the PR:

class MyParserRuleAContext extends RuleContext {
 
Object fieldA;

 void someUtilityMethodA() { ... }
}

class MyParserRuleBContext extends RuleContext {
 
Object fieldB;

 void someUtilityMethodB() { ... }
}

class RuleAContext extends MyParserRuleAContext {
}

class RuleBContext extends MyParserRuleBContext {
}

@XenoAmess please confirm ?

IMHO the risk is low since it only touches the tool, not the runtimes. There is some benefit because the latter is more accurate. But it's low because the MyParserRuleAContext knows nothing about RuleAContext without downcasting (which is easy in Java/C#, painful in Python, and horrible in TypeScript).

I agree that it needs to be tested, and a test would actually help assess the benefits/risk ratio. @XenoAmess would you be kind enough to add a test template such that we can prove that this works across all targets ? See https://github.com/antlr/antlr4)/runtime-testsuite

[parrt]

From what grammar are you "generating" that Java, @ericvergnaud ? From compilationUnit rule in Java.g4 I see:

public static class CompilationUnitContext extends ParserRuleContext {
    // label-derived fields here...
    public TerminalNode EOF() { return getToken(JavaParser.EOF, 0); }
     ...
}

Comment on context obj says:

Subclasses made for each rule and grammar track the parameters, return values, locals, and labels specific to that rule. These are the objects that are returned from rules.

So this would be adding the ability for us to change the generator code to be this?

public static class CompilationUnitContext extends MY_CONTEXT {
...
}

Is it that common to introduce virtual methods based upon the node type? Fields? If Fields, just add another argument or local to the rule:

grammar T;

start[int x] : A B ;

public static class StartContext extends ParserRuleContext {
        public int x;

There's no need to modify the code generation for fields.

As for functions, I can see the introduction of virtual methods being occasionally useful, but we already have listeners and visitors for this that don't embed the actions into the parse tree. This was a deliberate separation of concerns on our part.

In summary, I'm hesitant to merge this.

[ericvergnaud]

This was hand-written and it should indeed read ...extends ParserRuleContext, sorry for the confusion

[XenoAmess]

I guess the negative would be you couldn't add fields.

Yep. we wanna fields, and actually different field for defferent Context class...

[XenoAmess]

@parrt Here is my understanding of how things look like currently (expressed in Java)

class MyParserRuleContext extends RuleContext {
 
Object fieldA;
Object fieldB;

 void someUtilityMethodA() { ... }
 void someUtilityMethodB() { ... }
}

class RuleAContext extends MyParserRuleContext {
}

class RuleBContext extends MyParserRuleContext {
}

Here is how it would look if we merge the PR:

class MyParserRuleAContext extends RuleContext {
 
Object fieldA;

 void someUtilityMethodA() { ... }
}

class MyParserRuleBContext extends RuleContext {
 
Object fieldB;

 void someUtilityMethodB() { ... }
}

class RuleAContext extends MyParserRuleAContext {
}

class RuleBContext extends MyParserRuleBContext {
}

@XenoAmess please confirm ?

IMHO the risk is low since it only touches the tool, not the runtimes. There is some benefit because the latter is more accurate. But it's low because the MyParserRuleAContext knows nothing about RuleAContext without downcasting (which is easy in Java/C#, painful in Python, and horrible in TypeScript).

I agree that it needs to be tested, and a test would actually help assess the benefits/risk ratio. @XenoAmess would you be kind enough to add a test template such that we can prove that this works across all targets ? See https://github.com/antlr/antlr4)/runtime-testsuite

Yep your demo is quite like what we doing.

I think I can manage some time for learning about the test suite at 2023.1 when lunar new year vacation.

[parrt]

I think my point is that you don't need any changes. Just add args and you have specific fields.

[XenoAmess]

I think my point is that you don't need any changes. Just add args and you have specific fields.

public field and access it using visitor classes outside of this class? sounds really weird.

[parrt]

Well, those fields are annotations on the tree and I think it's totally fine that they are visible to outside code. I am not a priest in the OO church, so we might have a difference of opinion here. I have never felt the need for accessor methods.

By definition for visitors are trying to separate the logic from the tree mechanism, which we did on purpose.

Anyway, thank you very much for your efforts and the discussion but I think I will close this approach.