Skip to content

Commit 960c98a

Browse files
coehlrichcoehlrich
authored
Fix incorrectly renaming pattern match variables (#555)
* Fix incorrectly renaming pattern match variables * Add test and indent switch
1 parent f6d13ef commit 960c98a

5 files changed

Lines changed: 571 additions & 93 deletions

File tree

‎src/org/jetbrains/java/decompiler/modules/decompiler/vars/VarDefinitionHelper.java‎

Lines changed: 160 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
import org.jetbrains.java.decompiler.main.collectors.VarNamesCollector;
99
import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences;
1010
import org.jetbrains.java.decompiler.main.rels.MethodWrapper;
11-
import org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor;
12-
import org.jetbrains.java.decompiler.modules.decompiler.StackVarsProcessor;
13-
import org.jetbrains.java.decompiler.modules.decompiler.ValidationHelper;
11+
import org.jetbrains.java.decompiler.modules.decompiler.*;
1412
import org.jetbrains.java.decompiler.modules.decompiler.exps.*;
1513
import org.jetbrains.java.decompiler.modules.decompiler.flow.DirectGraph;
1614
import org.jetbrains.java.decompiler.modules.decompiler.flow.FlattenStatementsHelper;
@@ -1593,7 +1591,7 @@ private void iterateClashingNames(Statement stat, StructMethod mt, Map<Statement
15931591
// Process var definitions as owned by the parent- they come before the statement, and so their scope extends past the actual statement.
15941592
for (Exprent exprent : stat.getVarDefinitions()) {
15951593
Set<VarInMethod> upDefs = new HashSet<>();
1596-
iterateClashingExprent(stat, mt, varDefinitions, exprent, liveVarDefs, upDefs, nameMap, seenMethods);
1594+
iterateClashingExprent(stat, mt, varDefinitions, exprent, liveVarDefs, upDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
15971595
liveVarDefs.addAll(upDefs);
15981596
varDefinitions.computeIfAbsent(stat.getParent(), parent -> new HashSet<>()).addAll(upDefs);
15991597
}
@@ -1603,44 +1601,109 @@ private void iterateClashingNames(Statement stat, StructMethod mt, Map<Statement
16031601
Set<VarInMethod> upDefs = new HashSet<>();
16041602
BasicBlockStatement basic = stat.getBasichead();
16051603
for (Exprent exprent : basic.getExprents()) {
1606-
for (Exprent ex : exprent.getAllExprents(true, true)) {
1607-
iterateClashingExprent(basic, mt, varDefinitions, ex, liveVarDefs, upDefs, nameMap, seenMethods);
1608-
}
1604+
iterateClashingExprent(basic, mt, varDefinitions, exprent, liveVarDefs, upDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
16091605
}
16101606

16111607
liveVarDefs.addAll(upDefs);
16121608
varDefinitions.computeIfAbsent(stat.getParent(), parent -> new HashSet<>()).addAll(upDefs);
16131609
}
16141610

16151611
// If this is a basic block, iterate all exprents
1612+
Map<Statement, Set<VarInMethod>> statementVars = new HashMap<>();
1613+
Set<VarInMethod> post = new HashSet<>();
16161614
if (stat.getExprents() != null) {
16171615
for (Exprent exprent : stat.getExprents()) {
1618-
for (Exprent ex : exprent.getAllExprents(true, true)) {
1619-
// Sort order from getAllExprents here is crucial!
1620-
// Say, for example, "MyType t = method(t -> ....);"
1621-
// It is imperative that the lhs of the assign comes first, so that any defs in the rhs can be properly seen.
1622-
iterateClashingExprent(stat, mt, varDefinitions, ex, liveVarDefs, curVarDefs, nameMap, seenMethods);
1623-
}
1616+
// Sort order here is crucial!
1617+
// Say, for example, "MyType t = method(t -> ....);"
1618+
// It is imperative that the lhs of the assign comes first, so that any defs in the rhs can be properly seen.
1619+
iterateClashingExprent(stat, mt, varDefinitions, exprent, liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
16241620
}
16251621
} else {
1626-
if (stat instanceof CatchStatement) {
1622+
if (stat instanceof CatchStatement catchStat) {
16271623
// Resources and catch vars are only in context in certain statements
1624+
for (int i = 0; i < catchStat.getStats().size(); i++) {
1625+
Statement st = catchStat.getStats().get(i);
1626+
Set<VarInMethod> stVars = new HashSet<>();
1627+
if (i == 0) {
1628+
// Process var definitions in resources
1629+
for (Exprent exp : catchStat.getResources()) {
1630+
iterateClashingExprent(st, mt, varDefinitions, exp, liveVarDefs, stVars, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1631+
}
1632+
} else {
1633+
// Process var definitions in catch
1634+
VarExprent exp = catchStat.getVars().get(i - 1);
1635+
iterateClashingExprent(st, mt, varDefinitions, exp, liveVarDefs, stVars, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1636+
}
1637+
liveVarDefs.removeAll(stVars);
1638+
statementVars.put(st, stVars);
1639+
}
16281640
} else if (stat instanceof SwitchStatement switchStat) {
16291641
// Variables defined in case values and case guards are only in context in their respective statements
1630-
Exprent exp = switchStat.getHeadexprent();
1631-
List<Exprent> exprents = exp.getAllExprents(true, true);
1642+
Exprent headExp = switchStat.getHeadexprent();
1643+
iterateClashingExprent(stat, mt, varDefinitions, headExp, liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1644+
for (int i = 0; i < switchStat.getCaseStatements().size(); i++) {
1645+
Statement st = switchStat.getCaseStatements().get(i);
1646+
Set<VarInMethod> stVars = new HashSet<>();
1647+
// Process var definitions in case values
1648+
for (Exprent exp : switchStat.getCaseValues().get(i)) {
1649+
if (exp != null) {
1650+
iterateClashingExprent(st, mt, varDefinitions, exp, liveVarDefs, stVars, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1651+
}
1652+
}
1653+
// Process var definitions in case guard
1654+
if (switchStat.getCaseGuards().size() > i) {
1655+
Exprent exp = switchStat.getCaseGuards().get(i);
1656+
if (exp != null) {
1657+
iterateClashingExprent(st, mt, varDefinitions, exp, liveVarDefs, stVars, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1658+
}
1659+
}
1660+
liveVarDefs.removeAll(stVars);
1661+
statementVars.put(st, stVars);
1662+
}
1663+
} else if (stat instanceof IfStatement ifStat) {
1664+
Set<VarInMethod> whenTrue = new HashSet<>();
1665+
Set<VarInMethod> whenFalse = new HashSet<>();
1666+
iterateClashingExprent(stat, mt, varDefinitions, ifStat.getHeadexprent().getCondition(), liveVarDefs, curVarDefs, nameMap, seenMethods, whenTrue, whenFalse);
1667+
boolean ifCompleteNormally = ifStat.getIfstat() == null || !ifStat.getIfstat().getSuccessorEdges(StatEdge.TYPE_REGULAR).isEmpty();
1668+
boolean elseCompleteNormally = ifStat.getElsestat() == null || !ifStat.getElsestat().getSuccessorEdges(StatEdge.TYPE_REGULAR).isEmpty();
1669+
if (ifStat.getIfstat() != null) {
1670+
statementVars.put(ifStat.getIfstat(), whenTrue);
1671+
}
1672+
1673+
if (ifStat.getElsestat() != null) {
1674+
statementVars.put(ifStat.getElsestat(), whenFalse);
1675+
}
16321676

1633-
for (Exprent exprent : exprents) {
1634-
iterateClashingExprent(stat, mt, varDefinitions, exprent, liveVarDefs, curVarDefs, nameMap, seenMethods);
1677+
if (!ifCompleteNormally && elseCompleteNormally) {
1678+
post = whenFalse;
1679+
} else if (ifCompleteNormally && !elseCompleteNormally) {
1680+
post = whenTrue;
1681+
}
1682+
} else if (stat instanceof DoStatement doStat) {
1683+
Set<VarInMethod> whenTrue = new HashSet<>();
1684+
Set<VarInMethod> whenFalse = new HashSet<>();
1685+
if (doStat.getInitExprent() != null) {
1686+
iterateClashingExprent(stat, mt, varDefinitions, doStat.getInitExprent(), liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1687+
}
1688+
if (doStat.getConditionExprent() != null) {
1689+
iterateClashingExprent(stat, mt, varDefinitions, doStat.getConditionExprent(), liveVarDefs, curVarDefs, nameMap, seenMethods, whenTrue, whenFalse);
1690+
}
1691+
if (doStat.getIncExprent() != null) {
1692+
liveVarDefs.addAll(whenTrue);
1693+
iterateClashingExprent(doStat, mt, varDefinitions, doStat.getIncExprent(), liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
1694+
liveVarDefs.removeAll(whenTrue);
1695+
}
1696+
1697+
if (doStat.getLooptype() == DoStatement.Type.WHILE || doStat.getLooptype() == DoStatement.Type.FOR) {
1698+
statementVars.put(doStat.getFirst(), whenTrue);
1699+
}
1700+
if (doStat.getSuccessorEdges(StatEdge.TYPE_BREAK).isEmpty()) {
1701+
post = whenFalse;
16351702
}
16361703
} else {
16371704
// Process var definitions in statement head
16381705
for (Exprent exp : stat.getStatExprents()) {
1639-
List<Exprent> exprents = exp.getAllExprents(true, true);
1640-
1641-
for (Exprent exprent : exprents) {
1642-
iterateClashingExprent(stat, mt, varDefinitions, exprent, liveVarDefs, curVarDefs, nameMap, seenMethods);
1643-
}
1706+
iterateClashingExprent(stat, mt, varDefinitions, exp, liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
16441707
}
16451708
}
16461709

@@ -1661,80 +1724,24 @@ private void iterateClashingNames(Statement stat, StructMethod mt, Map<Statement
16611724
}
16621725

16631726
List<Statement> deferred = new ArrayList<>();
1727+
boolean seperate = stat instanceof IfStatement || stat instanceof CatchStatement;
16641728
if (iterate) {
16651729
for (Statement st : stat.getStats()) {
16661730
Set<VarInMethod> stVars = new HashSet<>();
1667-
Set<VarInMethod> tmpVars = new HashSet<>();
1668-
boolean remove = false;
1731+
Set<VarInMethod> tmpVars = statementVars.getOrDefault(st, new HashSet<>());
16691732
if (stat instanceof IfStatement) {
1670-
IfStatement ifstat = (IfStatement)stat;
1671-
1672-
if (ifstat.getElsestat() == st) {
1673-
// Defer else blocks of if statements, as they are independent from the context of the if block
1674-
deferred.add(st);
1675-
continue;
1676-
}
1677-
16781733
// We've already looked at the head- don't look again!
16791734
if (st == stat.getBasichead()) {
16801735
continue;
16811736
}
1682-
} else if (stat instanceof CatchStatement catchStat) {
1683-
if (catchStat.getFirst() == st) {
1684-
// Process var definitions in resources
1685-
for (Exprent exp : catchStat.getResources()) {
1686-
List<Exprent> exprents = exp.getAllExprents(true, true);
1687-
1688-
for (Exprent exprent : exprents) {
1689-
iterateClashingExprent(st, mt, varDefinitions, exprent, liveVarDefs, stVars, nameMap, seenMethods);
1690-
}
1691-
}
1692-
remove = true;
1693-
} else if (catchStat.getStats().indexOf(st) > 0) {
1694-
// Process var definitions in catch
1695-
int i = catchStat.getStats().indexOf(st);
1696-
VarExprent exp = catchStat.getVars().get(i - 1);
1697-
List<Exprent> exprents = exp.getAllExprents(true, true);
1698-
1699-
for (Exprent exprent : exprents) {
1700-
iterateClashingExprent(st, mt, varDefinitions, exprent, liveVarDefs, stVars, nameMap, seenMethods);
1701-
}
1702-
remove = true;
1703-
}
1704-
} else if (stat instanceof SwitchStatement switchStat) {
1705-
int i = switchStat.getCaseStatements().indexOf(st);
1706-
if (i >= 0) {
1707-
// Process var definitions in case values
1708-
for (Exprent exp : switchStat.getCaseValues().get(i)) {
1709-
if (exp != null) {
1710-
List<Exprent> exprents = exp.getAllExprents(true, true);
1711-
1712-
for (Exprent exprent : exprents) {
1713-
iterateClashingExprent(st, mt, varDefinitions, exprent, liveVarDefs, tmpVars, nameMap, seenMethods);
1714-
}
1715-
}
1716-
}
1717-
// Process var definitions in case guard
1718-
if (switchStat.getCaseGuards().size() > i) {
1719-
Exprent exp = switchStat.getCaseGuards().get(i);
1720-
if (exp != null) {
1721-
List<Exprent> exprents = exp.getAllExprents(true, true);
1722-
1723-
for (Exprent exprent : exprents) {
1724-
iterateClashingExprent(st, mt, varDefinitions, exprent, liveVarDefs, tmpVars, nameMap, seenMethods);
1725-
}
1726-
}
1727-
}
1728-
}
17291737
}
17301738
liveVarDefs.addAll(tmpVars);
1731-
17321739
iterateClashingNames(st, mt, varDefinitions, liveVarDefs, stVars, nameMap, seenMethods);
1733-
if (remove) {
1740+
liveVarDefs.removeAll(tmpVars);
1741+
if (seperate) {
17341742
clearStatement(varDefinitions, liveVarDefs, nameMap, st);
1743+
clearStatement(varDefinitions, liveVarDefs, nameMap, stat);
17351744
}
1736-
liveVarDefs.removeAll(tmpVars);
1737-
tmpVars.forEach(nameMap::remove);
17381745
}
17391746
}
17401747

@@ -1751,7 +1758,10 @@ private void iterateClashingNames(Statement stat, StructMethod mt, Map<Statement
17511758
// Process deferred statements
17521759
if (iterate) {
17531760
for (Statement st : deferred) {
1761+
Set<VarInMethod> tmpVars = statementVars.getOrDefault(st, new HashSet<>());
1762+
liveVarDefs.addAll(tmpVars);
17541763
iterateClashingNames(st, mt, varDefinitions, liveVarDefs, new HashSet<>(), nameMap, seenMethods);
1764+
liveVarDefs.removeAll(tmpVars);
17551765
}
17561766
}
17571767

@@ -1760,6 +1770,11 @@ private void iterateClashingNames(Statement stat, StructMethod mt, Map<Statement
17601770
clearStatement(varDefinitions, liveVarDefs, nameMap, st);
17611771
}
17621772
}
1773+
1774+
if (!(stat instanceof RootStatement)) {
1775+
varDefinitions.computeIfAbsent(stat.getParent(), st -> new HashSet<>()).addAll(post);
1776+
liveVarDefs.addAll(post);
1777+
}
17631778
}
17641779

17651780
private void clearStatement(Map<Statement, Set<VarInMethod>> varDefinitions, Set<VarInMethod> liveVarDefs, Map<VarInMethod, String> nameMap, Statement st) {
@@ -1774,7 +1789,8 @@ private void clearStatement(Map<Statement, Set<VarInMethod>> varDefinitions, Set
17741789
}
17751790

17761791
private void iterateClashingExprent(Statement stat, StructMethod mt, Map<Statement, Set<VarInMethod>> varDefinitions, Exprent exprent,
1777-
Set<VarInMethod> liveVarDefs, Set<VarInMethod> curVarDefs, Map<VarInMethod, String> nameMap, Set<String> seenMethods) {
1792+
Set<VarInMethod> liveVarDefs, Set<VarInMethod> curVarDefs, Map<VarInMethod, String> nameMap, Set<String> seenMethods, Set<VarInMethod> whenTrue, Set<VarInMethod> whenFalse) {
1793+
boolean iterate = true;
17781794
if (exprent instanceof NewExprent) {
17791795
NewExprent newExprent = (NewExprent) exprent;
17801796
// Check if this is a lambda with a body
@@ -1803,7 +1819,7 @@ private void iterateClashingExprent(Statement stat, StructMethod mt, Map<Stateme
18031819
// Try to perform a rename
18041820
String name = mw.varproc.getVarName(vvp);
18051821
String original = name;
1806-
name = rename(nameMap, name);
1822+
name = rename(nameMap, name, liveVarDefs);
18071823

18081824
// Did we rename? If so, we should add it to the name map and set as clashing
18091825
if (original != null && !original.equals(name)) {
@@ -1857,7 +1873,7 @@ private void iterateClashingExprent(Statement stat, StructMethod mt, Map<Stateme
18571873
// Try to perform a rename
18581874
String name = mw.varproc.getVarName(vvp);
18591875
String original = name;
1860-
name = rename(nameMap, name);
1876+
name = rename(nameMap, name, liveVarDefs);
18611877

18621878
// Did we rename? If so, we should add it to the name map and set as clashing
18631879
if (original != null && !original.equals(name)) {
@@ -1892,14 +1908,16 @@ private void iterateClashingExprent(Statement stat, StructMethod mt, Map<Stateme
18921908
VarExprent var = (VarExprent) exprent;
18931909

18941910
if (var.isDefinition()) {
1895-
curVarDefs.add(new VarInMethod(var.getVarVersionPair(), mt));
1911+
VarInMethod def = new VarInMethod(var.getVarVersionPair(), mt);
1912+
curVarDefs.add(def);
1913+
liveVarDefs.add(def);
18961914

18971915
// Only process vars that have lvt as the default var<index>_<version> names can never conflict
18981916
if (var.getLVT() != null || this.varproc.getVarName(var.getVarVersionPair()) != null) {
18991917
String name = var.getLVT() == null ? this.varproc.getVarName(var.getVarVersionPair()) : var.getLVT().getName();
19001918

19011919
String originalName = name;
1902-
name = rename(nameMap, name);
1920+
name = rename(nameMap, name, liveVarDefs);
19031921

19041922
boolean scopedSwitch = false;
19051923
if (!originalName.equals(name)) {
@@ -1958,10 +1976,61 @@ private void iterateClashingExprent(Statement stat, StructMethod mt, Map<Stateme
19581976
}
19591977
}
19601978
}
1979+
1980+
if (exprent instanceof FunctionExprent function) {
1981+
iterate = false;
1982+
// https://docs.oracle.com/javase/specs/jls/se25/html/jls-6.html#jls-6.3.1
1983+
switch (function.getFuncType()) {
1984+
case BOOLEAN_AND -> {
1985+
for (Exprent exp : function.getLstOperands()) {
1986+
liveVarDefs.addAll(whenTrue);
1987+
iterateClashingExprent(stat, mt, varDefinitions, exp, liveVarDefs, curVarDefs, nameMap, seenMethods, whenTrue, new HashSet<>());
1988+
}
1989+
liveVarDefs.removeAll(whenTrue);
1990+
}
1991+
case BOOLEAN_OR -> {
1992+
for (Exprent exp : function.getLstOperands()) {
1993+
liveVarDefs.addAll(whenFalse);
1994+
iterateClashingExprent(stat, mt, varDefinitions, exp, liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), whenFalse);
1995+
}
1996+
liveVarDefs.removeAll(whenFalse);
1997+
}
1998+
case BOOL_NOT -> {
1999+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(0), liveVarDefs, curVarDefs, nameMap, seenMethods, whenFalse, whenTrue);
2000+
}
2001+
case TERNARY -> {
2002+
Set<VarInMethod> subWhenTrue = new HashSet<>();
2003+
Set<VarInMethod> subWhenFalse = new HashSet<>();
2004+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(0), liveVarDefs, curVarDefs, nameMap, seenMethods, subWhenTrue, subWhenFalse);
2005+
liveVarDefs.addAll(subWhenTrue);
2006+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(1), liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
2007+
liveVarDefs.removeAll(subWhenTrue);
2008+
liveVarDefs.addAll(subWhenFalse);
2009+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(2), liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
2010+
liveVarDefs.removeAll(subWhenFalse);
2011+
}
2012+
case INSTANCEOF -> {
2013+
if (function.getLstOperands().size() > 2) {
2014+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(0), liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
2015+
iterateClashingExprent(stat, mt, varDefinitions, function.getLstOperands().get(2), liveVarDefs, whenTrue, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
2016+
liveVarDefs.removeAll(whenTrue);
2017+
} else {
2018+
iterate = true;
2019+
}
2020+
}
2021+
default -> iterate = true;
2022+
}
2023+
}
2024+
2025+
if (iterate) {
2026+
for (Exprent exp : exprent.getAllExprents()) {
2027+
iterateClashingExprent(stat, mt, varDefinitions, exp, liveVarDefs, curVarDefs, nameMap, seenMethods, new HashSet<>(), new HashSet<>());
2028+
}
2029+
}
19612030
}
19622031

1963-
private static @NotNull String rename(Map<VarInMethod, String> nameMap, String name) {
1964-
while (nameMap.containsValue(name)) {
2032+
private static @NotNull String rename(Map<VarInMethod, String> nameMap, String name, Set<VarInMethod> liveVarDefs) {
2033+
while (liveVarDefs.stream().map(nameMap::get).anyMatch(name::equals)) {
19652034
name += "x";
19662035
}
19672036
return name;

‎test/org/jetbrains/java/decompiler/SingleClassesTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,7 @@ private void registerDefault() {
827827
register(JAVA_8, "TestFinallyVarDef");
828828
// TODO: fails to decompile
829829
register(JAVA_25, "TestPatternMatchSwitchString");
830+
register(JAVA_16, "TestPatternMatchingVariableScope");
830831
}
831832

832833
private void registerEntireClassPath() {

‎testData/results/pkg/TestPatternMatching.dec‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public class TestPatternMatching {
3939
obj = obj.toString();// 39
4040
}
4141

42-
<unknown> str;
43-
System.out.println(str.hashCode());// 41
42+
<unknown> strx;
43+
System.out.println(strx.hashCode());// 41
4444
}
4545

4646
public void testInvertedLoopUnused(Object obj) {

0 commit comments

Comments
 (0)