Cleanup the runtime code

[lppedd]

Goals

• Improving the runtime performance by avoiding unnecessary (unchecked) casts, unnecessary non-null assertions, by using the right data structures (possibly the same used in the Java runtime) and immutable types
• Improving the development and debugging experience
• Improving the IDE navigation experience
• Easing the process of porting future changes

Non goals

It is a non goal to change the behavior of the code. It should possibly stay aligned with the Java runtime, line by line.

Methodology

Each commit is mapped to a runtime Kotlin package.

Each file is cleaned-up in multiple passes:

• A first pass is done using the IDE formatter
• A second pass is done by resolving all possible inspections
• A third pass is done by matching the file, line by line, with the Java runtime equivalent.
  If a method is missing (e.g., StringBuilder.appendCodePoint), it is implemented.

TODO

• [x] Calls to toHexString should be passed a custom hex format to align to the Java implementation.

  val hexFormat = HexFormat {
  upperCase = true
  number {
    prefix = "0x"
    removeLeadingZeros = true
  }
  }

• [x] Cleanup calls to System.out.println
  The Java runtime calls .format with a terminating \n, but we don't need that \n

[lppedd]

@ftomassetti I'm going to finish off the main package and then call it done.
There will be org.antlr.v4.kotlinruntime.atn still not cleaned up but I can do it in the future.

[ftomassetti]

I started taking a look at the code, but I am afraid I will not be able to do a proper review as there are too many changes. Also, I think this is code coming from the ANTLR Java runtime, so I assume that most of it will be correct. And I will rely on tests to catch blatant things

[lppedd]

Definitely don't review it in details, not worth it.
The code is a 1 to 1 mapping with the Java runtime (I'm doing it by reading each source file line by line), with some Kotlin-friendly code style changes.

[lppedd]

I'm almost done. Just missing a couple of big classes.

In the end I did also the atn package, as the type nullability is tied to the parser and lexer.

[lppedd]

Done!

Turns out there was code not on par with ANTLR 4.13.1.
I've also re-enabled functionalities which were commented out, such as ParserInterpreter, ATNSerializer, XPath* (although we still miss a multiplatform implementation for a couple of Java methods), and many others.

I've deleted two files, which were commented out completely:

• UnbufferedCharStream: pretty much unnecessary for us + relies on Java's Readers
• LogManager: unnecessary for us + relies on Java's Readers

We might be able to restore them at some point, but now it doesn't make sense.

[lppedd]

I'll do a couple tests (@ftomassetti you can run some tests on your grammars too if you want), update the branch with master's changes, and then mark it as ready to be merged.

Note that we now have a very strong type system, so a release with this code is not backward compatible.

[ftomassetti]

Note that we now have a very strong type system, so a release with this code is not backward compatible.

Makes sense, I think at this stage breaking changes have to be expected

[frett]

as someone using this in production multiplatform code I'm fine with the breaking changes since it's bringing it up to be more maintainable and Kotlin friendly :)

[lppedd]

Cool! Hopefully there won't be changes this big anymore.

We can merge this one and publish it as another RC, so you can also test it.
I'll try to integrate more grammar tests.

[lppedd]

The C++ grammar from the ANTLR4 grammars repository (which is enormous), compiles fine 🔝

The not-so-good news: the C++ generated parser (20000 LOC) crashes the Kotlin compiler lol

[lppedd]

Managed to get it to run by replacing the scoped { } inline function with if (true).

C++ tests run fine and pass. I'll open a YouTrack issue once this is merged, since this is valid for K2 too.

[lppedd]

I've inspected the outputted JS code and the good news is the K/JS compiler optimizes the if (true) check away.

I'll use that one in place of scoped for now, so that it works ok even if you've got an absurdly big grammar.

[ftomassetti]

Great work @lppedd , should we mark the PR as ready for review and merge it?

[lppedd]

Thank you! Yup, let's merge this one too, but first let's merge #115