C++ target: don't generate extra/duplicate semicolons

[felixn]

Currently, the generated C++ source code contains sporadic duplicate semicolons, e.g.:

Parser::FactorContext *factorContext = nullptr;;
std::vector<FactorContext *> f;;

This requires the use of -Wno-extra-semi with -Werror.

This PR removes the duplicate semicolons.
(Full disclosure: I couldn't figure out where the second semicolon is generated...)

[felixn]

Ping @mike-lischke

[mike-lischke]

I have worked on that as well in past. The problem here is the recursive nature of these ST rules. I couldn't solve the double semicolons.

I can promise you that the removal of the few semicolons as in your patch won't provide a complete solution. You will now end sometimes with no semicolon at all for certain statements. Unfortunately, I can't say for which. You should definitely test your patch with a complex grammar (say CPP14 or the complete MySQL grammars) to see if something is missing.

[felixn]

Hi Mike,
good suggestion about testing with complex grammars!
I tested with a C++14 grammar and a MySQL grammar, and I don't notice any regressions.
The PR seems to replace all double semicolons with single semicolons, while not removing any single semicolons.

I tested with this C++14 grammar: https://github.com/antlr/grammars-v4/blob/master/cpp/CPP14.g4

$ java -Xmx1024m -jar /antlr-4.8-complete.jar -Dlanguage=Cpp -listener -visitor -o orig CPP14.g4
$ java -Xmx1024m -jar antlr4-4.8-2-SNAPSHOT-complete.jar -Dlanguage=Cpp -listener -visitor -o mod CPP14.g4
$ diff orig/ mod/
diff orig/CPP14Parser.h mod/CPP14Parser.h
3214c3214
<     antlr4::Token *val = nullptr;;
---
>     antlr4::Token *val = nullptr;

And with this MySQL grammar: https://github.com/mysql/mysql-workbench/tree/8.0/library/parsers/grammars

$ java -Xmx1024m -jar antlr-4.8-complete.jar -Dlanguage=Cpp -listener -visitor -o orig -package parsers MySQLLexer.g4 MySQLParser.g4
$ java -Xmx1024m -jar antlr4-4.8-2-SNAPSHOT-complete.jar -Dlanguage=Cpp -listener -visitor -o mod -package parsers MySQLLexer.g4 MySQLParser.g4
$ diff orig/ mod/
diff orig/MySQLParser.h mod/MySQLParser.h
2173c2173
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
2231,2232c2231,2232
<     antlr4::Token *option = nullptr;;
<     antlr4::Token *security = nullptr;;
---
>     antlr4::Token *option = nullptr;
>     antlr4::Token *security = nullptr;
2259c2259
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
2427c2427
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
2792c2792
<     antlr4::Token *algorithm = nullptr;;
---
>     antlr4::Token *algorithm = nullptr;
2830,2831c2830,2831
<     antlr4::Token *timing = nullptr;;
<     antlr4::Token *event = nullptr;;
---
>     antlr4::Token *timing = nullptr;
>     antlr4::Token *event = nullptr;
2861c2861
<     antlr4::Token *ordering = nullptr;;
---
>     antlr4::Token *ordering = nullptr;
3071c3071
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
3146c3146
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
4705c4705
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
4724c4724
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
5433c5433
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
5909c5909
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
6178c6178
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
6510c6510
<     antlr4::Token *object = nullptr;;
---
>     antlr4::Token *object = nullptr;
6601c6601
<     antlr4::Token *element = nullptr;;
---
>     antlr4::Token *element = nullptr;
6620c6620
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
6701c6701
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
6791,6792c6791,6792
<     antlr4::Token *action = nullptr;;
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *action = nullptr;
>     antlr4::Token *type = nullptr;
7059,7060c7059,7060
<     antlr4::Token *value = nullptr;;
<     antlr4::Token *object = nullptr;;
---
>     antlr4::Token *value = nullptr;
>     antlr4::Token *object = nullptr;
7248c7248
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
7408c7408
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
7907c7907
<     antlr4::Token *op = nullptr;
---
>     antlr4::Token *op = nullptr
7934c7934
<     antlr4::Token *type = nullptr;
---
>     antlr4::Token *type = nullptr
7951c7951
<     antlr4::Token *op = nullptr;
---
>     antlr4::Token *op = nullptr
8161c8161
<     antlr4::Token *op = nullptr;;
---
>     antlr4::Token *op = nullptr;
8253c8253
<     antlr4::Token *op = nullptr;
---
>     antlr4::Token *op = nullptr
8573c8573
<     antlr4::Token *name = nullptr;;
---
>     antlr4::Token *name = nullptr;
8818c8818
<     antlr4::Token *name = nullptr;;
---
>     antlr4::Token *name = nullptr;
8914c8914
<     antlr4::Token *name = nullptr;;
---
>     antlr4::Token *name = nullptr;
10358c10358
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
10437c10437
<     antlr4::Token *value = nullptr;;
---
>     antlr4::Token *value = nullptr;
10535,10536c10535,10536
<     antlr4::Token *match = nullptr;;
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *match = nullptr;
>     antlr4::Token *option = nullptr;
10674c10674
<     antlr4::Token *algorithm = nullptr;;
---
>     antlr4::Token *algorithm = nullptr;
10810c10810
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
10882c10882
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
10900c10900
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
11110,11112c11110,11112
<     antlr4::Token *option = nullptr;;
<     antlr4::Token *format = nullptr;;
<     antlr4::Token *method = nullptr;;
---
>     antlr4::Token *option = nullptr;
>     antlr4::Token *format = nullptr;
>     antlr4::Token *method = nullptr;
11449c11449
<     antlr4::Token *option = nullptr;;
---
>     antlr4::Token *option = nullptr;
11583c11583
<     antlr4::Token *type = nullptr;;
---
>     antlr4::Token *type = nullptr;
13020c13020
<     antlr4::Token *value = nullptr;;
---
>     antlr4::Token *value = nullptr;

[mike-lischke]

OK, then let's hope we covered all relevant cases in the tests.

[felixn]

Thanks for the positive feedback.
@mike-lischke or @parrt - could you please trigger the merge? I don't have write access.

[parrt]

some tests are failing but I think it is just DART. I added an issue for them to fix that so we can proceed. it might be hiding an error here. #2881

[parrt]

Okay, we have the fix. I think I have to kick off another build

[parrt]

Hmm...i've pulled in a fix for dart from @lingyv-li but can't seem to kick off a new build for this PR. anybody know how?

[felixn]

I just rebased my branch, that hopefully triggers a new build

[felixn]

Thanks Terence!

[ericvergnaud]

This change breaks the build, see https://travis-ci.org/github/antlr/antlr4/jobs/726751569
@mike-lischke @felixn Please revert or fix as we no longer have a green build

[mike-lischke]

Now that's odd. Why has this PR no test run, actually? Looks like Travis never ran for this patch o_O

And what's even worse is that this patch is already 24 days old. In the meantime we had quite a few other merges.

[felixn]

It looks like the Github project is only configured to show the appveyor builds, not the travis builds...
All commits on master as well as all pull requests have a green checkmark because the appveyor builds pass, even though all the travis builds fail.

I'm fine with reverting this until I can investigate, if that's the quickest way to fix this.
I currently can't run the testcases locally, due to other errors:
InterpreterDataReader.h:20:5: error: call to implicitly-deleted default constructor of 'dfa::Vocabulary'

[ericvergnaud]

@parrt seems only you can fix the travis status no longer shown in GitHub PRs, cox it requires to be admin of the travis project
See https://travis-ci.community/t/github-pr-is-being-built-but-result-is-not-shown/7025/2 which is what we are experiencing

[parrt]

I wondered why those builds weren't shown. shit. i revoked and now can't get travis back to github. ugh.

[ericvergnaud]

suspect GitHub changed their security policies

[parrt]

i'm very confused. seems they are migrating us to .com from .org or were we doing .com before? i can't find travis-ci.org -> github integration instructions.

[parrt]

Builds show up on .org site so that's what we were using.

[ericvergnaud]

we were definitely on .org

[parrt]

ok, i think i have it reinstalled.

[felixn]

I'm looking into the regression due to this PR.
I think #2839 also introduced a regression, at least that what it looks like locally...

[parrt]

ok, can we kick off a build from a PR to see if it updates the github page?

[felixn]

Started an empty PR here: #2907
Looks like the travis build is being shown!

[ericvergnaud]

Thanks @parrt !

[mike-lischke]

I currently can't run the testcases locally, due to other errors:
InterpreterDataReader.h:20:5: error: call to implicitly-deleted default constructor of 'dfa::Vocabulary'

Oh, can you please investigate. We had a patch recently which might have to do with that (just guessing here).

[felixn]

I currently can't run the testcases locally, due to other errors:
InterpreterDataReader.h:20:5: error: call to implicitly-deleted default constructor of 'dfa::Vocabulary'

Oh, can you please investigate. We had a patch recently which might have to do with that (just guessing here).

Will do!

The issue reported by @ericvergnaud should be fixed now with #2907.

The travis build is not showing the other issues I mentioned / am experiencing locally (due to #2839 ?), probably because we're running an extremely old version of GCC (version 5) on an extremely old version of Ubuntu (14.04) for the travis build. I'll create a separate pull request to use newer versions for the travis build, we'll see if the issues show up then.