Mark expressions for correct recognition by "unreachable code"

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilder.java

@@ -85,6 +85,8 @@ public interface JetControlFlowBuilder {
     void repeatPseudocode(@NotNull Label startLabel, @NotNull Label finishLabel);
 
     // Reading values
+    void mark(@NotNull JetElement element);
+
     void loadUnit(@NotNull JetExpression expression);
     void loadConstant(@NotNull JetExpression expression, @Nullable CompileTimeConstant<?> constant);
     void createAnonymousObject(@NotNull JetObjectLiteralExpression expression);

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowBuilderAdapter.java

@@ -234,4 +234,9 @@ public abstract class JetControlFlowBuilderAdapter implements JetControlFlowBuil
     public void repeatPseudocode(@NotNull Label startLabel, @NotNull Label finishLabel) {
         getDelegateBuilder().repeatPseudocode(startLabel, finishLabel);
     }
+
+    @Override
+    public void mark(@NotNull JetElement element) {
+        getDelegateBuilder().mark(element);
+    }
 }

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowProcessor.java

@@ -16,6 +16,7 @@
 
 package org.jetbrains.jet.lang.cfg;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.intellij.openapi.util.Ref;
 import com.intellij.psi.PsiElement;
@@ -53,17 +54,17 @@ import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns;
 import org.jetbrains.jet.lexer.JetToken;
 import org.jetbrains.jet.lexer.JetTokens;
 
-import java.util.Iterator;
+import java.util.*;
-import java.util.LinkedList;
-import java.util.List;
 
 import static org.jetbrains.jet.lang.cfg.JetControlFlowBuilder.PredefinedOperation.*;
 import static org.jetbrains.jet.lang.diagnostics.Errors.*;
