IR: KT-56446 Lax TypedIntrinsic annotation package

[soywiz]

This makes TypedIntrinsic annotation to be able to appear as a
subpackage of kotlin.native.internal. This would allow libraries to
internally redefine that annotation in those subpackages as a workaround
for KT-56446

Example of usage here: korlibs/korge#1346

[soywiz]

I would like to add tests of the new functions supporting lax subpackage matching in the case this would be acceptable.
But since that API is using IR interfaces that are likely hard to instantiate manually, can someone point me to a similar test that I could use as reference here?

[SvyatoslavScherbina]

As I said, defining such an intrinsic function must require an explicit opt-in. How does this PR achieve that?

[soywiz]

As I said, defining such an intrinsic function must require an explicit opt-in. How does this PR achieve that?

Well, forcing the annotation to be in a subpackage of kotlin.native.internal made me feel enough opt-in. But if you prefer an explicit one, should it be a -Xsomething? If so, would that mean that we would need to propagate a lot thru methods? I could investigate if you feel this is totally needed, any pointers for implementing this?

[soywiz]

Okay, so I'm going with the current implementation:

1. I have moved the stuff so this change is isolated in the backend.native module as proposed udalov.

2. After some experiments, I have decided not to propose the -Xlax-... option. Let me elaborate on that. I'm asuming it is not possible to get the compiler context from an Ir node, so please correct me if I'm wrong, and then I will add the -X option.

The thing is that propagating the CompilerConfiguration manually in this case doesn't seems practical.
I have a branch where I try: https://github.com/korlibs/kotlin/compare/master...korlibs:kotlin:feature/lax.TypedIntrinsic.package.proposal.show.propagation?expand=1
And for me, it is not reasonable. And it is not even finished, there are other classes and functions requiring it.

In this specific case I strongly believe and I'm going to defend it, that all that complexity is not needed. Why? Because this is an annotation that is not exposed, and that should be redeclared in a subpackage of kotlin.native.internal. that's almost explicit that it is internal and the risks. And it is impossible for anyone to can't accidentally use this.
So I believe that the tradeoffs between simplicity, security tells me that the extra super complexity is not worth.

Again, if it is possible to get the CompilerConfiguration or other global context class where we can store the flag from the IrFunction node, I'm open to it, since my point is about having to propagate that context manually in so many places, including existing properties and classes.

[kunyavskiy]

Hello. Thank you for your effort.

I totally agree with you that passing context through all this machinery would be an overkill. But i'm not sure if API, where user need to add annotations in koltin package is good.

I need some time to consult with people in standard library team, maybe they can suggest some better design here. I'll try to return in couple days with better suggestion.

[kunyavskiy]

Hi!

Thanks for waiting, but unfortunately, I come back with bad news. While discussing with colleagues, we've found a significant flaw in your approach, which probably can't be fixed.

The native standard library is bound to the compiler, while others are not. So, for now, intrinsic support has no compatibility guarantees. For example, suppose we decide to implement some intrinsic in terms of lower-level ones. In that case, we can do it by making the current intrinsic function a normal one and introducing new ones to implement it.

Unfortunately, when this leaked to other published libraries, there would be no such an option. And we are stuck forever with the current implementation (and its restrictions and custom checks, changing which can require such intrinsic redesign).

I understand your use case, and it makes a lot of sense. But unfortunately, it's not supported now. I have created some issues that help its support (they are linked to your original youtrack issue). I can't guarantee any timeline, but I hope we will do it at some point.

[soywiz]

Thanks for the explanation and for considering it. For me, being able to do this without hacks would be definitely better. So as long as it is possible to redirect the invoke in another inline function declared as actual fun, would do the job.

[soywiz]

Hey @kunyavskiy I have been thinking on this.

Since this is something not a blocker but yet highly desirable for my use case, Id like to try to insist a bit:

I was aware that this cant have compatibility guarantees and I was willing to accept that and asume the need of recompile libraries once kotlin version is bumped. In my case thats korge and that should be easy to do, and modules using korge are likely to be source-based without ABI issues. Korge in addition is tied to a specific Kotlin version.
This PR is a temporal workaround and a proper fix should allow to support inline fun redirections somehow.

I believe people adding a class in the kotlin.internal package should be aware that it is risky and there is something that could prevent compatibilityin the future.
You shouldn't be worried about having to keep compatibility or anything required to evolve Kotlin itself, and people using this trick will require to recompile and adjust as required.

If you are more comfortable by having a flag. I could for example try to add a reference to the compilation context in the root node of the Ir tree and try to get it by iterating the tree to the root, potentially caching the reference in nodes, though getting that context would happen only at declaration places with the annotation there. That would prevent having to manually propagate the context over all the methofs.

Alternatively I could try to implement the proper fix myself by supporting calling indirectly inline function. But I would need to investigate further in the internals and invest potentially a lot of time and probably not worth if not working on the kotlin compiler myself, also I wont have that time before my own release

[kunyavskiy]

Sorry for the long response. I was on a short vacation.

Unfortunately, the compatibility for Kotlin/Native works is a bit different from Kotlin/jvm. While jvm backend compiles to jvm bytecode, and here it's more-or-less infinitely backward compatible by jvm authors (up to stdlib function signatures), in native, all libraries code is recompiled for the final user.

So, in fact, breaking change doesn't only mean that you would be forced to rewrite your code to upgrade the compiler, but all your users would be forced to upgrade your library to proceed with the new compiler version. If this happens to popular libraries, this will lead to ecosystem fragmentation and a very long feature adoption cycle.

We are working on providing a more clear description of what compatibility is for native libraries, and some tooling, library authors would be able to rely on.

[soywiz]

No worries.

In the case of KorGE, people using it is going to use the korge gradle plugin that configures exactly the Kotlin version. People creating "libraries" for KorGE is going to provide source-code instead of binaries, so they will be compiled later. So I think the impact of that issue is limited in this case.

This would be useful now to be able to use it already, and then make the change just once, once a proper implementation is done. The thing is that since the timeline for a proper fix is not determined, and I don't have the knowledge to make a PR with a proper fix without investing potentially a lot of time, this temporal workaround would be nice.
I guess once the implementation for this is changed, probably there will also be a fix for this issue. So before that happens we can use this fix, and after we can use the proper way. In any case this compatibility issue might happen just once, and my users won't even notice because how it works: fixed kotlin version + source-code-based sub-libraries.

[soywiz]

Bump

[kunyavskiy]

Sorry, I don't understand what your intent is. We can not provide an API only available in your case.

I may have a solution which would help you.

You can try to make actual external operator fun <R> KPointerTT<KFunctionTT<() -> R>>.invoke(): R. It seems, it would work as expected now, if you put it into kotlinx.cinterop package.

I must mention, that by doing this, you are using several very unrobust stuff

1. Adding declarations to stdlib-provided packages.
2. Using some expect/actual features, that are not well designed, but just somehow works.

It's quite possible, that it would break in future releases (especially in https://blog.jetbrains.com/kotlin/2023/02/k2-kotlin-2-0/).

But if you are fine with binding your library to the exact compiler version, this should probably work for you.

Just to clarify once again. This functionality was not intended to work like this. This is some implementation side effect, which could go away in any moment. By using this, you are taking full responsibility for future migration.

[soywiz]

I tried that, and it gave an error on mingwX64 target IIRC, so I couldn't use it. It was due because I had to place the same annotation class in my project since it is internal in the standard library and the symbol is duplicated. Here's an example of the error: https://github.com/korlibs/korge/actions/runs/4104300820/jobs/7079608832#step:5:919
Surprisingly that error didn't happen on other K/N targets. But prevented me from using it.

And yeah, I'm aware that's a workaround tied to the specific implementation. I'm aware of K2. But I'm using a specific Kotlin version for each release. I can bump the version when there is a way of doing this. Or change the workaround as required.

[kunyavskiy]

It can be chance work without any annotation at all. But I can imagine it can depend on the order of libraries passed in arguments or order of file names or something like that (and it is even more probable to break in that case), as it shouldn't work.

Sorry, sometimes doing too low-level things is impossible in high-level language. We need to improve it in the future, but not by adding some random hacks.

[soywiz]

Okay, had to try since that would have been really nice to have already.

Thanks considering it and for adding a proper fix to the backlog, I'll wait for it then 👍