From bd73ef4bd9f272ed7427f094f1208d7f51bfc0c0 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Thu, 14 Apr 2011 13:18:31 +0400 Subject: [PATCH] More tests for dead code detection --- .../jet/lang/cfg/JetControlFlowBuilder.java | 4 +- .../cfg/JetControlFlowBuilderAdapter.java | 8 +- .../jet/lang/cfg/JetControlFlowProcessor.java | 4 +- .../cfg/pseudocode/InstructionWithNext.java | 2 +- .../JetControlFlowInstructionsGenerator.java | 8 +- .../cfg/pseudocode/JetElementInstruction.java | 12 +- .../pseudocode/JetElementInstructionImpl.java | 21 ++++ .../pseudocode/ReturnNoValueInstruction.java | 4 +- .../pseudocode/ReturnValueInstruction.java | 16 ++- .../jet/lang/resolve/TopDownAnalyzer.java | 34 +++++- idea/testData/checker/UnreachableCode.jet | 113 ++++++++++++++++-- 11 files changed, 187 insertions(+), 39 deletions(-) create mode 100644 idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstructionImpl.java diff --git a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilder.java b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilder.java index 8af1ae5b281..64f40a198cd 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilder.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilder.java @@ -47,9 +47,9 @@ public interface JetControlFlowBuilder { @Nullable JetElement getCurrentSubroutine(); - void returnValue(@NotNull JetElement subroutine); - void returnNoValue(@NotNull JetElement expression, @NotNull JetElement subroutine); + void returnValue(@NotNull JetExpression returnExpression, @NotNull JetElement subroutine); + void returnNoValue(@NotNull JetElement returnExpression, @NotNull JetElement subroutine); void writeNode(@NotNull JetElement assignment, @NotNull JetElement lValue); diff --git a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilderAdapter.java b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilderAdapter.java index 0e0c62b68bd..626b584c9ca 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilderAdapter.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilderAdapter.java @@ -110,13 +110,13 @@ public class JetControlFlowBuilderAdapter implements JetControlFlowBuilder { } @Override - public void returnValue(@NotNull JetElement subroutine) { - builder.returnValue(subroutine); + public void returnValue(@NotNull JetExpression returnExpression, @NotNull JetElement subroutine) { + builder.returnValue(returnExpression, subroutine); } @Override - public void returnNoValue(@NotNull JetElement expression, @NotNull JetElement subroutine) { - builder.returnNoValue(expression, subroutine); + public void returnNoValue(@NotNull JetElement returnExpression, @NotNull JetElement subroutine) { + builder.returnNoValue(returnExpression, subroutine); } @Override diff --git a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java index 68d127306d3..ea44db9c24a 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java @@ -346,7 +346,9 @@ public class JetControlFlowProcessor { @Override public void visitForExpression(JetForExpression expression) { value(expression.getLoopRange(), false, false); + // TODO : primitive cases Label loopExitPoint = builder.createUnboundLabel(); + builder.nondeterministicJump(loopExitPoint); Label loopEntryPoint = builder.enterLoop(expression, loopExitPoint); JetExpression body = expression.getBody(); if (body != null) { @@ -421,7 +423,7 @@ public class JetControlFlowProcessor { builder.returnNoValue(expression, subroutine); } else { - builder.returnValue(subroutine); + builder.returnValue(expression, subroutine); } } } diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/InstructionWithNext.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/InstructionWithNext.java index a50bbf2cb08..78366a0af86 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/InstructionWithNext.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/InstructionWithNext.java @@ -9,7 +9,7 @@ import java.util.Collections; /** * @author abreslav */ -public abstract class InstructionWithNext extends JetElementInstruction { +public abstract class InstructionWithNext extends JetElementInstructionImpl { private Instruction next; protected InstructionWithNext(@NotNull JetElement element) { diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java index a7c24b612da..61511da0f48 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java @@ -150,13 +150,13 @@ public class JetControlFlowInstructionsGenerator extends JetControlFlowBuilderAd } @Override - public void returnValue(@NotNull JetElement subroutine) { - add(new ReturnValueInstruction(getExitPoint(subroutine))); + public void returnValue(@NotNull JetExpression returnExpression, @NotNull JetElement subroutine) { + add(new ReturnValueInstruction(returnExpression, getExitPoint(subroutine))); } @Override - public void returnNoValue(@NotNull JetElement expression, @NotNull JetElement subroutine) { - add(new ReturnNoValueInstruction(expression, getExitPoint(subroutine))); + public void returnNoValue(@NotNull JetElement returnExpression, @NotNull JetElement subroutine) { + add(new ReturnNoValueInstruction(returnExpression, getExitPoint(subroutine))); } @Override diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstruction.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstruction.java index c99d41a8d7c..cc1b8ad4424 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstruction.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstruction.java @@ -6,15 +6,7 @@ import org.jetbrains.jet.lang.psi.JetElement; /** * @author abreslav */ -public abstract class JetElementInstruction extends Instruction { - protected final JetElement element; - - public JetElementInstruction(@NotNull JetElement element) { - this.element = element; - } - +public interface JetElementInstruction { @NotNull - public JetElement getElement() { - return element; - } + JetElement getElement(); } diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstructionImpl.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstructionImpl.java new file mode 100644 index 00000000000..53d180ca111 --- /dev/null +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/JetElementInstructionImpl.java @@ -0,0 +1,21 @@ +package org.jetbrains.jet.lang.cfg.pseudocode; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.jet.lang.psi.JetElement; + +/** + * @author abreslav + */ +public abstract class JetElementInstructionImpl extends Instruction implements JetElementInstruction { + protected final JetElement element; + + public JetElementInstructionImpl(@NotNull JetElement element) { + this.element = element; + } + + @NotNull + @Override + public JetElement getElement() { + return element; + } +} diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnNoValueInstruction.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnNoValueInstruction.java index 169a51c048a..36731738734 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnNoValueInstruction.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnNoValueInstruction.java @@ -7,7 +7,7 @@ import org.jetbrains.jet.lang.psi.JetElement; /** * @author abreslav */ -public class ReturnNoValueInstruction extends AbstractJumpInstruction { +public class ReturnNoValueInstruction extends AbstractJumpInstruction implements JetElementInstruction { private final JetElement element; @@ -16,6 +16,8 @@ public class ReturnNoValueInstruction extends AbstractJumpInstruction { this.element = element; } + @NotNull + @Override public JetElement getElement() { return element; } diff --git a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnValueInstruction.java b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnValueInstruction.java index 88166b8da6d..9520897a673 100644 --- a/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnValueInstruction.java +++ b/idea/src/org/jetbrains/jet/lang/cfg/pseudocode/ReturnValueInstruction.java @@ -1,14 +1,20 @@ package org.jetbrains.jet.lang.cfg.pseudocode; +import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.cfg.Label; +import org.jetbrains.jet.lang.psi.JetElement; +import org.jetbrains.jet.lang.psi.JetExpression; /** * @author abreslav */ -public class ReturnValueInstruction extends AbstractJumpInstruction { +public class ReturnValueInstruction extends AbstractJumpInstruction implements JetElementInstruction { - public ReturnValueInstruction(Label targetLabel) { + private final JetElement element; + + public ReturnValueInstruction(@NotNull JetExpression returnExpression, @NotNull Label targetLabel) { super(targetLabel); + this.element = returnExpression; } @Override @@ -20,4 +26,10 @@ public class ReturnValueInstruction extends AbstractJumpInstruction { public String toString() { return "ret(*) " + getTargetLabel(); } + + @NotNull + @Override + public JetElement getElement() { + return element; + } } diff --git a/idea/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java b/idea/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java index 93073375829..eb04d9d63aa 100644 --- a/idea/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java +++ b/idea/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java @@ -286,8 +286,12 @@ public class TopDownAnalyzer { List unreachableElements = new ArrayList(); flowInformationProvider.collectUnreachableExpressions(function, unreachableElements); - for (JetElement unreachableElement : unreachableElements) { - semanticServices.getErrorHandler().genericError(unreachableElement.getNode(), "Unreachable code"); + + // This is needed in order to highlight only '1 < 2' and not '1', '<' and '2' as well + Set rootElements = findRootExpressions(unreachableElements); + + for (JetElement element : rootElements) { + semanticServices.getErrorHandler().genericError(element.getNode(), "Unreachable code"); } } else { @@ -310,6 +314,28 @@ public class TopDownAnalyzer { } } + private Set findRootExpressions(List unreachableElements) { + Set rootElements = new HashSet(); + final Set shadowedElements = new HashSet(); + JetVisitor shadowAllChildren = new JetVisitor() { + @Override + public void visitJetElement(JetElement elem) { + if (shadowedElements.add(elem)) { + elem.acceptChildren(this); + } + } + }; + + for (JetElement element : unreachableElements) { + if (shadowedElements.contains(element)) continue; + element.acceptChildren(shadowAllChildren); + + rootElements.removeAll(shadowedElements); + rootElements.add(element); + } + return rootElements; + } + private JetFlowInformationProvider computeFlowData(@NotNull JetDeclaration declaration, @NotNull JetExpression bodyExpression) { final JetPseudocodeTrace pseudocodeTrace = flowDataTraceFactory.createTrace(declaration); final Map pseudocodeMap = new HashMap(); @@ -415,8 +441,8 @@ public class TopDownAnalyzer { @Override public void visitInstruction(Instruction instruction) { - if (instruction instanceof JetElementInstruction) { - JetElementInstruction elementInstruction = (JetElementInstruction) instruction; + if (instruction instanceof JetElementInstructionImpl) { + JetElementInstructionImpl elementInstruction = (JetElementInstructionImpl) instruction; semanticServices.getErrorHandler().genericError(elementInstruction.getElement().getNode(), "Unsupported by control-flow builder " + elementInstruction.getElement()); } else { diff --git a/idea/testData/checker/UnreachableCode.jet b/idea/testData/checker/UnreachableCode.jet index 4b6afe5dab1..557d8f1c67d 100644 --- a/idea/testData/checker/UnreachableCode.jet +++ b/idea/testData/checker/UnreachableCode.jet @@ -3,6 +3,24 @@ fun t1() { 1 } +fun t1a() { + return + return + 1 +} + +fun t1b() { + return + return 1 + 1 +} + +fun t1c() { + return 1 + return + 1 +} + fun t2() { if (1 > 2) return @@ -10,6 +28,16 @@ fun t2() { 1 } +fun t2a() { + if (1 > 2) { + return + 1 + } else { return + 2 + } + 1 +} + fun t3() { if (1 > 2) return 2 @@ -25,14 +53,79 @@ fun t4(a : Boolean) { 1 } -//fun t5() { -// do { -// return -// } -// while (1 > 2) -// 1 -//} +fun t4break(a : Boolean) { + do { + break + } + while (a) + 1 +} -//fun blockAndAndMismatch() : Boolean { -// (return true) || (return false) -//} +fun t5() { + do { + return + 2 + } + while (1 > 2) + 1 +} + +fun t6() { + while (1 > 2) { + return + 2 + } + 1 +} + +fun t6break() { + while (1 > 2) { + break + 2 + } + 1 +} + +fun t7(b : Int) { + for (i in 1..b) { + return + 2 + } + 1 +} + +fun t7break(b : Int) { + for (i in 1..b) { + return + 2 + } + 1 +} + +fun t7() { + try { + return + 2 + } + catch (e : Any) { + 2 + } + 1 // this is OK, like in Java +} + +fun t8() { + try { + return + 2 + } + catch (e : Any) { + return + 2 + } + 1 +} + +fun blockAndAndMismatch() : Boolean { + (return true) || (return false) + 1 +}