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

Improve filter for Kotlin when-expressions with String #1156

Merged
merged 4 commits into from Feb 21, 2021

Conversation

Godin
Copy link
Member

@Godin Godin commented Feb 21, 2021

For the following Example.kt

fun example1(p: String) {
  when (p) {
    "a"  -> return
    "b"  -> return
    else -> return
  }
}

fun example2(p: String) {
  when (p) { // first case has biggest hashCode value
    "b"  -> return
    "a"  -> return
    else -> return
  }
}

fun main(args: Array<String>) {
  example1("")
  example1("a")
  example1("b")
  
  example2("")
  example2("a")
  example2("b")
}

Execution of

kotlin/bin/kotlinc Example.kt

java \
  -javaagent:jacoco-0.8.6/lib/jacocoagent.jar \
  -cp kotlin/lib/kotlin-stdlib.jar:. \
  ExampleKt

java \
  -jar jacoco-0.8.6/lib/jacococli.jar \
  report jacoco.exec \
  --classfiles ExampleKt.class \
  --sourcefiles . \
  --html report

produces following report

ExampleKt

Execution of

javap -v -p ExampleKt.class

(tested for kotlinc versions from 1.3.0 to 1.4.30)

shows that Kotlin compiler generates

  public static final void example1(java.lang.String);
    descriptor: (Ljava/lang/String;)V
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL
    Code:
      stack=2, locals=2, args_size=1
         0: aload_0
         1: ldc           #9                  // String p
         3: invokestatic  #15                 // Method kotlin/jvm/internal/Intrinsics.checkNotNullParameter:(Ljava/lang/Object;Ljava/lang/String;)V
         6: aload_0
         7: astore_1
         8: aload_1
         9: invokevirtual #21                 // Method java/lang/String.hashCode:()I
        12: tableswitch   { // 97 to 98
                      97: 36
                      98: 48
                 default: 62
            }
        36: aload_1
        37: ldc           #23                 // String a
        39: invokevirtual #27                 // Method java/lang/String.equals:(Ljava/lang/Object;)Z
        42: ifeq          62
        45: goto          60
        48: aload_1
        49: ldc           #29                 // String b
        51: invokevirtual #27                 // Method java/lang/String.equals:(Ljava/lang/Object;)Z
        54: ifeq          62
        57: goto          61
        60: return
        61: return
        62: return

  public static final void example2(java.lang.String);
    descriptor: (Ljava/lang/String;)V
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL
    Code:
      stack=2, locals=2, args_size=1
         0: aload_0
         1: ldc           #9                  // String p
         3: invokestatic  #15                 // Method kotlin/jvm/internal/Intrinsics.checkNotNullParameter:(Ljava/lang/Object;Ljava/lang/String;)V
         6: aload_0
         7: astore_1
         8: aload_1
         9: invokevirtual #21                 // Method java/lang/String.hashCode:()I
        12: tableswitch   { // 97 to 98
                      97: 36
                      98: 48
                 default: 59
            }
        36: aload_1
        37: ldc           #23                 // String a
        39: invokevirtual #27                 // Method java/lang/String.equals:(Ljava/lang/Object;)Z
        42: ifeq          59
        45: goto          58
        48: aload_1
        49: ldc           #29                 // String b
        51: invokevirtual #27                 // Method java/lang/String.equals:(Ljava/lang/Object;)Z
        54: ifeq          59
        57: return
        58: return
        59: return

where method example1 has goto at offset 57,
whereas no such goto in example2.

KotlinWhenStringFilter does not filter bytecode of example2 due to absence of this goto

This was discovered by @margarita-nedzelska-sonarsource ❤️

@Godin Godin added this to the 0.8.7 milestone Feb 20, 2021
@Godin Godin self-assigned this Feb 20, 2021
@Godin Godin added this to Candidates in Current work items via automation Feb 20, 2021
@Godin Godin moved this from Candidates to Implementation in Current work items Feb 20, 2021
@Godin Godin added this to To Do in Filtering via automation Feb 20, 2021
@Godin Godin moved this from To Do to In Progress in Filtering Feb 20, 2021
@Godin Godin marked this pull request as draft February 21, 2021 08:57
@Godin
Copy link
Member Author

Godin commented Feb 21, 2021

The same example after this change

ExampleKt

@Godin Godin moved this from Implementation to Review in Current work items Feb 21, 2021
@Godin Godin requested a review from marchof February 21, 2021 15:39
@Godin Godin marked this pull request as ready for review February 21, 2021 15:39
@Godin Godin changed the title Improve KotlinWhenStringFilter Improve filter for Kotlin when-expressions with String Feb 21, 2021
Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

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

👏 Thank‘s @Godin! You‘re the first human decompiler 😅

@marchof marchof merged commit 66b5fa9 into master Feb 21, 2021
Filtering automation moved this from In Progress to Done Feb 21, 2021
Current work items automation moved this from Review to Done Feb 21, 2021
@Godin Godin deleted the kotlin_when_string branch February 21, 2021 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Filtering
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants