Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

For loop filter #3

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

ChristopherKoellner
Copy link

@ChristopherKoellner ChristopherKoellner commented Nov 17, 2021

Context

For such for loops, the Kotlin compiler generates two comparisons that are almost impossible to fully cover in most scenarios. The only way to get full coverage on them would require full access to the loop boundaries.

Examples

Example 1

class Example {
  fun example() {
    for (j in 0 until i1()) {}
  }
  private fun i1() = 1
}
0: iconst_0
1: istore_0
2: invokestatic  #10                 // Method i1:()I
5: istore_1
6: iload_0
7: iload_1
8: if_icmpge     21
11: iload_0
12: istore_2
13: iinc          0, 1
16: iload_0
17: iload_1
18: if_icmplt     11
21: return

Example 2

class Example {
  fun example() {
    for (j in i1() downTo 0) {}
  }
  private fun i1() = 1
}
 0: invokestatic  #10                 // Method i1:()I
 3: istore_0
 4: iconst_0
 5: iload_0
 6: if_icmpgt     19
 9: iload_0
10: istore_1
11: iinc          0, -1
14: iconst_0
15: iload_0
16: if_icmple     9
19: return

Example 3

class Example {                       
  fun example() {                     
    val limit = 10                    
    for (j in limit downTo i1()) {}   
  }                                   
  private fun i1() = 1                
}                                     
0: bipush        10
2: istore_0
3: iload_0
4: istore_1
5: invokestatic  #10                 // Method i1:()I
8: istore_2
9: iload_2
10: iload_1
11: if_icmpgt     24
14: iload_1
15: istore_3
16: iinc          1, -1
19: iload_3
20: iload_2
21: if_icmpne     14
24: return

This PR addresses this issue by searching for IF_ICMPGT or IF_ICMPGE then following the jump and and checking the previous node. If this node matches one of IF_ICMPLT, IF_ICMPNE or IF_ICMPLE that means that we found our loop and both nodes are going to be ignored.

IMO these branches can be safely ignored as jacoco would just check coverage on compiler generated code. As long as the loop body is checked for coverage we are good.

@poseidon-mysugr poseidon-mysugr changed the base branch from myMaster to master November 17, 2021 14:12
@poseidon-mysugr poseidon-mysugr self-requested a review December 18, 2022 12:30
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants