drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [drill] vdiravka commented on a change in pull request #1686: DRILL-6524: Prevent incorrect scalar replacement for the case of assigning references inside if block
Date Tue, 12 Mar 2019 16:43:59 GMT
vdiravka commented on a change in pull request #1686: DRILL-6524: Prevent incorrect scalar
replacement for the case of assigning references inside if block
URL: https://github.com/apache/drill/pull/1686#discussion_r264686127
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/compile/bytecode/MethodAnalyzer.java
 ##########
 @@ -36,70 +46,147 @@
  * as factories that will provide our own derivative of Frame<> which we use to detect
  */
 public class MethodAnalyzer<V extends Value> extends Analyzer <V> {
+
+  // list of method instructions which is analyzed
+  private InsnList insnList;
+
+  /**
+   * Constructor.
+   *
+   * @param interpreter the interpreter to use
+   */
+  public MethodAnalyzer(Interpreter<V> interpreter) {
+    super(interpreter);
+  }
+
+  @Override
+  protected Frame<V> newFrame(int maxLocals, int maxStack) {
+    return new AssignmentTrackingFrame<>(maxLocals, maxStack);
+  }
+
+  @Override
+  protected Frame<V> newFrame(Frame<? extends V> src) {
+    return new AssignmentTrackingFrame<>(src);
+  }
+
+  @Override
+  protected void newControlFlowEdge(int insnIndex, int successorIndex) {
+    AssignmentTrackingFrame<V> oldFrame = (AssignmentTrackingFrame<V>) getFrames()[insnIndex];
+    AbstractInsnNode insn = insnList.get(insnIndex);
+    if (insn.getType() == AbstractInsnNode.LABEL) {
+      // checks whether current label corresponds to the end of conditional block to restore
previous
+      // local variables set
+      if (insn.equals(oldFrame.labelsStack.peekFirst())) {
+        oldFrame.localVariablesSet.pop();
+        oldFrame.labelsStack.pop();
+      }
+    }
+  }
+
+  @Override
+  public Frame<V>[] analyze(String owner, MethodNode method) throws AnalyzerException
{
+    insnList = method.instructions;
+    return super.analyze(owner, method);
+  }
+
   /**
    * Custom Frame<> that captures setLocal() calls in order to associate values
-   * that are assigned to the same local variable slot.
+   * that are assigned to the same local variable slot. Also it controls stack to determine
whether
+   * object was assigned to the value declared outside of conditional block.
    *
    * <p>Since this is almost a pass-through, the constructors' arguments match
    * those from Frame<>.
    */
   private static class AssignmentTrackingFrame<V extends Value> extends Frame<V>
{
+
+    // represents stacks of variable sets declared inside current code block
 
 Review comment:
   ```suggestion
       // represents stack of variable sets declared inside current code block
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message