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

Optimizer preserves tuple for match failure case #12930

Open
som-snytt opened this issue Jan 20, 2024 · 3 comments
Open

Optimizer preserves tuple for match failure case #12930

som-snytt opened this issue Jan 20, 2024 · 3 comments

Comments

@som-snytt
Copy link

Reproduction steps

Scala version: 2.13.12

  def t8790(i: Int, j: Int) =
    (i, j) match {
      case (42, 27) => i+j
      //case _ => ???
    }
  def slow(x: Int, y: Int): String = (x, y) match {
    case (7, 8) => "You got it!"
    case _ => "No such luck."
  }

Problem

Under -opt:local, the tuple is not eliminated if it is used for the match error. However, it is always allocated instead of only in the failure case.

It would be nice to warn -Wopt if a tuple allocation is not eliminated.

@som-snytt
Copy link
Author

➜  snips scala -J--enable-preview -opt:local -J--add-exports -Jjdk.jdeps/com.sun.tools.javap=ALL-UNNAMED -nobootcp -Xlint
Welcome to Scala 2.13.13 (OpenJDK 64-Bit Server VM, Java 21.0.2).
Type in expressions for evaluation. Or try :help.

scala> :load array-destruct.scala
val args: Array[String] = Array()
Loading array-destruct.scala...

         def t8790(i: Int, j: Int) = (i, j) match {
                                     ^
array-destruct.scala:20: warning: match may not be exhaustive.
       It would fail on the following inputs: (42, _), (_, 27), (_, _)
object C
class X
class Y

scala> :javap C#t8790
  public int t8790(int, int);
    descriptor: (II)I
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=4, locals=4, args_size=3
         0: new           #80                 // class scala/Tuple2$mcII$sp
         3: dup
         4: iload_1
         5: iload_2
         6: invokespecial #83                 // Method scala/Tuple2$mcII$sp."<init>":(II)V
         9: astore_3
        10: bipush        42
        12: iload_1
        13: if_icmpne     26
        16: bipush        27
        18: iload_2
        19: if_icmpne     26
        22: iload_1
        23: iload_2
        24: iadd
        25: ireturn
        26: new           #67                 // class scala/MatchError
        29: dup
        30: aload_3
        31: invokespecial #68                 // Method scala/MatchError."<init>":(Ljava/lang/Object;)V
        34: athrow
      StackMapTable: number_of_entries = 1
        frame_type = 252 /* append */
          offset_delta = 26
          locals = [ class scala/Tuple2$mcII$sp ]
      LineNumberTable:
        line 20: 0
        line 21: 10
        line 20: 26
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      35     0  this   L$line4/$read$$iw$C$;
            0      35     1     i   I
            0      35     2     j   I
    MethodParameters:
      Name                           Flags
      i                              final
      j                              final

scala> :javap C#slow
  public java.lang.String slow(int, int);
    descriptor: (II)Ljava/lang/String;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=2, locals=3, args_size=3
         0: bipush        7
         2: iload_1
         3: if_icmpne     15
         6: bipush        8
         8: iload_2
         9: if_icmpne     15
        12: ldc           #89                 // String You got it!
        14: areturn
        15: ldc           #91                 // String No such luck.
        17: areturn
      StackMapTable: number_of_entries = 1
        frame_type = 15 /* same */
      LineNumberTable:
        line 25: 0
        line 26: 15
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      18     0  this   L$line4/$read$$iw$C$;
            0      18     1     x   I
            0      18     2     y   I
    MethodParameters:
      Name                           Flags
      x                              final
      y                              final

@lrytz
Copy link
Member

lrytz commented Apr 18, 2024

(pasted the wrong code in my previous, now deleted comment).

this could be done by a new local optimization, the tuple value only has a single consumer, so it can be inlined / moved (if the arguments don't have side effects).

it seems it would also work in this case

  def i = 0
  def j = 0
  def t8790 =
    (i, j) match {
      case (42, 27) => 0
      //case _ => ???
    }

as there are local variables (by cfr-decompiler)

    public int t8790() {
        int n = this.j();
        int n2 = this.i();
        Tuple2$mcII$sp tuple2$mcII$sp = new Tuple2$mcII$sp(n2, n);
        if (42 == n2 && 27 == n) {
            return 0;
        }
        throw new MatchError(tuple2$mcII$sp);
    }

cfr-decompiler is a bit confused, i is actually invoked before j. the bytecode is a bit special in storing the locals between the new and <init>.

    NEW scala/Tuple2$mcII$sp
    DUP
    ALOAD 0
    INVOKEVIRTUAL C.i ()I
    ALOAD 0
    INVOKEVIRTUAL C.j ()I
    ISTORE 3
    ISTORE 2
    ILOAD 2
    ILOAD 3
    INVOKESPECIAL scala/Tuple2$mcII$sp.<init> (II)V

@sjrd
Copy link
Member

sjrd commented Apr 18, 2024

Scala.js also suffers from this issue. So far I haven't found a good way to solve it. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants