Skip to content

Update KotlinCoroutineFilter for Kotlin 1.6#1283

Merged
Godin merged 7 commits intomasterfrom
kotlin_1_6_suspending_lambda_without_suspension_points
Jul 13, 2024
Merged

Update KotlinCoroutineFilter for Kotlin 1.6#1283
Godin merged 7 commits intomasterfrom
kotlin_1_6_suspending_lambda_without_suspension_points

Conversation

@Godin
Copy link
Copy Markdown
Member

@Godin Godin commented Feb 3, 2022

For the following Example.kt

fun example() {
  exec {
    // suspending lambda without suspension points,
    // i.e. without invocations of suspending functions/lambdas
  }
}

fun exec(suspendingLambda: suspend () -> Unit) {}

execution of

kotlin/bin/kotlinc Example.kt -d classes
javap -v -p classes/ExampleKt\$example\$1.class

using Kotlin compiler version 1.5.32 (latest as of today in 1.5 series) produces

  public final java.lang.Object invokeSuspend(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_FINAL
    Code:
      stack=3, locals=3, args_size=2
         0: invokestatic  #36                 // Method kotlin/coroutines/intrinsics/IntrinsicsKt.getCOROUTINE_SUSPENDED:()Ljava/lang/Object;
         3: astore_2
         4: aload_0
         5: getfield      #40                 // Field label:I
         8: tableswitch   { // 0 to 0
                       0: 28
                 default: 36
            }
        28: aload_1
        29: invokestatic  #46                 // Method kotlin/ResultKt.throwOnFailure:(Ljava/lang/Object;)V
        32: getstatic     #52                 // Field kotlin/Unit.INSTANCE:Lkotlin/Unit;
        35: areturn
        36: new           #54                 // class java/lang/IllegalStateException
        39: dup
        40: ldc           #56                 // String call to 'resume' before 'invoke' with coroutine
        42: invokespecial #59                 // Method java/lang/IllegalStateException."<init>":(Ljava/lang/String;)V
        45: athrow

whereas using Kotlin compiler version 1.6.0 produces

  public final java.lang.Object invokeSuspend(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_FINAL
    Code:
      stack=3, locals=2, args_size=2
         0: invokestatic  #36                 // Method kotlin/coroutines/intrinsics/IntrinsicsKt.getCOROUTINE_SUSPENDED:()Ljava/lang/Object;
         3: pop
         4: aload_0
         5: getfield      #40                 // Field label:I
         8: tableswitch   { // 0 to 0
                       0: 28
                 default: 36
            }
        28: aload_1
        29: invokestatic  #46                 // Method kotlin/ResultKt.throwOnFailure:(Ljava/lang/Object;)V
        32: getstatic     #52                 // Field kotlin/Unit.INSTANCE:Lkotlin/Unit;
        35: areturn
        36: new           #54                 // class java/lang/IllegalStateException
        39: dup
        40: ldc           #56                 // String call to 'resume' before 'invoke' with coroutine
        42: invokespecial #59                 // Method java/lang/IllegalStateException."<init>":(Ljava/lang/String;)V
        45: athrow

Currently the first one is filtered by KotlinCoroutineFilter, but not the second.

And the diff between them

-      stack=3, locals=3, args_size=2
+      stack=3, locals=2, args_size=2
          0: invokestatic  #36                 // Method kotlin/coroutines/intrinsics/IntrinsicsKt.getCOROUTINE_SUSPENDED:()Ljava/lang/Object;
-         3: astore_2
+         3: pop
          4: aload_0

This relates to the following change in Kotlin compiler - JetBrains/kotlin@f4a1e27

Closes #1258

@Godin Godin self-assigned this Feb 3, 2022
@Godin Godin added this to the 0.8.9 milestone Apr 26, 2022
@Godin Godin mentioned this pull request Nov 16, 2022
Closed
@Godin Godin mentioned this pull request Mar 31, 2023
Closed
@Godin Godin modified the milestones: 0.8.9, 0.8.10 Mar 31, 2023
@Godin Godin modified the milestones: 0.8.10, 0.8.11 Apr 24, 2023
@Godin Godin modified the milestones: 0.8.11, 0.8.12 Oct 14, 2023
@Godin Godin removed this from the 0.8.12 milestone Apr 9, 2024
@Godin Godin force-pushed the kotlin_1_6_suspending_lambda_without_suspension_points branch 2 times, most recently from 5b1d09c to 86b594c Compare May 14, 2024 07:43
@Godin Godin added this to the 0.8.13 milestone May 14, 2024
@Godin Godin marked this pull request as ready for review May 14, 2024 09:45
@Godin Godin requested a review from marchof May 14, 2024 09:45
@Godin Godin removed the request for review from marchof May 31, 2024 08:43
@Godin Godin force-pushed the kotlin_1_6_suspending_lambda_without_suspension_points branch from 14493cb to 8e01f01 Compare May 31, 2024 08:44
@Godin Godin requested a review from leveretka May 31, 2024 09:55
leveretka
leveretka reviewed Jun 7, 2024
View reviewed changes
Comment thread org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java Outdated
&& Opcodes.POP == skipNonOpcodes(cursor.getNext())
.getOpcode()) {
nextIs(Opcodes.POP);
vars.put("COROUTINE_SUSPENDED",
Copy link
Copy Markdown
Collaborator

@leveretka leveretka Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not fully clear from the PR why this line is needed.

After discussing it, I suggest either adding a comment that this is a safe guard or refactoring the code, so you don't need it.

Copy link
Copy Markdown
Member Author

@Godin Godin Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leveretka ok, I did refactoring

@leveretka leveretka self-requested a review June 7, 2024 12:55
leveretka
leveretka requested changes Jun 7, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@leveretka leveretka left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above

@Godin Godin requested a review from leveretka July 13, 2024 20:53
@Godin Godin force-pushed the kotlin_1_6_suspending_lambda_without_suspension_points branch from b29d7a3 to 0ff3fd7 Compare July 13, 2024 20:58
@Godin Godin enabled auto-merge (squash) July 13, 2024 21:14
@Godin Godin merged commit fce2e5b into master Jul 13, 2024
@Godin Godin deleted the kotlin_1_6_suspending_lambda_without_suspension_points branch July 13, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leveretka leveretka leveretka approved these changes

Assignees

@Godin Godin

Labels

component: core.filters language: Kotlin type: enhancement

Projects

Current work items
Status: Done
Filtering
Status: Done

Milestone

0.8.13

Development

Successfully merging this pull request may close these issues.

2 participants

@Godin @leveretka