Assert in ParserRuleContext.java makes copyFrom unusable

[fedorusov]

https://github.com/tunnelvisionlabs/antlr4/blob/269c382dd23ebccf0f987523205bf9fe6d72e021/runtime/Java/src/org/antlr/v4/runtime/ParserRuleContext.java#L143

If you call

context1.copyFrom(context2)

you necessarily call addAnyChild which have this assert, so it fails. Vanilla antlr doesn't have this, why do you?

[sharwell]

Can you provide an example showing how this code path is hit?

Vanilla antlr doesn't have this, why do you?

ANTLR made some breaking API changes during earlier releases, which are not allowed in this repository. It's possible this assertion was incorrectly translated during a merge, but I'm surprised I haven't heard about it sooner since this code is more than 2 years old.

[fedorusov]

Before, I used the old 4.5.3 version (without assertion) which occurred to have some performance problems in some cases, which ones are solved in the latest one. You don't need to have any concrete examples here, just try to copy any context and experience assertion error here because if context2 has children you call addAnyChild to context1, so t.getParent() != this.

[sharwell]

You don't need to have any concrete examples here, just try to copy any context ...

The context passed to copyFrom is not expected to have children. Can you clarify how it would end up with children prior to the copyFrom call?

[fedorusov]

with 4.5.3 I could have like

ctx1.copyFrom(ctx2);
for (child: ctx2.childrens)
  ctx1.addChild(child)

(I know parents are not reassigned here, but it's ok for me.) where copyFrom was

    public void copyFrom(ParserRuleContext ctx) {
        this.parent = ctx.parent;
        this.invokingState = ctx.invokingState;
        this.start = ctx.start;
        this.stop = ctx.stop;
    }

and addAnyChild didn't have assertion. But now I should have my own copyFrom and addAnyChild like it was before and update it every time when I update antlr to have the same code.

[sharwell]

@fedorusov Is it OK if I leave this one open? I can't promise it will change but I'd like to think about ways to improve the validation condition.

[fedorusov]

Sure