Fix #5054: Remove the dead code generated by the Labeled-based pattern matcher

[sjrd]

After this PR, the tree after patmat is

    def test(s: String): Int =
      matchResult1[Int]:
        {
          case val x1: String = s
          if "Hello".==(x1) then return[matchResult1] 1 else ()
          return[matchResult1] 2
        }

and the bytecode is

  public int test(java.lang.String);
    Code:
       0: aload_1
       1: astore_2
       2: ldc           #15                 // String Hello
       4: aload_2
       5: invokevirtual #19                 // Method java/lang/Object.equals:(Ljava/lang/Object;)Z
       8: ifeq          15
      11: iconst_1
      12: goto          19
      15: iconst_2
      16: goto          19
      19: ireturn

The only difference wrt. the previous implementation is now the useless goto 19 at offset 16. But that should be trivial for a peephole optimizer to get rid of.

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

1. Separate subject from body with a blank line
2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
3. Limit the subject line to 72 characters
4. Capitalize the subject line
5. Do not end the subject line with a period
6. Use the imperative mood in the subject line ("Add" instead of "Added")
7. Wrap the body at 80 characters
8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

[allanrenucci]

After this PR, the tree after patmat is

    def test(s: String): Int =
      matchResult1[Int]:
        {
          case val x1: String = s
          if "Hello".==(x1) then return[matchResult1] 1 else ()
          return[matchResult1] 2
        }

I see that you added a special case in the backend to not generate code if the else part if the Unit constant. Can you not generate the else branch of the if in the first place?

Also I think it would be good to add a bytecode test, that make sure that no athrow instruction is generated:

  @Test def labeledBlock = {
    val source =
      """class Test {
        |  def test(s: String) = s match {
        |    case "Hello" => 1
        |    case _       => 2
        |  }
        |}
      """.stripMargin

    checkBCode(source) { dir =>
      val clsIn = dir.lookupName("Test.class", directory = false)
      val clsNode = loadClassNode(clsIn.input)
      val method = getMethod(clsNode, "test")
      val throwMatchError = instructionsFromMethod(method).exists {
        case Op(Opcodes.ATHROW) => true
        case _ => false
      }
      assertFalse(throwMatchError)
    }
  }

[sjrd]

Can you not generate the else branch of the if in the first place?

I'm afraid not. dotty internals do not like that. In dotty a tpd.If must have an elsep, and it cannot be EmptyTree (I tried, things blow up everywhere).

[sjrd]

Also I think it would be good to add a bytecode test, that make sure that no athrow instruction is generated:

I have added the test, thanks! As well as a few other tests for switches.

[allanrenucci]

LGTM. Although, this would benefit from some documentation.

You should push your backend changes to sharing-backend and update the submodule to track the new HEAD

[allanrenucci]

You should update the doc comment of def inlineVars saying that it drops unreachable branch as well

[sjrd]

Updated. Rebased on master and addressed @allanrenucci's comments.