kaitai-io / kaitai_struct

Kaitai Struct: declarative language to generate binary data parsers in C++ / C# / Go / Java / JavaScript / Lua / Nim / Perl / PHP / Python / Ruby
https://kaitai.io
4.04k stars 199 forks source link

Extended nullabillity support for Kotlin and other null-safe languages #1111

Open xpathexception opened 6 months ago

xpathexception commented 6 months ago

Hello!

I'm currently developing Kotlin language support in order to bring Kaitai Struct to the Kotlin Multiplatform world. For now I've implemented most of the things and the implementation is able to pass 231 out of 233 tests on JVM and MacOS ARM64 targets.

But there's an issue with 7 of them related to kotlin's Null Safety concept.

There are at least two main operators in Kotlin exist to provide null-safety support - Non-null Assertion Operator (!!) and Safe Calls Operator (?.). The main issue is with the safe call operator - in order to use it and in order to be able to perform safe call chaining, both compiler and translator should be aware of nullability of each type.

I've came up with two approaches which could help resolve this issue. Let's pick NavParentRecursive and it's test as an example.

The most interesting part of the test looks like this:

assertEquals(actual = r.next().value(), expected = 1)
assertEquals(actual = r.next().parentValue(), expected = 255)
assertNull(r.next().next())

We don't care about null

Using this approach we can simply make every attributeReader return non-nullable type by requiring attribute being not null explicitly. Nullability in instances can be handled in the same way.

More on this **Pros**: - easy to implement - already works **Cons**: - on the call site it is unclear will reading fail or not - nullability is basically unmanageable - need to adjust tests by replacing `assertNull` with `assertFails` ```kotlin /* attributeDeclaration */ /* isNullable: false */ private var value: IntU1 by Delegates.notNull() // avoiding default values for primitive types fun value(): IntU1 = value /* attributeDeclaration */ /* isNullable: true */ private var next: NavParentRecursive? = null fun next(): NavParentRecursive = requireNotNull(next) private fun _read() { this.value = this._io.readU1() if (value() == 255.toIntU1()) { this.next = NavParentRecursive(_io = this._io, _parent = this, _root = _root) } } /* instanceDeclaration */ private var parentValue: IntU1? = null private var f_parentValue = false fun parentValue(): IntU1? { if (f_parentValue) { return this.parentValue } if (value() != 255.toIntU1()) { this.parentValue = ((_parent() as NavParentRecursive).value()).toIntU1() } f_parentValue = true return this.parentValue } ``` Giving that adjusting test as follows: ```kotlin assertFails { r.next().next() } ```

Nullable types should be supported at DataType level

Using this approach wi need to be able to propagate nullability to the resulting type of every expression.

More on this **Pros**: - explicitly nullable types would be easier to use on the call site - looks much safer than the other way - expected behaviour in Kotlin - tests will receive support automatically **Cons**: - looks like it requires adjusting the whole `DataType` layer - requires to develop a set of rules for cases like `DataType? %op% DataType` in order to resolve resulting nullability - requires special handling for most of the operators - requires adjusting compiler and translator contracts and implementations Giving that if we declare attribute reader's type nullable like this: ```kotlin /* attributeDeclaration */ /* isNullable: true */ private var next: NavParentRecursive? = null fun next(): NavParentRecursive? = next ``` compiler should be able to generate something like this: ```kotlin assertEquals(actual = r.next()?.value(), expected = 1) assertEquals(actual = r.next()?.parentValue(), expected = 255) assertNull(r.next()?.next()) ``` In order to do so we need to be able to know if the `lvalue` nullable or not and propagate nullability to the right side. Moreover there's one more caveat: if expression is used as parameter, we need to know if target function accepts nullable parameters and perform non-null assertion in some cases.

As the bottom line, I believe it would be much better to extend nullability support, at least for the Kotlin implementation.

I'm not sure that approach with adjusting the whole DataType system with, for example isNullable flag is the proper way to implement null safety support. Therefore I'm looking for some help with this using that or alternative approach.

In short, there are very few things needs to be done:

Skaldebane commented 4 months ago

This might be related: https://github.com/kaitai-io/kaitai_struct/issues/141

Looking forward to Kotlin Multiplatform support!

Skaldebane commented 4 months ago

@xpathexception Can open a pull request with your work on the Kotlin support?

Since there's no direct support for nullability, I think shipping the current implementation (where nulls are ignored) may be okay, until there's support in Kaitai itself. This affects other languages like C# as well (and I'm not sure how the Rust folks are handling this).

Another path to take is to err on the safe side and mark literally everything as nullable, but that would be very inconvenient and useless to the end-user of the generated code, just more annoying to avoid (i.e. a catch-all try/catch vs. 100s of ? or !! symbols) without much benefit over just throwing an NPE.

xpathexception commented 4 months ago

@Skaldebane I had no plans on publishing my work (i.e. opening PR) until this issue resolved somehow because of couple of reasons:

Giving all of that, I can't see Kotlin Multiplatform support as a part of the main, at least in it's current state and without interest and support from maintainers.

And finally answering your question. It will take some time and effort to wrap things up and publish them. I can try to do so if you're willing to help with further improvement and take a part in this adventure :)

Skaldebane commented 3 months ago

@xpathexception Thanks for the thorough response!

I also dislike throwing NPEs, but if they're the only way forward in the current state of things, then I guess we'll have to make do with that. We may be able to take inspiration from the way this works in the C# or Rust implementation.

As for inclusion in main, I think (after cleaning things) that should be possible, but while marking it as "entry-level support", like Rust and Go are marked in Kaitai's website.

While I don't currently have much time to help with this (due to some life circumstances), I'm quite interested and will surely hit you up when I'm ready to hop on this (hopefully very soon). I never worked with Scala or code generation before, but I can help with the runtime part and getting it working on all KMP platforms at the very least.

xpathexception commented 3 months ago

@Skaldebane I've managed to collect all the things I've done as of now.

There are three parts: compiler branch, tests branch and runtime repository.

I've synced all of it with the latest changes, but haven't tried to compile. It is hard to recall latest state and roadmaps I had in mind, so it will require some effort to make all of that at least compile.

Tests itself were compiling, but they're will require some manual adjustments because of nullability issues/readers implementations inconsistency. I've tried to mimic original tests buildscripts in ruby, but had no luck, so they're may be broken at all.

There may also be a set of "homemade feature-flags" because I've tried some different approaches. I have to say that this code was not meant to be shared at this moment and may be very ugly in a bunch of ways - it is still just a half-working POC.

At last, I've decided to stop working on kotlin support as a part of kaitai struct at all, but rather started implementation of compiler, tests and runtime in pure kotlin. I'm working on it for more than a month now and it has advanced very well (I've even managed to add more or less proper nullability support), but I'll keep it private for some time until some major issues are resolved. Maybe I'll make a public roadmap of it somewhere in my repos, I still haven't decided on it.

Mingun commented 3 months ago

@xpathexception , just for your reference, you may be interested to look at my compiler in Rust to get some inspiration. Recently, I have resumed active work on it.

Skaldebane commented 3 months ago

@xpathexception That's great to hear! The pure-Kotlin implementation sounds great, excited to see how that turns out.

I'd be happy to help if you need assistance with any part of the work.