[go: nahoru, domu]

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

KT-54363 Allow using reified types for catch parameters #5194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kralliv
Copy link
Contributor
@kralliv kralliv commented Sep 28, 2023

Allow using reified types for catch parameters

inline fun <reified E : Throwable> fails(block: () -> Unit): E {
    try {
        block()
    } catch (exception: E) {
        return exception
    }
    
    error("should have failed")
}

For more context read the comment in the YouTrack Issue.

Kotlin/JVM

In Bytecode catch clauses are described by two things: an entry in the exception table and a handler entrypoint in the instructions.

TRYCATCHBLOCK L0 L1 L2 java/lang/Throwable

L0:         // start
  [...]
L1:         // end
  NOP
L2:         // handler
  LINENUMBER 4 L2
 FRAME FULL [] [java/lang/Throwable]
  ASTORE 0
L3:
  [...]

The entry consists out of three labels and type. The first two labels describe the effective range, where exceptions of the specified type are to be caught. If caught, the exception is pushed onto the stack and the execution jumps to the third label. The third label is the handler. Its responsibility its to the receive and store the exception on top of the stack. It is not to be confused with the catch body, which follows with the next label.

The goal is to store information about where type parameters are and replace them with the actual type once known.

The simplest solution would be to store the type parameter in the exception table itself. While the type is a string, it cannot be used for storing the type parameter. The type is loaded eagerly during class initialization. Storing for example [T? would lead to a ClassNotFound. This means, that Bytecode does not allow for the exception table to be used for storing a reification markers directly.

Instead, I opted to reuse the current reification marker system targeted at the ASTORE instruction in the handler.

TRYCATCHBLOCK L0 L1 L2 java/lang/Throwable

L0:         // start
  [...]
L1:         // end
  NOP
L2:         // handler
  LINENUMBER 4 L2
 FRAME FULL [] [java/lang/Throwable]
  BIPUSH 7
  LDC "E"
  INVOKESTATIC kotlin/jvm/internal/Intrinsics.reifiedOperationMarker (ILjava/lang/String;)V
  ASTORE 0
L3:
  [...]

The ASTORE instruction can be directly traced back to its entry in the exception table via the label of the handler.

During inlining, if the type is known, the instruction is left as is. Instead, the type in the exception table is replaced by the actual type and the marker is removed. If the type parameter is just another type parameter, the value of LDC instruction is modified accordingly instead, like it is done in the case of all the others reified operations.

Kotlin/Other

As suspected by Roman Elizarov in the issue, the IR inliner seems to be able to correctly inline the catch clause parameter. All of my test cases worked out of the box, once I had disabled the error diagnostics.

Diagnostics

I updated the diagnostics code to only forbid reified types in catch clauses in versions lower than 2.0 and changed the messages accordingly. This way, red code will be preserved for older version.

@udalov udalov self-assigned this Oct 2, 2023
@kralliv
Copy link
Contributor Author
kralliv commented Jan 29, 2024

Any updates on this? Is this indirectly covered by KT-64570 and therefor not relevant anymore?

@udalov
Copy link
Member
udalov commented Jan 29, 2024

@kralliv Sorry for the delay, and thank you for the ping. It's still relevant but we're pretty busy with finishing the K2 compiler right now. Moreover, we have an embargo on new language features in 2.0, so the earliest this feature can be enabled is Kotlin 2.1. Your change seems fine, but I'll have to review it in detail a little bit later.

@kralliv
Copy link
Contributor Author
kralliv commented Jan 30, 2024

Absolutely no problem. I'm well aware that finishing K2 is a churn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants