ControlFlowUtil#internalDepthFirstSearch: rewrite subroutine tracking

Now every graph edge is accompanied with subroutine stack.
Also shared mutable state is removed from instructions (now it's local to DFS procedure).
Fixes IDEA-201093 Unreachable bug in three try catch while two have finally

GitOrigin-RevId: b600ba63e2da315c9830836adcdc4d8fecf92ca3

java/java-psi-impl/src/com/intellij/psi/controlFlow/CallInstruction.java

@@ -19,13 +19,11 @@ import org.jetbrains.annotations.NotNull;
 
 
 public class CallInstruction extends GoToInstruction {
-  public final ControlFlowStack stack;
   int procBegin;
   int procEnd;
 
-  public CallInstruction(int procBegin, int procEnd, @NotNull ControlFlowStack stack) {
+  public CallInstruction(int procBegin, int procEnd) {
     super(procBegin);
-    this.stack = stack;
     this.procBegin = procBegin;
     this.procEnd = procEnd;
   }
@@ -34,12 +32,6 @@ public class CallInstruction extends GoToInstruction {
     return "CALL " + offset ;
   }
 
-  public void execute(int returnOffset) {
-    synchronized (stack) {
-      stack.push(returnOffset, this);
-    }
-  }
-
   @Override
   public void accept(@NotNull ControlFlowInstructionVisitor visitor, int offset, int nextOffset) {
     visitor.visitCallInstruction(this, offset, nextOffset);

java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowAnalyzer.java

@@ -27,7 +27,6 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
   private final ControlFlowPolicy myPolicy;
 
   private ControlFlowImpl myCurrentFlow;
-  private final ControlFlowStack myStack = new ControlFlowStack();
   private final Stack<PsiParameter> myCatchParameters = new Stack<>();// stack of PsiParameter for catch
   private final Stack<PsiElement> myCatchBlocks = new Stack<>();
 
@@ -410,7 +409,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
       if (finallyBlock != null && finallyStartOffset != -1) {
         // go out of finally, use return
         CallInstruction callInstruction = (CallInstruction)myCurrentFlow.getInstructions().get(finallyStartOffset - 2);
-        instruction = new ReturnInstruction(0, myStack, callInstruction);
+        instruction = new ReturnInstruction(0, callInstruction);
       }
       else {
         instruction = new GoToInstruction(0, BranchingInstruction.Role.END, PsiTreeUtil.isAncestor(exitedStatement, myCodeFragment, true));
@@ -430,7 +429,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
       if (enclosingTryStatement == null || !PsiTreeUtil.isAncestor(exitedStatement, enclosingTryStatement, false)) {
         break;
       }
-      CallInstruction instruction = new CallInstruction(0, 0, myStack);
+      CallInstruction instruction = new CallInstruction(0, 0);
       finallyBlockSubroutine.addCall(instruction);
       myCurrentFlow.addInstruction(instruction);
       addElementOffsetLater(finallyBlock, true);
@@ -472,7 +471,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
       if (finallyBlock != null && finallyStartOffset != -1) {
         // go out of finally, use return
         CallInstruction callInstruction = (CallInstruction)myCurrentFlow.getInstructions().get(finallyStartOffset - 2);
-        instruction = new ReturnInstruction(0, myStack, callInstruction);
+        instruction = new ReturnInstruction(0, callInstruction);
       }
       else {
         instruction = new GoToInstruction(0, BranchingInstruction.Role.END, PsiTreeUtil.isAncestor(body, myCodeFragment, true));
@@ -1169,20 +1168,20 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
 
     if (finallyBlock != null) {
       // normal completion, call finally block and proceed
-      CallInstruction normalCompletion = new CallInstruction(0, 0, myStack);
+      CallInstruction normalCompletion = new CallInstruction(0, 0);
       finallyBlockSubroutine.addCall(normalCompletion);
       myCurrentFlow.addInstruction(normalCompletion);
       addElementOffsetLater(finallyBlock, true);
       myCurrentFlow.addInstruction(new GoToInstruction(0));
       addElementOffsetLater(statement, false);
       // return completion, call finally block and return
-      CallInstruction returnCompletion = new CallInstruction(0, 0, myStack);
+      CallInstruction returnCompletion = new CallInstruction(0, 0);
       finallyBlockSubroutine.addCall(returnCompletion);
       myCurrentFlow.addInstruction(returnCompletion);
       addElementOffsetLater(finallyBlock, true);
       addReturnInstruction(statement);
       // throw exception completion, call finally block and rethrow
-      CallInstruction throwExceptionCompletion = new CallInstruction(0, 0, myStack);
+      CallInstruction throwExceptionCompletion = new CallInstruction(0, 0);
       finallyBlockSubroutine.addCall(throwExceptionCompletion);
       myCurrentFlow.addInstruction(throwExceptionCompletion);
       addElementOffsetLater(finallyBlock, true);
@@ -1202,13 +1201,13 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
       // first three return instructions are for normal completion, return statement call completion and unchecked exception throwing completion resp.
 
       // normal completion
-      myCurrentFlow.addInstruction(new ReturnInstruction(0, myStack, normalCompletion));
+      myCurrentFlow.addInstruction(new ReturnInstruction(0, normalCompletion));
 
       // return statement call completion
-      myCurrentFlow.addInstruction(new ReturnInstruction(procStart - 3, myStack, returnCompletion));
+      myCurrentFlow.addInstruction(new ReturnInstruction(procStart - 3, returnCompletion));
 
       // unchecked exception throwing completion
-      myCurrentFlow.addInstruction(new ReturnInstruction(procStart - 1, myStack, throwExceptionCompletion));
+      myCurrentFlow.addInstruction(new ReturnInstruction(procStart - 1, throwExceptionCompletion));
 
       // checked exception throwing completion. need to dispatch to the correct catch clause
       final List<PsiElement> unhandledExceptionCatchBlocks = finallyBlockToUnhandledExceptions.remove(finallyBlock);
@@ -1216,7 +1215,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor {
         ProgressManager.checkCanceled();
         PsiElement catchBlock = unhandledExceptionCatchBlocks.get(i);
 
-        final ReturnInstruction returnInstruction = new ReturnInstruction(0, myStack, throwExceptionCompletion);
+        final ReturnInstruction returnInstruction = new ReturnInstruction(0, throwExceptionCompletion);
         returnInstruction.setRethrowFromFinally();
         myCurrentFlow.addInstruction(returnInstruction);
         if (catchBlock == null) {

java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowStack.java

@@ -1,47 +0,0 @@
-/*
- * Copyright 2000-2009 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 com.intellij.psi.controlFlow;
-
-import com.intellij.util.containers.IntArrayList;
-
-import java.util.ArrayList;
-
-public class ControlFlowStack {
-  private final IntArrayList myIpStack = new IntArrayList();
-  private final ArrayList<CallInstruction> myCallInstructionStack = new ArrayList<>();
-
-  public void push(int ip, CallInstruction callInstruction) {
-    myIpStack.add(ip);
-    myCallInstructionStack.add(callInstruction);
-  }
-
-  public int pop(boolean pushBack) {
-    final int i = myIpStack.get(myIpStack.size() - 1);
-    if (!pushBack) {
-      myIpStack.remove(myIpStack.size()-1);
-      myCallInstructionStack.remove(myCallInstructionStack.size()-1);
-    }
-    return i;
-  }
-  public int peekReturnOffset() {
-    return myIpStack.get(myIpStack.size() - 1);
-  }
-  public int size() {
-    return myIpStack.size();
-  }
-
-}

java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowSubRange.java

@@ -54,7 +54,7 @@ public class ControlFlowSubRange implements ControlFlow {
         }
         if (instruction instanceof ReturnInstruction) {
           final ReturnInstruction returnInstruction = (ReturnInstruction)instruction;
-          CallInstruction callInstruction = new CallInstruction(patchOffset(returnInstruction.getProcBegin()), patchOffset(returnInstruction.getProcEnd()), returnInstruction.getStack());
+          CallInstruction callInstruction = new CallInstruction(patchOffset(returnInstruction.getProcBegin()), patchOffset(returnInstruction.getProcEnd()));
           returnInstruction.setCallInstruction(callInstruction);
         }
         list.add(instruction);

java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java

@@ -1571,109 +1571,107 @@ public class ControlFlowUtil {
     final WalkThroughStack walkThroughStack = new WalkThroughStack(instructions.size() / 2);
     walkThroughStack.push(startOffset);
 
-    // we can change instruction internal state here (e.g. CallInstruction.stack)
-    synchronized (instructions) {
-      final IntArrayList currentProcedureReturnOffsets = new IntArrayList();
-      ControlFlowInstructionVisitor getNextOffsetVisitor = new ControlFlowInstructionVisitor() {
-        @Override
-        public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) {
-          instruction.execute(offset + 1);
-          int newOffset = instruction.offset;
-          // 'procedure' pointed by call instruction should be processed regardless of whether it was already visited or not
-          // clear procedure text and return instructions afterwards
-          int i;
-          for (i = instruction.procBegin;
-               i < clientVisitor.processedInstructions.length &&
-               (i < instruction.procEnd || i < instructions.size() && instructions.get(i) instanceof ReturnInstruction); i++) {
-            clientVisitor.processedInstructions[i] = false;
-          }
-          clientVisitor.procedureEntered(instruction.procBegin, i);
-          walkThroughStack.push(offset, newOffset);
-          walkThroughStack.push(newOffset);
-
-          currentProcedureReturnOffsets.add(offset + 1);
+    ControlFlowInstructionVisitor getNextOffsetVisitor = new ControlFlowInstructionVisitor() {
+      @Override
+      public void visitCallInstruction(CallInstruction instruction, int offset, int nextOffset) {
+        int newOffset = instruction.offset;
+        // 'procedure' pointed by call instruction should be processed regardless of whether it was already visited or not
+        // clear procedure text and return instructions afterwards
+        int i;
+        for (i = instruction.procBegin;
+             i < clientVisitor.processedInstructions.length &&
+             (i < instruction.procEnd || i < instructions.size() && instructions.get(i) instanceof ReturnInstruction); i++) {
+          clientVisitor.processedInstructions[i] = false;
         }
-
-        @Override
-        public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) {
-          int newOffset = instruction.execute(false);
-          if (newOffset != -1) {
-            walkThroughStack.push(offset, newOffset);
-            walkThroughStack.push(newOffset);
-          }
-        }
-
-        @Override
-        public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) {
-          int newOffset = instruction.offset;
-          walkThroughStack.push(offset, newOffset);
-          walkThroughStack.push(newOffset);
-        }
-
-        @Override
-        public void visitConditionalBranchingInstruction(ConditionalBranchingInstruction instruction, int offset, int nextOffset) {
-          int newOffset = instruction.offset;
-
-          walkThroughStack.push(offset, newOffset);
-          walkThroughStack.push(offset, offset + 1);
-          walkThroughStack.push(newOffset);
-          walkThroughStack.push(offset + 1);
-        }
-
-        @Override
-        public void visitInstruction(Instruction instruction, int offset, int nextOffset) {
-          int newOffset = offset + 1;
-          walkThroughStack.push(offset, newOffset);
-          walkThroughStack.push(newOffset);
-        }
-      };
-      while (!walkThroughStack.isEmpty()) {
-        final int offset = walkThroughStack.peekOldOffset();
-        final int newOffset = walkThroughStack.popNewOffset();
-
-        if (offset >= endOffset) {
-          continue;
-        }
-        Instruction instruction = instructions.get(offset);
-
-        if (clientVisitor.processedInstructions[offset]) {
-          if (newOffset != -1) {
-            instruction.accept(clientVisitor, offset, newOffset);
-          }
-          // when traversing call instruction, we have traversed all procedure control flows, so pop return address
-          if (!currentProcedureReturnOffsets.isEmpty() && currentProcedureReturnOffsets.get(currentProcedureReturnOffsets.size() - 1) - 1 == offset) {
-            currentProcedureReturnOffsets.remove(currentProcedureReturnOffsets.size() - 1);
-          }
-          continue;
-        }
-        if (!currentProcedureReturnOffsets.isEmpty()) {
-          int returnOffset = currentProcedureReturnOffsets.get(currentProcedureReturnOffsets.size() - 1);
-          CallInstruction callInstruction = (CallInstruction)instructions.get(returnOffset - 1);
-          // check if we inside procedure but 'return offset' stack is empty, so
-          // we should push back to 'return offset' stack
-          synchronized (callInstruction.stack) {
-            if (callInstruction.procBegin <= offset && offset < callInstruction.procEnd + 2
-                && (callInstruction.stack.size() == 0 || callInstruction.stack.peekReturnOffset() != returnOffset)) {
-              callInstruction.stack.push(returnOffset, callInstruction);
-            }
-          }
-        }
-
-        clientVisitor.processedInstructions[offset] = true;
-        instruction.accept(getNextOffsetVisitor, offset, newOffset);
+        clientVisitor.procedureEntered(instruction.procBegin, i);
+        walkThroughStack.currentStack = new CallStackItem(walkThroughStack.currentStack, offset + 1);
+        walkThroughStack.push(offset, newOffset);
+        walkThroughStack.push(newOffset);
       }
+
+      @Override
+      public void visitReturnInstruction(ReturnInstruction instruction, int offset, int nextOffset) {
+        int newOffset = -1;
+        if (walkThroughStack.currentStack != null) {
+          newOffset = walkThroughStack.currentStack.target;
+          walkThroughStack.currentStack = walkThroughStack.currentStack.next;
+        }
+        if (instruction.offset != 0) {
+          newOffset = instruction.offset;
+        }
+        if (newOffset != -1) {
+          walkThroughStack.push(offset, newOffset);
+          walkThroughStack.push(newOffset);
+        }
+      }
+
+      @Override
+      public void visitBranchingInstruction(BranchingInstruction instruction, int offset, int nextOffset) {
+        int newOffset = instruction.offset;
+        walkThroughStack.push(offset, newOffset);
+        walkThroughStack.push(newOffset);
+      }
+
+      @Override
+      public void visitConditionalBranchingInstruction(ConditionalBranchingInstruction instruction, int offset, int nextOffset) {
+        int newOffset = instruction.offset;
+
+        walkThroughStack.push(offset, newOffset);
+        walkThroughStack.push(offset, offset + 1);
+        walkThroughStack.push(newOffset);
+        walkThroughStack.push(offset + 1);
+      }
+
+      @Override
+      public void visitInstruction(Instruction instruction, int offset, int nextOffset) {
+        int newOffset = offset + 1;
+        walkThroughStack.push(offset, newOffset);
+        walkThroughStack.push(newOffset);
+      }
+    };
+    while (!walkThroughStack.isEmpty()) {
+      final int offset = walkThroughStack.peekOldOffset();
+      final int newOffset = walkThroughStack.popNewOffset();
+
+      if (offset >= endOffset) {
+        continue;
+      }
+      Instruction instruction = instructions.get(offset);
+
+      if (clientVisitor.processedInstructions[offset]) {
+        if (newOffset != -1) {
+          instruction.accept(clientVisitor, offset, newOffset);
+        }
+        continue;
+      }
+
+      clientVisitor.processedInstructions[offset] = true;
+      instruction.accept(getNextOffsetVisitor, offset, newOffset);
+    }
+  }
+
+  private static final class CallStackItem {
+    final CallStackItem next;
+    final int target;
+
+    private CallStackItem(CallStackItem next, int target) {
+      this.next = next;
+      this.target = target;
     }
   }
 
   private static class WalkThroughStack {
     private int[] oldOffsets;
     private int[] newOffsets;
+    private CallStackItem[] callStacks;
+    private CallStackItem currentStack;
     private int size;
 
     WalkThroughStack(int initialSize) {
       if (initialSize < 2) initialSize = 2;
       oldOffsets = new int[initialSize];
       newOffsets = new int[initialSize];
+      callStacks = new CallStackItem[initialSize];
     }
 
     /**
@@ -1682,11 +1680,14 @@ public class ControlFlowUtil {
     void push(int oldOffset, int newOffset) {
       LOG.assertTrue(oldOffset >= 0, "negative offset is pushed to walk-through stack");
       if (size >= newOffsets.length) {
-        oldOffsets = ArrayUtil.realloc(oldOffsets, size * 3 / 2);
-        newOffsets = ArrayUtil.realloc(newOffsets, size * 3 / 2);
+        int newSize = size * 3 / 2;
+        oldOffsets = Arrays.copyOf(oldOffsets, newSize);
+        newOffsets = Arrays.copyOf(newOffsets, newSize);
+        callStacks = Arrays.copyOf(callStacks, newSize);
       }
       oldOffsets[size] = oldOffset;
       newOffsets[size] = newOffset;
+      callStacks[size] = currentStack;
       size++;
     }
 
@@ -1708,7 +1709,8 @@ public class ControlFlowUtil {
      * Should be used in pair with {@link #peekOldOffset()}
      */
     int popNewOffset() {
-      return newOffsets[--size];
+      currentStack = callStacks[--size];
+      return newOffsets[size];
     }
 
     boolean isEmpty() {

java/java-psi-impl/src/com/intellij/psi/controlFlow/ReturnInstruction.java

@@ -22,13 +22,11 @@ import org.jetbrains.annotations.NotNull;
 public class ReturnInstruction extends GoToInstruction {
   private static final Logger LOG = Logger.getInstance("#com.intellij.psi.controlFlow.ReturnInstruction");
 
-  @NotNull private final ControlFlowStack myStack;
   @NotNull private CallInstruction myCallInstruction;
   private boolean myRethrowFromFinally;
 
-  public ReturnInstruction(int offset, @NotNull ControlFlowStack stack, @NotNull CallInstruction callInstruction) {
+  public ReturnInstruction(int offset, @NotNull CallInstruction callInstruction) {
     super(offset, Role.END, false);
-    myStack = stack;
     myCallInstruction = callInstruction;
   }
 
@@ -36,19 +34,6 @@ public class ReturnInstruction extends GoToInstruction {
     return "RETURN FROM " + getProcBegin() + (offset == 0 ? "" : " TO "+offset);
   }
 
-  public int execute(boolean pushBack) {
-    synchronized (myStack) {
-      int jumpTo = -1;
-      if (myStack.size() != 0) {
-        jumpTo = myStack.pop(pushBack);
-      }
-      if (offset != 0) {
-        jumpTo = offset;
-      }
-      return jumpTo;
-    }
-  }
-
   @NotNull
   int[] getPossibleReturnOffsets() {
     return offset == 0 ?
@@ -104,11 +89,6 @@ public class ReturnInstruction extends GoToInstruction {
     visitor.visitReturnInstruction(this, offset, nextOffset);
   }
 
-  @NotNull
-  public ControlFlowStack getStack() {
-    return myStack;
-  }
-
   void setRethrowFromFinally() {
     myRethrowFromFinally = true;
   }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/UnreachableMultiFinally.java

@@ -0,0 +1,13 @@
+class Test {
+  public static void main(String[] args) {
+    try {
+      try {} catch (Exception e) {} finally {
+        try {} catch (Exception e) {} finally {
+          for (int i = 0; i < 10; i++) {
+          }
+        }
+      }
+      System.out.println("Reachable");
+    }catch(Exception e) {}
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightAdvHighlightingTest.java

@@ -116,6 +116,7 @@ public class LightAdvHighlightingTest extends LightDaemonAnalyzerTestCase {
   public void testOverriddenMethodIsFinal() { doTest(false); }
   public void testMissingReturn() { doTest(false); }
   public void testUnreachable() { doTest(false); }
+  public void testUnreachableMultiFinally() { doTest(false); }
   public void testFinalFieldInit() { doTest(false); }
   public void testLocalVariableInitialization() { doTest(false); }
   public void testVarDoubleInitialization() { doTest(false); }