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

Use Kotlin core for java runtime #40

Merged
merged 32 commits into from
Feb 29, 2024

Conversation

ericvergnaud
Copy link
Contributor

This is far from being mergeable (many tests fail) but I thought it might be worth starting to look at the changes and provide comments.

@ericvergnaud ericvergnaud changed the title [WIP] Use core for java runtime [WIP] Use Kotlin core for java runtime Feb 25, 2024
Signed-off-by: Eric Vergnaud <[email protected]>

# Conflicts:
#	runtime-testsuite/test/org/antlr/v5/test/runtime/java/api/TestExpectedTokens.java
#	runtime/Java/src/main/java/org/antlr/v5/runtime/atn/ATN.java
#	runtime/Java/src/main/java/org/antlr/v5/runtime/atn/ATNDeserializer.java
#	runtime/Java/src/main/java/org/antlr/v5/runtime/atn/ATNState.java
#	runtime/Java/src/main/java/org/antlr/v5/runtime/atn/CodePointTransitions.java
#	tool/src/org/antlr/v5/automata/ATNOptimizer.java
#	tool/src/org/antlr/v5/automata/ATNVisitor.java
#	tool/src/org/antlr/v5/automata/ParserATNFactory.java
#	tool/src/org/antlr/v5/automata/TailEpsilonRemover.java
@lppedd
Copy link
Collaborator

lppedd commented Feb 25, 2024

Quickly skimmed through the changes. Since I suppose at some point ANTLR 5 will get rid of the Java runtime, I'd say it's all good as long as tests pass.

Just a couple of commits I'd revisit: e8ae531, 9c125e4
Better not make stuff nullable to avoid all those !! assertions. I recall in the Strumenta repository I used an ad-hoc object to replace a null value.

@ericvergnaud
Copy link
Contributor Author

ericvergnaud commented Feb 27, 2024

Quickly skimmed through the changes. Since I suppose at some point ANTLR 5 will get rid of the Java runtime, I'd say it's all good as long as tests pass.

Just a couple of commits I'd revisit: e8ae531, 9c125e4 Better not make stuff nullable to avoid all those !! assertions. I recall in the Strumenta repository I used an ad-hoc object to replace a null value.

Actually the nullables are required to make the tests pass. That said your suggestions are welcome and we might change that in favor of your proposed strategy. I'd be in favor of doing that in a separate PR such that we know what we're breaking and we can measure the impact on performance ? See #44

@ericvergnaud
Copy link
Contributor Author

@KvanTTT @lppedd All tests pass !!! Your comments are welcome. Changing the title of the PR.

@ericvergnaud ericvergnaud changed the title [WIP] Use Kotlin core for java runtime Use Kotlin core for java runtime Feb 27, 2024
@ericvergnaud
Copy link
Contributor Author

@KvanTTT given the broad scope of this PR, I had to merge your latest changes from dev into this branch. It wasn't seamless, so you might want to double-check the affected files.

@ericvergnaud
Copy link
Contributor Author

@KvanTTT any comments ?

@KvanTTT
Copy link
Member

KvanTTT commented Feb 29, 2024

No, I'm on vacation. Feel free to merge without my approval.

@ericvergnaud ericvergnaud merged commit e98089c into antlr:dev Feb 29, 2024
10 checks passed
@ericvergnaud ericvergnaud deleted the use-core-for-java-runtime branch February 29, 2024 13:52
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.

3 participants