+import static org.jetbrains.jet.lexer.JetTokens.*;
 
 public class JetControlFlowProcessor {
 
     private final JetControlFlowBuilder builder;
     private final BindingTrace trace;
+    private final Set<PsiElement> marked = new HashSet<PsiElement>();
 
     public JetControlFlowProcessor(BindingTrace trace) {
         this.builder = new JetControlFlowInstructionsGenerator();
@@ -140,6 +141,11 @@ public class JetControlFlowProcessor {
             this.inCondition = inCondition;
         }
 
+        private void mark(JetElement element) {
+            if (!marked.add(element)) return;
+            builder.mark(element);
+        }
+
         public void generateInstructions(@Nullable JetElement element) {
             generateInstructions(element, inCondition);
         }
@@ -173,6 +179,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitParenthesizedExpression(@NotNull JetParenthesizedExpression expression) {
+            mark(expression);
             JetExpression innerExpression = expression.getExpression();
             if (innerExpression != null) {
                 generateInstructions(innerExpression, inCondition);
@@ -243,8 +250,11 @@ public class JetControlFlowProcessor {
         public void visitBinaryExpression(@NotNull JetBinaryExpression expression) {
             JetSimpleNameExpression operationReference = expression.getOperationReference();
             IElementType operationType = operationReference.getReferencedNameElementType();
+            if (!ImmutableSet.of(ANDAND, OROR, EQ, ELVIS).contains(operationType)) {
+                mark(expression);
+            }
             JetExpression right = expression.getRight();
-            if (operationType == JetTokens.ANDAND) {
+            if (operationType == ANDAND) {
                 generateInstructions(expression.getLeft(), true);
                 Label resultLabel = builder.createUnboundLabel();
                 builder.jumpOnFalse(resultLabel);
@@ -256,7 +266,7 @@ public class JetControlFlowProcessor {
                     builder.predefinedOperation(expression, AND);
                 }
             }
-            else if (operationType == JetTokens.OROR) {
+            else if (operationType == OROR) {
                 generateInstructions(expression.getLeft(), true);
                 Label resultLabel = builder.createUnboundLabel();
                 builder.jumpOnTrue(resultLabel);
@@ -268,7 +278,7 @@ public class JetControlFlowProcessor {
                     builder.predefinedOperation(expression, OR);
                 }
             }
-            else if (operationType == JetTokens.EQ) {
+            else if (operationType == EQ) {
                 visitAssignment(expression.getLeft(), right, expression);
             }
             else if (OperatorConventions.ASSIGNMENT_OPERATIONS.containsKey(operationType)) {
@@ -286,7 +296,7 @@ public class JetControlFlowProcessor {
                     generateBothArguments(expression);
                 }
             }
-            else if (operationType == JetTokens.ELVIS) {
+            else if (operationType == ELVIS) {
                 generateInstructions(expression.getLeft(), false);
                 Label afterElvis = builder.createUnboundLabel();
                 builder.jumpOnTrue(afterElvis);
@@ -379,6 +389,7 @@ public class JetControlFlowProcessor {
         }
 
         private void generateArrayAccess(JetArrayAccessExpression arrayAccessExpression, @Nullable ResolvedCall<?> setResolvedCall) {
+            mark(arrayAccessExpression);
             if (!checkAndGenerateCall(arrayAccessExpression, setResolvedCall)) {
                 for (JetExpression index : arrayAccessExpression.getIndexExpressions()) {
                     generateInstructions(index, false);
@@ -390,6 +401,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitUnaryExpression(@NotNull JetUnaryExpression expression) {
+            mark(expression);
             JetSimpleNameExpression operationSign = expression.getOperationReference();
             IElementType operationType = operationSign.getReferencedNameElementType();
             JetExpression baseExpression = expression.getBaseExpression();
@@ -422,6 +434,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitIfExpression(@NotNull JetIfExpression expression) {
+            mark(expression);
             JetExpression condition = expression.getCondition();
             if (condition != null) {
                 generateInstructions(condition, true);
@@ -476,6 +489,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitTryExpression(@NotNull JetTryExpression expression) {
+            mark(expression);
             JetFinallySection finallyBlock = expression.getFinallyBlock();
             final FinallyBlockGenerator finallyBlockGenerator = new FinallyBlockGenerator(finallyBlock);
             if (finallyBlock != null) {
@@ -559,6 +573,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitWhileExpression(@NotNull JetWhileExpression expression) {
+            mark(expression);
             LoopInfo loopInfo = builder.enterLoop(expression, null, null);
 
             builder.bindLabel(loopInfo.getConditionEntryPoint());
@@ -589,6 +604,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitDoWhileExpression(@NotNull JetDoWhileExpression expression) {
+            mark(expression);
             LoopInfo loopInfo = builder.enterLoop(expression, null, null);
 
             builder.bindLabel(loopInfo.getBodyEntryPoint());
@@ -608,6 +624,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitForExpression(@NotNull JetForExpression expression) {
+            mark(expression);
             JetExpression loopRange = expression.getLoopRange();
             if (loopRange != null) {
                 generateInstructions(loopRange, false);
@@ -728,6 +745,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitBlockExpression(@NotNull JetBlockExpression expression) {
+            mark(expression);
             List<JetElement> statements = expression.getStatements();
             for (JetElement statement : statements) {
                 generateInstructions(statement, false);
@@ -744,6 +762,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitFunctionLiteralExpression(@NotNull JetFunctionLiteralExpression expression) {
+            mark(expression);
             JetFunctionLiteral functionLiteral = expression.getFunctionLiteral();
             processLocalDeclaration(functionLiteral);
             builder.createFunctionLiteral(expression);
@@ -751,6 +770,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitQualifiedExpression(@NotNull JetQualifiedExpression expression) {
+            mark(expression);
             JetExpression selectorExpression = expression.getSelectorExpression();
             if (selectorExpression != null) {
                 final Ref<Boolean> error = new Ref<Boolean>(false);
@@ -777,6 +797,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitCallExpression(@NotNull JetCallExpression expression) {
+            mark(expression);
             if (!generateCall(expression.getCalleeExpression())) {
                 for (ValueArgument argument : expression.getValueArguments()) {
                     JetExpression argumentExpression = argument.getArgumentExpression();
@@ -834,6 +855,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitBinaryWithTypeRHSExpression(@NotNull JetBinaryExpressionWithTypeRHS expression) {
+            mark(expression);
             IElementType operationType = expression.getOperationReference().getReferencedNameElementType();
             if (operationType == JetTokens.COLON || operationType == JetTokens.AS_KEYWORD || operationType == JetTokens.AS_SAFE) {
                 generateInstructions(expression.getLeft(), false);
@@ -845,6 +867,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitThrowExpression(@NotNull JetThrowExpression expression) {
+            mark(expression);
             JetExpression thrownExpression = expression.getThrownExpression();
             if (thrownExpression != null) {
                 generateInstructions(thrownExpression, false);
@@ -854,6 +877,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitArrayAccessExpression(@NotNull JetArrayAccessExpression expression) {
+            mark(expression);
             if (!generateCall(expression)) {
                 generateArrayAccess(expression, getResolvedCall(expression));
             }
@@ -861,11 +885,13 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitIsExpression(@NotNull JetIsExpression expression) {
+            mark(expression);
             generateInstructions(expression.getLeftHandSide(), inCondition);
         }
 
         @Override
         public void visitWhenExpression(@NotNull JetWhenExpression expression) {
+            mark(expression);
             JetExpression subjectExpression = expression.getSubjectExpression();
             if (subjectExpression != null) {
                 generateInstructions(subjectExpression, inCondition);
@@ -877,6 +903,7 @@ public class JetControlFlowProcessor {
             Label nextLabel = null;
             for (Iterator<JetWhenEntry> iterator = expression.getEntries().iterator(); iterator.hasNext(); ) {
                 JetWhenEntry whenEntry = iterator.next();
+                mark(whenEntry);
 
                 boolean isElse = whenEntry.isElse();
                 if (isElse) {
@@ -917,6 +944,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitObjectLiteralExpression(@NotNull JetObjectLiteralExpression expression) {
+            mark(expression);
             JetObjectDeclaration declaration = expression.getObjectDeclaration();
             generateInstructions(declaration, inCondition);
 
@@ -940,6 +968,7 @@ public class JetControlFlowProcessor {
 
         @Override
         public void visitStringTemplateExpression(@NotNull JetStringTemplateExpression expression) {
+            mark(expression);
             for (JetStringTemplateEntry entry : expression.getEntries()) {
                 if (entry instanceof JetStringTemplateEntryWithExpression) {
                     JetStringTemplateEntryWithExpression entryWithExpression = (JetStringTemplateEntryWithExpression) entry;

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java

@@ -109,6 +109,11 @@ public class JetFlowInformationProvider {
                     redirectToPrevInstructions(instruction);
                 }
 
+                @Override
+                public void visitMarkInstruction(MarkInstruction instruction) {
+                    redirectToPrevInstructions(instruction);
+                }
+
                 @Override
                 public void visitInstruction(Instruction instruction) {
                     if (instruction instanceof JetElementInstruction) {

compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/InstructionVisitor.java

@@ -95,4 +95,9 @@ public class InstructionVisitor {
     public void visitCompilationErrorInstruction(CompilationErrorInstruction instruction) {
         visitInstructionWithNext(instruction);
     }
+
+    public void visitMarkInstruction(MarkInstruction instruction) {
+        visitInstructionWithNext(instruction);
+    }
+
 }

compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/JetControlFlowInstructionsGenerator.java

@@ -229,6 +229,11 @@ public class JetControlFlowInstructionsGenerator extends JetControlFlowBuilderAd
             return pseudocode;
         }
 
+        @Override
+        public void mark(@NotNull JetElement element) {
+            add(new MarkInstruction(element));
+        }
+
         @Override
         public void returnValue(@NotNull JetExpression returnExpression, @NotNull JetElement subroutine) {
             Label exitPoint = getExitPoint(subroutine);

compiler/frontend/src/org/jetbrains/jet/lang/cfg/pseudocode/instructions.kt

@@ -46,4 +46,20 @@ class CompilationErrorInstruction(
     override fun createCopy() = CompilationErrorInstruction(element, message)
 
     override fun toString() = "error(${render(element)}, $message)"
+}
+
+// This instruciton is used to let the dead code detector know the syntactic structure of unreachable code
+// otherwise only individual parts of expression would be reported as unreachable
+// e.g. for (i in foo) {} -- only i and foo would be marked unreachable
+class MarkInstruction(
+        element: JetElement
+) : InstructionWithNext(element) {
+
+    override fun accept(visitor: InstructionVisitor) {
+        visitor.visitMarkInstruction(this)
+    }
+
+    override fun createCopy() = MarkInstruction(element)
+
+    override fun toString() = "mark(${render(element)})"
 }

compiler/testData/cfg/returnsInWhen.instructions

@@ -2,32 +2,28 @@
 fun illegalWhenBlock(a: Any): Any {
     when(a) {
         is Int -> return a
-        is String -> return a
     }
 }
 ---------------------
 L0:
-    <START>  
+    <START>
     v(a: Any)
-    w(a)     
+    w(a)
-    r(a)     
+    mark({ when(a) { is Int -> return a } })
-    jmp?(L4)     NEXT:[jmp?(L6), r(a)]
+    mark(when(a) { is Int -> return a })
+    r(a)
+    mark(is Int -> return a)
+    jmp?(L4)                                    NEXT:[<END>, r(a)]
 L3:
-    r(a)     
+    r(a)
-    ret(*) L1    NEXT:[<END>]         
+    ret(*) L1                                   NEXT:[<END>]
--   jmp(L2)      NEXT:[<END>]             PREV:[]
+-   jmp(L2)                                     PREV:[]
-L4:
-    jmp?(L6)     NEXT:[<END>, r(a)]       PREV:[jmp?(L4)]
-L5:
-    r(a)     
-    ret(*) L1    NEXT:[<END>]         
--   jmp(L2)      PREV:[]
 L1:
 L2:
-L6:
+L4:
-    <END>        NEXT:[<SINK>]            PREV:[ret(*) L1, jmp?(L6), ret(*) L1]
+    <END>                                       NEXT:[<SINK>]                                      PREV:[jmp?(L4), ret(*) L1]
 error:
-    <ERROR>      PREV:[]
+    <ERROR>                                     PREV:[]
 sink:
-    <SINK>       PREV:[<ERROR>, <END>]
+    <SINK>                                      PREV:[<ERROR>, <END>]
 =====================

compiler/testData/cfg/returnsInWhen.kt

@@ -1,6 +1,5 @@
 fun illegalWhenBlock(a: Any): Any {
     when(a) {
         is Int -> return a
-        is String -> return a
     }
 }

compiler/testData/diagnostics/tests/UnreachableCode.kt

@@ -137,9 +137,9 @@ fun tf() : Int {
 }
 
 fun failtest(<!UNUSED_PARAMETER!>a<!> : Int) : Int {
-  if (fail() || <!UNREACHABLE_CODE!>true<!>) {
+  if (fail() || <!UNREACHABLE_CODE!>true<!>) <!UNREACHABLE_CODE!>{
 
-  }
+  }<!>
   <!UNREACHABLE_CODE!>return 1<!>
 }
 

compiler/testData/diagnostics/tests/controlFlowAnalysis/unreachableCode.kt

@@ -0,0 +1,55 @@
+fun foo(<!UNUSED_PARAMETER!>a<!>: Any) {}
+fun bar(<!UNUSED_PARAMETER!>a<!>: Any, <!UNUSED_PARAMETER!>b<!>: Any) {}
+
+fun test(arr: Array<Int>) {
+    while (true) {
+        <!UNREACHABLE_CODE!>foo<!>(break)
+    }
+
+
+    while (true) {
+        <!UNREACHABLE_CODE!>bar<!>(arr, break)
+    }
+
+    while (true) {
+        <!UNREACHABLE_CODE!>arr[break]<!>
+    }
+
+    while (true) {
+        <!UNREACHABLE_CODE!>arr[1]<!> = break
+    }
+
+    while (true) {
+        break
+        <!UNREACHABLE_CODE!>foo(1)<!>
+    }
+
+    while (true) {
+        var <!UNUSED_VARIABLE!>x<!> = 1
+        break
+        <!UNREACHABLE_CODE!>x = 2<!>
+    }
+
+    while (true) {
+        var <!UNUSED_VARIABLE!>x<!> = 1
+        <!UNREACHABLE_CODE!>x = break<!>
+    }
+
+    // TODO: bug, should be fixed in CFA
+    while (true) {
+        if (1 > 2 && break && 2 > 3) {
+
+        }
+    }
+
+    // TODO: bug, should be fixed in CFA
+    while (true) {
+        if (1 > 2 || break || 2 > 3) {
+
+        }
+    }
+
+    while (true) {
+        <!USELESS_ELVIS!>break<!> ?: <!UNREACHABLE_CODE!>null<!>
+    }
+}

compiler/testData/diagnostics/tests/inference/regressions/kt2445.kt

@@ -7,5 +7,4 @@ fun main(args: Array<String>) {
     }
 }
 
-// callback is unused due to KT-4233
+fun test<R>(callback: (R) -> Unit):Unit = <!UNREACHABLE_CODE!>callback<!>(null!!)
-fun test<R>(<!UNUSED_PARAMETER!>callback<!>: (R) -> Unit):Unit = <!UNREACHABLE_CODE!>callback(null!!)<!>

compiler/testData/diagnostics/tests/inference/regressions/kt2838.kt

@@ -9,9 +9,9 @@ fun test(a: Int) {
     bar(a, <!NULL_FOR_NONNULL_TYPE!>null<!>)
 }
 fun test1(a: Int) {
-    <!UNREACHABLE_CODE!>foo(a, throw Exception())<!>
+    <!UNREACHABLE_CODE!>foo<!>(a, throw Exception())
 }
 
 fun test2(a: Int) {
-    <!UNREACHABLE_CODE!>bar(a, throw Exception())<!>
+    <!UNREACHABLE_CODE!>bar<!>(a, throw Exception())
 }

compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java

@@ -1562,6 +1562,11 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage
                 doTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/uninitializedInLocalDeclarations.kt");
             }
             
+            @TestMetadata("unreachableCode.kt")
+            public void testUnreachableCode() throws Exception {
+                doTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/unreachableCode.kt");
+            }
+            
         }
         
         @TestMetadata("compiler/testData/diagnostics/tests/controlStructures")

idea/testData/checker/UnreachableCode.kt

@@ -136,9 +136,9 @@ fun tf() : Int {
 }
 
 fun failtest(<warning>a</warning> : Int) : Int {
-  if (fail() || <warning>true</warning>) {
+  if (fail() || <warning>true</warning>) <warning>{
 
-  }
+  }</warning>
   <warning>return 1</warning>
 }