Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travis: use newer versions #2908

Closed
wants to merge 3 commits into from

Conversation

felixn
Copy link
Contributor

@felixn felixn commented Sep 13, 2020

During investigation of #2806 (comment), I noticed that the Cpp runtime tests were failing locally (with clang 10) with the following errors:

In file included from /tmp/BaseCppTest-main-1600443341009/Test.cpp:3:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/antlr4-runtime.h:126:
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/misc/InterpreterDataReader.h:20:5: error: call to implicitly-deleted default constructor of 'dfa::Vocabulary'
    InterpreterData() {}; // For invalid content.
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:25:5: note: explicitly defaulted function was implicitly deleted here
    Vocabulary() = default;
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:185:36: note: default constructor of 'Vocabulary' is implicitly deleted because field '_literalNames' of const-qualified type 'const std::vector<std::string>' (aka 'const vector<basic_string<char> >') would not be initialized
    std::vector<std::string> const _literalNames;
                                   ^
1 warning and 1 error generated.
In file included from /tmp/BaseCppTest-main-1600443341009/TBaseVisitor.cpp:5:
In file included from /tmp/BaseCppTest-main-1600443341009/TBaseVisitor.h:7:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/antlr4-runtime.h:32:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/LexerInterpreter.h:10:
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:25:5: warning: explicitly defaulted default constructor is implicitly deleted [-Wdefaulted-function-deleted]
    Vocabulary() = default;
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:185:36: note: default constructor of 'Vocabulary' is implicitly deleted because field '_literalNames' of const-qualified type 'const std::vector<std::string>' (aka 'const vector<basic_string<char> >') would not be initialized
    std::vector<std::string> const _literalNames;
                                   ^

The travis builds were not showing the same errors, since they were running with clang-3.7 / g++-5.

This PR updates the Ubuntu version used for the travis builds to focal / 20.04, clang / g++ 10, and openjdk11, php7.4, and python3.8.

@felixn felixn marked this pull request as draft September 13, 2020 20:22
@felixn felixn force-pushed the travis-use-newer-versions branch 3 times, most recently from e243d4a to 36a99ff Compare September 18, 2020 14:02
@felixn
Copy link
Contributor Author

felixn commented Sep 22, 2020

@IohannRabeson:
Could you please take a look at the Travis failures of the Cpp builds/tests?
https://travis-ci.org/github/antlr/antlr4/builds/728336025

The following errors about dfa::Vocabulary pop up:

In file included from /tmp/BaseCppTest-main-1600443341009/Test.cpp:3:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/antlr4-runtime.h:126:
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/misc/InterpreterDataReader.h:20:5: error: call to implicitly-deleted default constructor of 'dfa::Vocabulary'
    InterpreterData() {}; // For invalid content.
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:25:5: note: explicitly defaulted function was implicitly deleted here
    Vocabulary() = default;
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:185:36: note: default constructor of 'Vocabulary' is implicitly deleted because field '_literalNames' of const-qualified type 'const std::vector<std::string>' (aka 'const vector<basic_string<char> >') would not be initialized
    std::vector<std::string> const _literalNames;
                                   ^
1 warning and 1 error generated.
In file included from /tmp/BaseCppTest-main-1600443341009/TBaseVisitor.cpp:5:
In file included from /tmp/BaseCppTest-main-1600443341009/TBaseVisitor.h:7:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/antlr4-runtime.h:32:
In file included from /home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/LexerInterpreter.h:10:
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:25:5: warning: explicitly defaulted default constructor is implicitly deleted [-Wdefaulted-function-deleted]
    Vocabulary() = default;
    ^
/home/felix/src/antlr4/runtime-testsuite/target/classes/Cpp/runtime/src/Vocabulary.h:185:36: note: default constructor of 'Vocabulary' is implicitly deleted because field '_literalNames' of const-qualified type 'const std::vector<std::string>' (aka 'const vector<basic_string<char> >') would not be initialized
    std::vector<std::string> const _literalNames;
                                   ^

I can reproduce the errors locally by running (with clang 10):

cd runtime-testsuite
mvn -q -Dgroups="org.antlr.v4.test.runtime.category.ParserTests" -Dtest=cpp.TestParserExec test

The issue seems to be related to your PR #2839,
because when I revert to a revision older than your changes (for example f68c47a), the test runs successfully.

@IohannRabeson
Copy link
Contributor

IohannRabeson commented Sep 22, 2020 via email

@felixn felixn force-pushed the travis-use-newer-versions branch 2 times, most recently from b86e5c7 to fb43f64 Compare November 25, 2020 17:48
@parrt parrt mentioned this pull request Nov 26, 2020
@felixn felixn marked this pull request as ready for review November 27, 2020 23:18
@felixn
Copy link
Contributor Author

felixn commented Nov 27, 2020

This is ready to merge, from my point of view.
@parrt - are you ok with the upgrades to the Travis build environment?
@mike-lischke / @IohannRabeson - are you ok with the changes to the C++ runtime?

@IohannRabeson
Copy link
Contributor

@felixn The c++ side looks ok to me. That said I never been able to reproduce the behavior you mentioned before though, with clang 10 or g++, either on OSX or on Linux.

@ericvergnaud
Copy link
Contributor

This is ready to merge, from my point of view.
@parrt - are you ok with the upgrades to the Travis build environment?
@mike-lischke / @IohannRabeson - are you ok with the changes to the C++ runtime?

Hi, I don't think we're ok with the scope of changes in travis build.
As of writing we officially support antlr-tool starting from java 1.8 and antlr-runtime from java 1.7.
Not sure what benefit would be created by requiring java 11?

@felixn
Copy link
Contributor Author

felixn commented Nov 28, 2020

Thanks for the feedback. I wasn't sure what the right scope of changes was, so I started with "upgrade everything" as it's easy to roll back as requested ;)

Ideally, we'd test every target both with "oldest supported" and with "newest released", but that's a lot of additional testing.

So how about we only upgrade the linux c++ target, that way we have a mix of java8 (non-c++ targets) and java11 (linux c++ target)?

@mike-lischke
Copy link
Member

We've seen similar errors before and added fixes. I'm surprised you found more of that. But the fix seems ok.

However, I guess @parr is already through the release process and hence this fix won't make it into the 4.9 release.

@parrt
Copy link
Member

parrt commented Nov 28, 2020

Actually, I think we only require java7.

Can we fix C++ w/o touching Java?

@felixn
Copy link
Contributor Author

felixn commented Dec 2, 2020

Hi @ericvergnaud / @parrt / @mike-lischke :
I've created an alternative PR #2983, which only upgrades the C++ build environment and fixes the regression in the C++ runtime.
Could you please check it out?

@parrt
Copy link
Member

parrt commented Dec 2, 2020

closing in favor of the new one

@parrt parrt closed this Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants