From a62cc3f70953ca957ee0b93bc38f7e75dfbfda1a Mon Sep 17 00:00:00 2001 From: "Egor.Ushakov" Date: Tue, 25 Apr 2017 17:39:52 +0300 Subject: [PATCH] IDEA-127499 Decompiler doesn't support switch over enums --- .../modules/decompiler/SwitchHelper.java | 94 ++++++++++++++++++ .../modules/decompiler/exps/ConstExprent.java | 7 ++ .../decompiler/exps/SwitchExprent.java | 14 +-- .../decompiler/stats/SwitchStatement.java | 38 ++++--- .../java/decompiler/SingleClassesTest.java | 3 +- testData/classes/pkg/TestSwitchOnEnum$1.class | Bin 618 -> 700 bytes testData/classes/pkg/TestSwitchOnEnum.class | Bin 557 -> 711 bytes .../classes/pkg/TestSwitchOnStrings.class | Bin 0 -> 672 bytes testData/results/TestSwitchOnEnum.dec | 33 +++--- testData/results/TestSwitchOnStrings.dec | 13 +++ testData/src/pkg/TestSwitchOnEnum.java | 2 + testData/src/pkg/TestSwitchOnStrings.java | 13 +++ 12 files changed, 183 insertions(+), 34 deletions(-) create mode 100644 src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java create mode 100644 testData/classes/pkg/TestSwitchOnStrings.class create mode 100644 testData/results/TestSwitchOnStrings.dec create mode 100644 testData/src/pkg/TestSwitchOnStrings.java diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java b/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java new file mode 100644 index 0000000..01d56e2 --- /dev/null +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/SwitchHelper.java @@ -0,0 +1,94 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.java.decompiler.modules.decompiler; + +import org.jetbrains.java.decompiler.code.CodeConstants; +import org.jetbrains.java.decompiler.main.ClassesProcessor; +import org.jetbrains.java.decompiler.main.DecompilerContext; +import org.jetbrains.java.decompiler.main.extern.IFernflowerLogger; +import org.jetbrains.java.decompiler.main.rels.MethodWrapper; +import org.jetbrains.java.decompiler.modules.decompiler.exps.*; +import org.jetbrains.java.decompiler.modules.decompiler.sforms.DirectGraph; +import org.jetbrains.java.decompiler.modules.decompiler.stats.SwitchStatement; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class SwitchHelper { + public static void simplify(SwitchStatement switchStatement) { + SwitchExprent switchExprent = (SwitchExprent)switchStatement.getHeadexprent(); + Exprent value = switchExprent.getValue(); + if (isEnumArray(value)) { + List> caseValues = switchStatement.getCaseValues(); + Map mapping = new HashMap<>(caseValues.size()); + ArrayExprent array = (ArrayExprent)value; + ClassesProcessor.ClassNode classNode = + DecompilerContext.getClassProcessor().getMapRootClasses().get(((FieldExprent)array.getArray()).getClassname()); + if (classNode != null) { + MethodWrapper wrapper = classNode.getWrapper().getMethodWrapper(CodeConstants.CLINIT_NAME, "()V"); + if (wrapper != null) { + wrapper.getOrBuildGraph().iterateExprents(new DirectGraph.ExprentIterator() { + @Override + public int processExprent(Exprent exprent) { + if (exprent instanceof AssignmentExprent) { + AssignmentExprent assignment = (AssignmentExprent)exprent; + Exprent left = assignment.getLeft(); + if (isEnumArray(left)) { + mapping.put(assignment.getRight(), ((InvocationExprent)((ArrayExprent)left).getIndex()).getInstance()); + } + } + return 0; + } + }); + } + } + + List> realCaseValues = new ArrayList<>(caseValues.size()); + for (List caseValue : caseValues) { + List values = new ArrayList<>(caseValue.size()); + realCaseValues.add(values); + for (Exprent exprent : caseValue) { + if (exprent == null) { + values.add(null); + } + else { + Exprent realConst = mapping.get(exprent); + if (realConst == null) { + DecompilerContext.getLogger() + .writeMessage("Unable to simplify switch on enum: " + exprent + " not found, available: " + mapping, + IFernflowerLogger.Severity.ERROR); + return; + } + values.add(realConst.copy()); + } + } + } + caseValues.clear(); + caseValues.addAll(realCaseValues); + switchExprent.replaceExprent(value, ((InvocationExprent)array.getIndex()).getInstance().copy()); + } + } + + private static boolean isEnumArray(Exprent exprent) { + if (exprent instanceof ArrayExprent) { + Exprent field = ((ArrayExprent)exprent).getArray(); + return field instanceof FieldExprent && ((FieldExprent)field).getName().startsWith("$SwitchMap"); + } + return false; + } +} diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/ConstExprent.java b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/ConstExprent.java index 962bb77..d4ed0c3 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/ConstExprent.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/ConstExprent.java @@ -300,6 +300,13 @@ public class ConstExprent extends Exprent { InterpreterUtil.equalObjects(value, cn.getValue()); } + @Override + public int hashCode() { + int result = constType != null ? constType.hashCode() : 0; + result = 31 * result + (value != null ? value.hashCode() : 0); + return result; + } + public boolean hasBooleanValue() { switch (constType.type) { case CodeConstants.TYPE_BOOLEAN: 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 27730a9..cadf94f 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/exps/SwitchExprent.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2017 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,7 +28,7 @@ import java.util.Set; public class SwitchExprent extends Exprent { private Exprent value; - private List> caseValues = new ArrayList<>(); + private List> caseValues = new ArrayList<>(); public SwitchExprent(Exprent value, Set bytecodeOffsets) { super(EXPRENT_SWITCH); @@ -41,8 +41,8 @@ public class SwitchExprent extends Exprent { public Exprent copy() { SwitchExprent swExpr = new SwitchExprent(value.copy(), bytecode); - List> lstCaseValues = new ArrayList<>(); - for (List lst : caseValues) { + List> lstCaseValues = new ArrayList<>(); + for (List lst : caseValues) { lstCaseValues.add(new ArrayList<>(lst)); } swExpr.setCaseValues(lstCaseValues); @@ -63,8 +63,8 @@ public class SwitchExprent extends Exprent { result.addMaxTypeExprent(value, VarType.VARTYPE_INT); VarType valType = value.getExprType(); - for (List lst : caseValues) { - for (ConstExprent expr : lst) { + for (List lst : caseValues) { + for (Exprent expr : lst) { if (expr != null) { VarType caseType = expr.getExprType(); if (!caseType.equals(valType)) { @@ -116,7 +116,7 @@ public class SwitchExprent extends Exprent { return value; } - public void setCaseValues(List> caseValues) { + public void setCaseValues(List> caseValues) { this.caseValues = caseValues; } } diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/stats/SwitchStatement.java b/src/org/jetbrains/java/decompiler/modules/decompiler/stats/SwitchStatement.java index dd92238..bcef5fe 100644 --- a/src/org/jetbrains/java/decompiler/modules/decompiler/stats/SwitchStatement.java +++ b/src/org/jetbrains/java/decompiler/modules/decompiler/stats/SwitchStatement.java @@ -24,8 +24,10 @@ import org.jetbrains.java.decompiler.main.collectors.CounterContainer; import org.jetbrains.java.decompiler.modules.decompiler.DecHelper; import org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor; import org.jetbrains.java.decompiler.modules.decompiler.StatEdge; +import org.jetbrains.java.decompiler.modules.decompiler.SwitchHelper; import org.jetbrains.java.decompiler.modules.decompiler.exps.ConstExprent; import org.jetbrains.java.decompiler.modules.decompiler.exps.Exprent; +import org.jetbrains.java.decompiler.modules.decompiler.exps.FieldExprent; import org.jetbrains.java.decompiler.modules.decompiler.exps.SwitchExprent; import org.jetbrains.java.decompiler.struct.gen.VarType; @@ -41,7 +43,7 @@ public class SwitchStatement extends Statement { private List> caseEdges = new ArrayList<>(); - private List> caseValues = new ArrayList<>(); + private List> caseValues = new ArrayList<>(); private StatEdge default_edge; @@ -108,6 +110,8 @@ public class SwitchStatement extends Statement { } public TextBuffer toJava(int indent, BytecodeMappingTracer tracer) { + SwitchHelper.simplify(this); + TextBuffer buf = new TextBuffer(); buf.append(ExprProcessor.listToJava(varDefinitions, indent, tracer)); buf.append(first.toJava(indent, tracer)); @@ -126,7 +130,7 @@ public class SwitchStatement extends Statement { Statement stat = caseStatements.get(i); List edges = caseEdges.get(i); - List values = caseValues.get(i); + List values = caseValues.get(i); for (int j = 0; j < edges.size(); j++) { if (edges.get(j) == default_edge) { @@ -134,10 +138,20 @@ public class SwitchStatement extends Statement { tracer.incrementCurrentSourceLine(); } else { - ConstExprent value = (ConstExprent)values.get(j).copy(); - value.setConstType(switch_type); + buf.appendIndent(indent).append("case "); + Exprent value = values.get(j); + if (value instanceof ConstExprent) { + value = value.copy(); + ((ConstExprent)value).setConstType(switch_type); + } + if (value instanceof FieldExprent && ((FieldExprent)value).isStatic()) { // enum values + buf.append(((FieldExprent)value).getName()); + } + else { + buf.append(value.toJava(indent, tracer)); + } - buf.appendIndent(indent).append("case ").append(value.toJava(indent, tracer)).append(":").appendLineSeparator(); + buf.append(":").appendLineSeparator(); tracer.incrementCurrentSourceLine(); } } @@ -211,8 +225,8 @@ public class SwitchStatement extends Statement { BasicBlockStatement bbstat = (BasicBlockStatement)first; int[] values = ((SwitchInstruction)bbstat.getBlock().getLastInstruction()).getValues(); - List nodes = new ArrayList<>(); - List> edges = new ArrayList<>(); + List nodes = new ArrayList<>(stats.size() - 1); + List> edges = new ArrayList<>(stats.size() - 1); // collect regular edges for (int i = 1; i < stats.size(); i++) { @@ -293,12 +307,12 @@ public class SwitchStatement extends Statement { } // translate indices back into edges - List> lstEdges = new ArrayList<>(); - List> lstValues = new ArrayList<>(); + List> lstEdges = new ArrayList<>(edges.size()); + List> lstValues = new ArrayList<>(edges.size()); for (List lst : edges) { - List lste = new ArrayList<>(); - List lstv = new ArrayList<>(); + List lste = new ArrayList<>(lst.size()); + List lstv = new ArrayList<>(lst.size()); List lstSuccs = first.getSuccessorEdges(STATEDGE_DIRECT_ALL); for (Integer in : lst) { @@ -362,7 +376,7 @@ public class SwitchStatement extends Statement { return default_edge; } - public List> getCaseValues() { + public List> getCaseValues() { return caseValues; } } diff --git a/test/org/jetbrains/java/decompiler/SingleClassesTest.java b/test/org/jetbrains/java/decompiler/SingleClassesTest.java index 0ce7f87..505fe81 100644 --- a/test/org/jetbrains/java/decompiler/SingleClassesTest.java +++ b/test/org/jetbrains/java/decompiler/SingleClassesTest.java @@ -105,7 +105,8 @@ public class SingleClassesTest { @Test public void testClashName() { doTest("pkg/TestClashName", "pkg/SharedName1", "pkg/SharedName2", "pkg/SharedName3", "pkg/SharedName4", "pkg/NonSharedName", "pkg/TestClashNameParent", "ext/TestClashNameParent","pkg/TestClashNameIface", "ext/TestClashNameIface"); } - @Test public void testSwitchOnEnum() { doTest("pkg/TestSwitchOnEnum","pkg/TestSwitchOnEnum$1");} + @Test public void testSwitchOnEnum() { doTest("pkg/TestSwitchOnEnum");} + //@Test public void TestSwitchOnStrings() { doTest("pkg/TestSwitchOnStrings");} @Test public void testVarArgCalls() { doTest("pkg/TestVarArgCalls"); } private void doTest(String testFile, String... companionFiles) { diff --git a/testData/classes/pkg/TestSwitchOnEnum$1.class b/testData/classes/pkg/TestSwitchOnEnum$1.class index a3f6924427ab5cdb2e3ff4e01e70e4a567a02156..aebc1c568ce9e3d2e92dc364bfca59486d4c7ff1 100644 GIT binary patch delta 363 zcmW-b%}T>S6ot>UNv4U@8f)#Jf7RL_LFmR!5kaUGMB9K7-MATJ(bixks9PVvRf(@4 zu2m3`(g*NSdaUu~>!{QP}S0BmC2LKZm_9C?#mL1B?&Ucmy#BFFNm zc2-Rv4n66g$e=IJ{ob(>hNOEZz3Yy=jhL23&&}-;$BMuz$^s@Vfi+YFs;Ds(2l7ES ze0g)(=nma`?`p5#^Do*#Fbo9hs4!&19hM0T?8y+F{nn8azOd#a19#gkr@QOgB|0-f zXEQsm!j&VQf}oxx8X!&%sDFV?#?}X7@9M=rp=r)5w1Wr@LOsGa#3(0&9CYMSrZaU) sNKt|j<)ko;Ezl5PQweCN95d)Bp}{o$#V|_&fQC8JB`S+nId#~He>)jB<^TWy delta 248 zcmW-by-osA5QV?HT=uf-MHl>AR22M)4W$h)U}KDhCN?%Jq0+*_-e=hG1~hagYGRBb z^zbM?gnBN?WX?I6GiPR6{jsNu-|rc4#!1Qsn+Y*niP(0KxufOSaqN}7yGH%`xfncL z7Ed?D{o_!h^0lA09s8aGI-VMdr^}(|h~x6FJ?fgVaih8EzhoWdI^8i@9(;ibQ&xmt z>YR!gw7xL2Y5GC*9;)(*wO14D#hb>$~Gt4||-+Jxm@8c7|0bC0@*0M0LZeatPCWu&^^}}xOWYiu_u3Ayh$Q}tK6jZ#2Qz=O+dOVz~ zplV{vMh)B1@7H}3w6I^Vze8R~6wGrP=`w>Hxji*t0eLFHA_|-~`GjtTSbv4G_Xf4` zM8Q|(PxeW0amLGfti@_frKSx9XynH^xQyLlD{24$ delta 210 zcmXAhOAf(c6otRPub-;6Xgyn!4iO_UGL=|??N~!&Y6oIa0}=xxiNt2?K-|_nx#yny zIxqWdW$*9(0W6ug*mUYRL@qI1O;6L;4DxJ~ALnY9f7Xp5BTeFwGCr)-bDyFFU{7zT zLWH6z?EHtqK7p_&vn_@I#utp)6?1WtMk1ChEy|3F(!;1cd`!_EiFl-h0?2f@ac-MuOGm z*=e_S7><(mnFGg zDmXn*XX!Ktw*_u{FdhcsqexR)`Nzb4J;FmCbIkW}2OHdMGMRYT#1`G@bShZ>4_!g= z55rdHBn*-(P)1si@2OEQdnmpdSFpqLvFdY=D?4T#>Xl<`z~&!l$${mVqjnk9Xoc3= z{RH`tUSFfOQ_z4%-^&tEz&s=V42smc_#>N{v@iA!a*gT*a_<>X?(uNUw+U?-^8Fd@ zZq+v~V7|qI(RhdXs&A!{Wj5B*s`a#nic;dyq{`OJoEbBE1~e-_oo3-FS2^Yi&xvY= jOPod)mkhOcgSmnQqD9z0vF%_{3T{b$5ak;Y&td5|&dq`) literal 0 HcmV?d00001 diff --git a/testData/results/TestSwitchOnEnum.dec b/testData/results/TestSwitchOnEnum.dec index 4a6d980..33b0464 100644 --- a/testData/results/TestSwitchOnEnum.dec +++ b/testData/results/TestSwitchOnEnum.dec @@ -5,27 +5,32 @@ import java.util.concurrent.TimeUnit; public class TestSwitchOnEnum { int myInt; - public void testSOE(TimeUnit var1) { - switch(null.$SwitchMap$java$util$concurrent$TimeUnit[var1.ordinal()]) {// 12 - case 1: - return;// 14 + public int testSOE(TimeUnit t) { + switch(t) {// 14 + case MICROSECONDS: + return 2;// 16 + case SECONDS: + return 1;// 18 default: + return 0;// 20 } - }// 16 + } } class 'pkg/TestSwitchOnEnum' { - method 'testSOE (Ljava/util/concurrent/TimeUnit;)V' { - 0 8 - 4 8 - 7 8 + method 'testSOE (Ljava/util/concurrent/TimeUnit;)I' { 8 8 - 1c 10 - 1d 13 + 24 10 + 25 10 + 26 12 + 27 12 + 28 14 + 29 14 } } Lines mapping: -12 <-> 9 -14 <-> 11 -16 <-> 14 +14 <-> 9 +16 <-> 11 +18 <-> 13 +20 <-> 15 diff --git a/testData/results/TestSwitchOnStrings.dec b/testData/results/TestSwitchOnStrings.dec new file mode 100644 index 0000000..cd120e8 --- /dev/null +++ b/testData/results/TestSwitchOnStrings.dec @@ -0,0 +1,13 @@ +package pkg; + +public class TestSwitchOnStrings { + public int testSOE(String s) { + switch (s) { + case "xxx": + return 2; + case "yyy": + return 1; + } + return 0; + } +} diff --git a/testData/src/pkg/TestSwitchOnEnum.java b/testData/src/pkg/TestSwitchOnEnum.java index ec07604..e26d407 100644 --- a/testData/src/pkg/TestSwitchOnEnum.java +++ b/testData/src/pkg/TestSwitchOnEnum.java @@ -12,6 +12,8 @@ public class TestSwitchOnEnum { public int testSOE(TimeUnit t) { // This creates anonymous SwitchMap inner class. switch (t) { + case MICROSECONDS: + return 2; case SECONDS: return 1; } diff --git a/testData/src/pkg/TestSwitchOnStrings.java b/testData/src/pkg/TestSwitchOnStrings.java new file mode 100644 index 0000000..cd120e8 --- /dev/null +++ b/testData/src/pkg/TestSwitchOnStrings.java @@ -0,0 +1,13 @@ +package pkg; + +public class TestSwitchOnStrings { + public int testSOE(String s) { + switch (s) { + case "xxx": + return 2; + case "yyy": + return 1; + } + return 0; + } +}