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

Update the KotlinUnsafeCastOperatorFilter for Kotlin 1.6.0 #1255

Closed

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.

@Godin Godin added this to Awaiting triage in Filtering via automation Nov 18, 2021
@Godin Godin moved this from Awaiting triage to To Do in Filtering Dec 15, 2021
@Godin Godin self-requested a review December 22, 2021 00:10
@Godin Godin added this to Implementation in Current work items via automation Dec 22, 2021
@Godin Godin moved this from Implementation to Review in Current work items Dec 22, 2021
@Godin Godin self-assigned this Dec 22, 2021
@Godin Godin added this to the 0.8.8 milestone Dec 22, 2021
Copy link
Member

@Godin Godin left a comment

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
Current work items automation moved this from Review to Done 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
Projects
Filtering
  
To Do
Development

Successfully merging this pull request may close these issues.

None yet

2 participants