8326404: Assertion error when trying to compile switch with fallthrough with pattern

[biboudis]

The assertion error that was raised in the bug report is a result of local variables not being defined in the case of switches that mix type patterns and record patterns with fall-through. e.g., a print after the TransPatterns phase:

private static int run(Object o) {
  int i = 0;
  /*synthetic*/ Object selector2$temp = <*nullchk*>(o);
  /*synthetic*/ int index$3 = 0;
  switch (java.lang.runtime.SwitchBootstraps.typeSwitch(selector2$temp, index$3)) {
    case 0:
      String s;
      if (!(let s = (String)selector2$temp; in true)) {
        index$3 = 1;
        continue;
      }
      i++;
    case 1:
      {
      /*synthetic*/ Object selector4$temp = $b$O.a(); // $b$O appears undefined here 
      ...
      }
    ...
  }
}

The code for unrolling a case label into local variable declaration statements appears in handleSwitch here: https://github.com/openjdk/jdk/pull/17981/files#diff-e50bbfa8783f3bc8f5542452740b78f3167bee19be7365a87da2298c6333cca6L593-L606

patchCompletingNormallyCases takes care of statement block propagation for code that completes normally. Unfortunately this code is called late (after the optimization that merges common type tests together e.g., case R(...): .... case R(...): ....).

The fix bails out of this optimization in the presence of fall-through. For example in the run3_break1 included, the optimization is triggered. In the rest, not.

Additionally, this patch elides the unnecessary binding generation of unnamed pattern variables from the binding context.

Thanks @lahodaj for contributing this solution 🥇

---

Progress

• [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue

Issue

• JDK-8326404: Assertion error when trying to compile switch with fallthrough with pattern (Bug - P4)

Reviewers

• Vicente Romero (@vicente-romero-oracle - Reviewer)

Contributors

• Jan Lahoda <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17981/head:pull/17981
$ git checkout pull/17981

Update a local copy of the PR:
$ git checkout pull/17981
$ git pull https://git.openjdk.org/jdk.git pull/17981/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17981

View PR using the GUI difftool:
$ git pr show -t 17981

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17981.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back abimpoudis! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@biboudis The following label will be automatically applied to this pull request:

• compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[biboudis]

/contributor add @lahodaj

[openjdk[bot]]

@biboudis Contributor Jan Lahoda <[email protected]> successfully added.

[mlbridge[bot]]

Webrevs

• 00: Full (b6c90ca4)

[openjdk[bot]]

@biboudis This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8326404: Assertion error when trying to compile switch with fallthrough with pattern

Co-authored-by: Jan Lahoda <[email protected]>
Reviewed-by: vromero

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1076 new commits pushed to the master branch:

• 5ded8da676d62158d0011086d7f80ff2c9096e13: 8332085: Remove 10 year old transition check in GenerateCurrencyData tool
• 7c2c24fc0511b36132952c96be46eea5904a53c5: 8261433: Better pkcs11 performance for libpkcs11:C_EncryptInit/libpkcs11:C_DecryptInit
• ff4bf1cf9f18547cff8f484433c3c55b4c288aaa: 8332102: Add @since to package-info of jdk.security.jarsigner
• abf54bb1e6da6d7bc432b3e9bb3ff164a895bd3e: 8332100: Add missing @since to KeyValue::EC_TYPE in java.xml.crypto
• 1484153c1a092cefc20270b35aa1e508280843a4: 8332080: Update troff man page for javadoc
• 391bbbc7d0fb95b0cd55e2f56c43bee019aeab7f: 8330584: IGV: XML does not save all node properties
• adaa509b6ed3d12569b8e5f2ec802cef22ab53c7: 8327499: MethodHandleStatics.traceLambdaForm includes methods that cannot be generated
• 5a8df4106ac5386eb72e874dcadf2b18defe27d8: 8331535: Incorrect prompt for Console.readLine
• 3e3f7cf4bddf243fddfeac8cfc1d9b2a1be55043: 8330387: Crash with a different types patterns (primitive vs generic) in instanceof
• d517d2df451e135583083ed3684d7d3241b36f76: 8331764: C2 SuperWord: refactor _align_to_ref/_mem_ref_for_main_loop_alignment
• ... and 1066 more: https://git.openjdk.org/jdk/compare/5d414da50459b7a1e6f0f537ff3b318854b2c427...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[bridgekeeper[bot]]

@biboudis This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[biboudis]

Keep it open.

[bridgekeeper[bot]]

@biboudis This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[biboudis]

/integrate

[openjdk[bot]]

Going to push as commit ea5eb74a65f20ce28fa0a94ea851915d4a6f83da. Since your change was applied there have been 1078 commits pushed to the master branch:

• beea5305b071820e2b128a55c5ca384caf470fdd: 8331907: BigInteger and BigDecimal should use optimized division
• 440782e0160f867f08afbec0abf48d557a522c72: 8331466: Problemlist serviceability/dcmd/gc/RunFinalizationTest.java on generic-all
• 5ded8da676d62158d0011086d7f80ff2c9096e13: 8332085: Remove 10 year old transition check in GenerateCurrencyData tool
• 7c2c24fc0511b36132952c96be46eea5904a53c5: 8261433: Better pkcs11 performance for libpkcs11:C_EncryptInit/libpkcs11:C_DecryptInit
• ff4bf1cf9f18547cff8f484433c3c55b4c288aaa: 8332102: Add @since to package-info of jdk.security.jarsigner
• abf54bb1e6da6d7bc432b3e9bb3ff164a895bd3e: 8332100: Add missing @since to KeyValue::EC_TYPE in java.xml.crypto
• 1484153c1a092cefc20270b35aa1e508280843a4: 8332080: Update troff man page for javadoc
• 391bbbc7d0fb95b0cd55e2f56c43bee019aeab7f: 8330584: IGV: XML does not save all node properties
• adaa509b6ed3d12569b8e5f2ec802cef22ab53c7: 8327499: MethodHandleStatics.traceLambdaForm includes methods that cannot be generated
• 5a8df4106ac5386eb72e874dcadf2b18defe27d8: 8331535: Incorrect prompt for Console.readLine
• ... and 1068 more: https://git.openjdk.org/jdk/compare/5d414da50459b7a1e6f0f537ff3b318854b2c427...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@biboudis Pushed as commit ea5eb74a65f20ce28fa0a94ea851915d4a6f83da.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.