Skip to content

Build TABLESWITCH from sorted arrays in SWITCH#518

Merged
garydgregory merged 1 commit into
apache:masterfrom
rootvector2:switch-tableswitch-sorted-arrays
Jul 11, 2026
Merged

Build TABLESWITCH from sorted arrays in SWITCH#518
garydgregory merged 1 commit into
apache:masterfrom
rootvector2:switch-tableswitch-sorted-arrays

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

SWITCH clones the caller's match/targets arrays and sorts only the clones, but the TABLESWITCH branch fills the table from the original unsorted match/targets, while the LOOKUPSWITCH branch uses matchClone/targetsClone. When the match array is passed unsorted (the class javadoc documents unsorted input and promises to leave the caller's arrays unaltered) and its sorted form is gap-free, the fill loop runs over unsorted keys, so the generated TABLESWITCH gets out-of-order case values padded with default entries. TABLESWITCH.dump then writes low = match[0] and high = match[last] that disagree with the count of offset words emitted, and re-reading the dumped instruction consumes only high - low + 1 offsets, reinterpreting the surplus words as the next instructions.

Fill the table from matchClone/targetsClone to match the LOOKUPSWITCH branch. Found while auditing the switch-generation paths.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

The TABLESWITCH branch filled the table from the caller's unsorted match/targets arrays instead of the sorted clones the LOOKUPSWITCH branch uses, so an unsorted match array produced a table with out-of-order case values and a low/high header that disagrees with the emitted offset count.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect TABLESWITCH generation in SWITCH when the caller provides an unsorted match array whose sorted form is gap-free. The TABLESWITCH table is now built from the already-sorted clones (matchClone/targetsClone), aligning behavior with the LOOKUPSWITCH path and preventing invalid low/high metadata vs. offset table size.

Changes:

  • Build the TABLESWITCH mVec/tVec table using matchClone and targetsClone (sorted clones) instead of the original input arrays.
  • Add a regression test ensuring an unsorted-but-gap-free match produces a TABLESWITCH with sorted case keys and correctly paired targets.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/main/java/org/apache/bcel/generic/SWITCH.java Fixes TABLESWITCH table construction to use sorted clones, preventing malformed serialized switches.
src/test/java/org/apache/bcel/generic/SWITCHTest.java Adds coverage for the unsorted-input, gap-free case to prevent regressions in TABLESWITCH generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@garydgregory garydgregory merged commit d6ff11a into apache:master Jul 11, 2026
21 of 22 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Merged, thank you @rootvector2 🚀

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants