Skip to content

Update the KotlinUnsafeCastOperatorFilter for Kotlin 1.6.0 #1255

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Update the KotlinUnsafeCastOperatorFilter for Kotlin 1.6.0 #1255

wants to merge 1 commit into from

Conversation

poseidon-mysugr
Copy link
Contributor

The Kotlin 1.6.0 compiler changed how "unsafe casts" are compiled. Consider the following Kotlin class:

class Cast {
    fun cast(any: Any?): String {
        return any as String
    }
}

The Kotlin 1.5.32 compiler compiles the cast function to

  public final java.lang.String cast(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/String;
    flags: (0x0011) ACC_PUBLIC, ACC_FINAL
    Code:
      stack=3, locals=2, args_size=2
         0: aload_1
         1: ifnonnull     14
         4: new           #16                 // class java/lang/NullPointerException
         7: dup
         8: ldc           #18                 // String null cannot be cast to non-null type kotlin.String
        10: invokespecial #21                 // Method java/lang/NullPointerException."<init>":(Ljava/lang/String;)V
        13: athrow
        14: aload_1
        15: checkcast     #23                 // class java/lang/String
        18: areturn
      StackMapTable: number_of_entries = 1
        frame_type = 14 /* same */
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      19     0  this   Lcom/mysugr/Cast;
            0      19     1   any   Ljava/lang/Object;
    RuntimeInvisibleAnnotations:
      0: #13()
        org.jetbrains.annotations.NotNull
    RuntimeInvisibleParameterAnnotations:
      parameter 0:
        0: #14()
          org.jetbrains.annotations.Nullable

while the Kotlin 1.6.0 compiler produces:

  public final java.lang.String cast(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/String;
    flags: (0x0011) ACC_PUBLIC, ACC_FINAL
    Code:
      stack=3, locals=2, args_size=2
         0: aload_1
         1: dup
         2: ifnonnull     16
         5: pop
         6: new           #16                 // class java/lang/NullPointerException
         9: dup
        10: ldc           #18                 // String null cannot be cast to non-null type kotlin.String
        12: invokespecial #21                 // Method java/lang/NullPointerException."<init>":(Ljava/lang/String;)V
        15: athrow
        16: checkcast     #23                 // class java/lang/String
        19: areturn
      StackMapTable: number_of_entries = 1
        frame_type = 80 /* same_locals_1_stack_item */
          stack = [ class java/lang/Object ]
      LineNumberTable:
        line 5: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      20     0  this   Lcom/mysugr/Cast;
            0      20     1   any   Ljava/lang/Object;
    RuntimeInvisibleAnnotations:
      0: #13()
        org.jetbrains.annotations.NotNull
    RuntimeInvisibleParameterAnnotations:
      parameter 0:
        0: #14()
          org.jetbrains.annotations.Nullable

The 1.6.0 compiler duplicates the variable that's being casted before checking whether it's null, and pops the extra copy from the stack in case it is null; previous compilers instead loaded the variable a second time if it wasn't null.

This change makes the KotlinUnsafeCastOperatorFilter not detect the unsafe cast anymore because of the extra POP instruction; allowing this instruction to be there but not requiring it makes the filter compatible with both variants.

@poseidon-mysugr poseidon-mysugr force-pushed the feature/kotlinUnsafeCast1_6 branch from 21db2f1 to 988d041 Compare November 18, 2021 13:24
@Godin Godin self-requested a review December 22, 2021 00:10
@Godin Godin self-assigned this Dec 22, 2021
@Godin Godin mentioned this pull request Dec 22, 2021
Merged
@Godin Godin added this to the 0.8.8 milestone Dec 22, 2021
Godin
Godin requested changes Dec 22, 2021
View reviewed changes
Copy link
Member

@Godin Godin 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.

For the record this relates to the following change in Kotlin compiler - JetBrains/kotlin@041773f#diff-c479e51c961b3189024716a2e505117e50c2f05b630be1ea40ec70304a34f16aR250-R331

Change in KotlinUnsafeCastOperatorFilter looks good to me 👍 , however requires addition of test to corresponding KotlinUnsafeCastOperatorFilterTest, and update of changelog.

Unfortunately I can't push these little missing things directly into your branch, because seems that your branch is in org-repository and GitHub doesn't allow to enable "Allow edits from maintainers" for such - https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

So finalized this in #1266

Thank you for your contribution ❤️

@Godin Godin closed this in #1266 Dec 22, 2021
@poseidon-mysugr
Copy link
Contributor Author

Thanks a lot, @Godin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Godin Godin Godin requested changes

Assignees

@Godin Godin

Labels
component: core.filters language: Kotlin type: enhancement
Projects
Current work items
Status: Done
Filtering
Status: Done
Milestone
0.8.8
Development

Successfully merging this pull request may close these issues.
2 participants
@poseidon-mysugr @Godin