From 773d4f6c41a40bcb38679c9ac901ca2733f182ed Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Sat, 4 May 2024 20:36:51 -0700 Subject: [PATCH] Move default case reordering to MethodProcessor This way it also fixes it for old-style switches. --- .../decompiler/main/rels/MethodProcessor.java | 1 + .../modules/decompiler/SwitchHelper.java | 39 ++++ .../decompiler/exps/SwitchExprent.java | 32 ---- .../pkg/TestSwitchPatternMatching19.dec | 151 ++++++++------- .../pkg/TestSwitchPatternMatching21.dec | 174 +++++++++--------- 5 files changed, 202 insertions(+), 195 deletions(-) diff --git a/src/org/jetbrains/java/decompiler/main/rels/MethodProcessor.java b/src/org/jetbrains/java/decompiler/main/rels/MethodProcessor.java index c7ef4f4033..2ed58e2abf 100644 --- a/src/org/jetbrains/java/decompiler/main/rels/MethodProcessor.java +++ b/src/org/jetbrains/java/decompiler/main/rels/MethodProcessor.java @@ -326,6 +326,7 @@ public static RootStatement codeToJava(StructClass cl, StructMethod mt, MethodDe } if (changed) { + SwitchHelper.ensureDefaultCaseLast(root); continue; } } diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java b/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java index a1689a9905..0b92ceffc4 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java @@ -24,6 +24,45 @@ import java.util.stream.Collectors; public final class SwitchHelper { + public static void ensureDefaultCaseLast(Statement stat) { + for (Statement st : new ArrayList<>(stat.getStats())) { + ensureDefaultCaseLast(st); + } + + if (stat instanceof SwitchStatement switchStmt) { + ensureDefaultCaseLast(switchStmt); + } + } + + private static void ensureDefaultCaseLast(final SwitchStatement stat) { + // Ensure default case is last. Mostly needed when edges are merged and it loses it's initial ending position. + int defaultIdx = -1; + // First, find default case if it exists. + for (int i = 0; i < stat.getCaseStatements().size(); i++) { + List edges = stat.getCaseEdges().get(i); + + // As switch expressions can be compiled to a tableswitch, any gaps will contain a jump to the default element. + // Switch expressions cannot have a case point to the same statement as the default, so we check for default first and don't check for cases if it exists [TestConstructorSwitchExpression1] + + for (StatEdge edge : edges) { + if (edge == stat.getDefaultEdge()) { + defaultIdx = i; + break; + } + } + } + // Shift found default statement to end if needed + if (defaultIdx != -1 && defaultIdx != stat.getCaseEdges().size() - 1) { + stat.getCaseStatements().add(stat.getCaseStatements().remove(defaultIdx)); + stat.getCaseEdges().add(stat.getCaseEdges().remove(defaultIdx)); + stat.getCaseValues().add(stat.getCaseValues().remove(defaultIdx)); + Exprent removedGuard = stat.getCaseGuards().size() > defaultIdx ? stat.getCaseGuards().remove(defaultIdx) : null; + if (removedGuard != null) { + stat.getCaseGuards().add(removedGuard); + } + } + } + public static boolean simplifySwitches(Statement stat, StructMethod mt, RootStatement root) { boolean ret = false; if (stat instanceof SwitchStatement) { diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java index 6078611380..e1c223485e 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java @@ -43,38 +43,6 @@ public TextBuffer toJava(int indent) { buf.append(this.backing.getHeadexprent().toJava(indent)).append(" {").appendLineSeparator(); - // Ensure default case is last. Mostly needed when edges are merged and it loses it's initial ending position. - int defaultIdx = -1; - // First, find default case if it exists. - for (int i = 0; i < this.backing.getCaseStatements().size(); i++) { - List edges = this.backing.getCaseEdges().get(i); - - // As switch expressions can be compiled to a tableswitch, any gaps will contain a jump to the default element. - // Switch expressions cannot have a case point to the same statement as the default, so we check for default first and don't check for cases if it exists [TestConstructorSwitchExpression1] - - for (StatEdge edge : edges) { - if (edge == this.backing.getDefaultEdge()) { - if (isExhaustive) { - if (isSyntheticThrowEdge(edge)) { - break; // just don't mark as default - } - } - defaultIdx = i; - break; - } - } - } - // Shift found default statement to end if needed - if (defaultIdx != -1 && defaultIdx != this.backing.getCaseEdges().size() - 1) { - this.backing.getCaseStatements().add(this.backing.getCaseStatements().remove(defaultIdx)); - this.backing.getCaseEdges().add(this.backing.getCaseEdges().remove(defaultIdx)); - this.backing.getCaseValues().add(this.backing.getCaseValues().remove(defaultIdx)); - Exprent removedGuard = this.backing.getCaseGuards().size() > defaultIdx ? this.backing.getCaseGuards().remove(defaultIdx) : null; - if (removedGuard != null) { - this.backing.getCaseGuards().add(removedGuard); - } - } - for (int i = 0; i < this.backing.getCaseStatements().size(); i++) { Statement stat = this.backing.getCaseStatements().get(i); List edges = this.backing.getCaseEdges().get(i); diff --git a/testData/results/pkg/TestSwitchPatternMatching19.dec b/testData/results/pkg/TestSwitchPatternMatching19.dec index 6661b91b7d..76a5a64f11 100644 --- a/testData/results/pkg/TestSwitchPatternMatching19.dec +++ b/testData/results/pkg/TestSwitchPatternMatching19.dec @@ -5,10 +5,6 @@ public class TestSwitchPatternMatching19 { static void test(TestSwitchPatternMatching19.XXX s) { var1; switch (s) {// 5 - case null: - default: - System.out.println("f");// 11 - break; case X1: System.out.println("x1");// 6 break; @@ -22,6 +18,10 @@ public class TestSwitchPatternMatching19 { System.out.println("d"); break; case TestSwitchPatternMatching19.XXX var11 when false: + break; + case null: + default: + System.out.println("f");// 11 } }// 13 @@ -114,73 +114,72 @@ class 'pkg/TestSwitchPatternMatching19' { 2d 6 2e 6 2f 6 - 30 12 - 31 12 - 32 12 - 33 12 - 34 12 - 35 12 - 36 12 - 37 12 - 38 13 - 3b 14 - 3d 14 - 3e 14 - 3f 14 - 40 14 - 41 14 - 42 14 - 43 14 - 49 15 - 4a 15 - 4b 15 - 4c 15 - 4d 15 - 4e 15 - 4f 15 - 50 15 - 51 16 - 54 17 - 57 17 - 58 17 - 59 17 - 5a 17 - 5b 17 - 64 18 - 65 18 - 66 18 - 67 18 - 68 18 - 69 18 - 6a 18 - 6b 18 - 6c 19 - 6f 20 - 72 20 - 73 20 - 74 20 - 75 20 - 76 20 - 77 20 - 78 20 - 7e 21 - 7f 21 - 80 21 - 81 21 - 82 21 - 83 21 - 84 21 - 85 21 - 86 22 - 94 9 - 95 9 - 96 9 - 97 9 - 98 9 - 99 9 - 9a 9 - 9b 9 - 9c 10 + 30 8 + 31 8 + 32 8 + 33 8 + 34 8 + 35 8 + 36 8 + 37 8 + 38 9 + 3b 10 + 3d 10 + 3e 10 + 3f 10 + 40 10 + 41 10 + 42 10 + 43 10 + 49 11 + 4a 11 + 4b 11 + 4c 11 + 4d 11 + 4e 11 + 4f 11 + 50 11 + 51 12 + 54 13 + 57 13 + 58 13 + 59 13 + 5a 13 + 5b 13 + 64 14 + 65 14 + 66 14 + 67 14 + 68 14 + 69 14 + 6a 14 + 6b 14 + 6c 15 + 6f 16 + 72 16 + 73 16 + 74 16 + 75 16 + 76 16 + 77 16 + 78 16 + 7e 17 + 7f 17 + 80 17 + 81 17 + 82 17 + 83 17 + 84 17 + 85 17 + 86 18 + 94 23 + 95 23 + 96 23 + 97 23 + 98 23 + 99 23 + 9a 23 + 9b 23 9f 25 } @@ -262,11 +261,11 @@ class 'pkg/TestSwitchPatternMatching19' { Lines mapping: 5 <-> 7 -6 <-> 13 -7 <-> 15 -8 <-> 18 -9 <-> 21 -11 <-> 10 +6 <-> 9 +7 <-> 11 +8 <-> 14 +9 <-> 17 +11 <-> 24 13 <-> 26 16 <-> 29 17 <-> 35 diff --git a/testData/results/pkg/TestSwitchPatternMatching21.dec b/testData/results/pkg/TestSwitchPatternMatching21.dec index e94a79326a..2e99a40bd6 100644 --- a/testData/results/pkg/TestSwitchPatternMatching21.dec +++ b/testData/results/pkg/TestSwitchPatternMatching21.dec @@ -30,10 +30,6 @@ public class TestSwitchPatternMatching21 { public void test2(String it) { switch (it) {// 19 - case null: - default: - System.out.println(it + "?");// 26 27 - break; case "": System.out.println("nothing");// 20 break; @@ -45,6 +41,10 @@ public class TestSwitchPatternMatching21 { break; case String var8 when Math.random() > 0.0: System.out.println(it + "!!");// 24 25 + break; + case null: + default: + System.out.println(it + "?");// 26 27 } }// 29 @@ -236,81 +236,81 @@ class 'pkg/TestSwitchPatternMatching21' { 29 31 2a 31 2b 31 - 2c 37 - 2d 37 - 2e 37 - 2f 37 - 30 37 - 31 37 - 32 37 - 33 37 - 34 38 - 37 40 - 38 40 - 39 40 - 3a 40 - 3b 40 - 3c 40 - 3d 40 - 3e 40 - 3f 41 - 42 43 - 45 42 - 46 42 - 47 42 - 48 42 - 49 42 - 52 43 - 53 43 - 54 43 - 55 43 - 56 43 - 57 43 - 58 43 - 59 43 - 5a 43 - 5b 43 - 5c 43 - 5d 43 - 5e 43 - 5f 44 - 62 46 - 65 45 - 66 45 - 67 45 - 68 45 - 69 45 - 6a 45 - 6b 45 - 6c 45 - 72 46 - 73 46 - 74 46 - 75 46 - 76 46 - 77 46 - 78 46 - 79 46 - 7a 46 - 7b 46 - 7c 46 - 7d 46 - 7e 46 - 82 34 - 85 34 - 86 34 - 87 34 - 88 34 - 89 34 - 8a 34 - 8b 34 - 8c 34 - 8d 34 - 8e 34 - 8f 34 - 90 34 - 91 34 - 92 35 + 2c 33 + 2d 33 + 2e 33 + 2f 33 + 30 33 + 31 33 + 32 33 + 33 33 + 34 34 + 37 36 + 38 36 + 39 36 + 3a 36 + 3b 36 + 3c 36 + 3d 36 + 3e 36 + 3f 37 + 42 39 + 45 38 + 46 38 + 47 38 + 48 38 + 49 38 + 52 39 + 53 39 + 54 39 + 55 39 + 56 39 + 57 39 + 58 39 + 59 39 + 5a 39 + 5b 39 + 5c 39 + 5d 39 + 5e 39 + 5f 40 + 62 42 + 65 41 + 66 41 + 67 41 + 68 41 + 69 41 + 6a 41 + 6b 41 + 6c 41 + 72 42 + 73 42 + 74 42 + 75 42 + 76 42 + 77 42 + 78 42 + 79 42 + 7a 42 + 7b 42 + 7c 42 + 7d 42 + 7e 42 + 7f 43 + 82 46 + 85 46 + 86 46 + 87 46 + 88 46 + 89 46 + 8a 46 + 8b 46 + 8c 46 + 8d 46 + 8e 46 + 8f 46 + 90 46 + 91 46 95 48 } @@ -409,14 +409,14 @@ Lines mapping: 14 <-> 27 16 <-> 29 19 <-> 32 -20 <-> 38 -21 <-> 41 -22 <-> 44 -23 <-> 44 -24 <-> 47 -25 <-> 47 -26 <-> 35 -27 <-> 35 +20 <-> 34 +21 <-> 37 +22 <-> 40 +23 <-> 40 +24 <-> 43 +25 <-> 43 +26 <-> 47 +27 <-> 47 29 <-> 49 32 <-> 52 33 <-> 55