openclover / clover

This repository contains source code of OpenClover Core as well as its integrations: Clover-for-Ant, Clover-for-Eclipse and Clover-for-IDEA plugins. Sources are licensed under Apache 2.0 license.
Other
61 stars 16 forks source link

Instrumentation of case expressions referencing non-final variables produces invalid Java code #247

Closed matic277 closed 6 months ago

matic277 commented 6 months ago

Mi Marek, me again. Another bug, this time regarding instrumentation in switch statements (using ->), which use non-final variables. Using Java 17, clover 4.5.2 Reproduced in repository: https://github.com/matic277/CloverInvalidJavaCode

Source class:

public class Main
{
  public static void main(String[] args)
  {
    final int x = 0;
    for (int i = 0; i <5 ; i++)
    {
      switch(x)
      {
        case 0 -> function(i);
        case 1 -> function(i);
      }
    }
  }

  private static void function(int s)
  {
    System.out.println(s);
  }
}

Will produce instrumented code:

  public static void main(String[] args)
  {__CLR4_5_2hhltimsdah.R.inc(17);
    __CLR4_5_2hhltimsdah.R.inc(18);final int x = 0;
    __CLR4_5_2hhltimsdah.R.inc(19);for (int i = 0; (((i <5 )&&(__CLR4_5_2hhltimsdah.R.iget(20)!=0|true))||(__CLR4_5_2hhltimsdah.R.iget(21)==0&false)); i++)
    {{
      __CLR4_5_2hhltimsdah.R.inc(22);switch(x)
      {
        case 0 ->__CLR4_5_2hhltimsdah.caseInc(23,()-> function(i));
        case 1 ->__CLR4_5_2hhltimsdah.caseInc(24,()-> function(i));
      }
    }
  }}

  private static void function(int s)
  {__CLR4_5_2hhltimsdah.R.inc(25);
    __CLR4_5_2hhltimsdah.R.inc(26);System.out.println(s);
  }

Note that this java code is no longer valid. Function call is put into a lambda expression on the case statement, and then passes var i to the function function, which is illegal. Vars used inside a lambda must be final.

Attempting to run the instrumented file will throw an exception: error: local variables referenced from a lambda expression must be final or effectively final

Simplest solution seems to be to wrap it into an array of size 1?

Workaround

Rewrite case expression to case block, e.g.:

// void
for (int i = 0; i <5 ; i++) {
    switch(x) {
        case 0 -> { function(i); } 
        case 1 -> { function(i); }
    }
}

or

// values
for (int i = 0; i <5 ; i++) {
    ret = switch(x) {
        case 0 -> { return function(i); }
        case 1 -> { return function(i); }
    }
}
marek-parfianowicz commented 6 months ago

Great finding! Thank you for reporting this.

marek-parfianowicz commented 6 months ago

PR: https://github.com/openclover/clover/pull/248