fixed JASSIST-230

diff --git a/Readme.html b/Readme.html
index 7b1b5046ecd7e7e2ede05993bc8ead95ac10f4bc..117374792d53bb36b3da868b874b6ccfe4c41eda 100644 (file)
--- a/Readme.html
+++ b/Readme.html
@@ -283,7 +283,7 @@ see javassist.Dump.
 
 <p>-version 3.19
 <ul>
-<li>JIRA JASSIST-158, 205, 206, 207, 211, 212, 216, 220, 223, 224, 227.
+<li>JIRA JASSIST-158, 205, 206, 207, 211, 212, 216, 220, 223, 224, 227, 230.
 </ul>
 </p>
 

diff --git a/src/main/javassist/compiler/CodeGen.java b/src/main/javassist/compiler/CodeGen.java
index bbcce89592293fe0d45df529776c851cca8ff930..d2bdea9a35a0e9f6b1177270c0da368fa7288b0d 100644 (file)
--- a/src/main/javassist/compiler/CodeGen.java
+++ b/src/main/javassist/compiler/CodeGen.java
@@ -381,7 +381,14 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         ASTree expr = st.head();
         Stmnt thenp = (Stmnt)st.tail().head();
         Stmnt elsep = (Stmnt)st.tail().tail().head();
-        compileBooleanExpr(false, expr);
+        if (compileBooleanExpr(false, expr)) {
+            hasReturned = false;
+            if (elsep != null)
+                elsep.accept(this);
+
+            return;
+        }
+
         int pc = bytecode.currentPc();
         int pc2 = 0;
         bytecode.addIndex(0);   // correct later
@@ -400,7 +407,6 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         }
 
         bytecode.write16bit(pc, bytecode.currentPc() - pc + 1);
-
         if (elsep != null) {
             elsep.accept(this);
             if (!thenHasReturned)
@@ -434,9 +440,13 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         if (notDo)
             bytecode.write16bit(pc, pc3 - pc + 1);
 
-        boolean alwaysBranch = compileBooleanExpr(true, expr) && breakList.size() == 0;
-        bytecode.addIndex(pc2 - bytecode.currentPc() + 1);
+        boolean alwaysBranch = compileBooleanExpr(true, expr);
+        if (alwaysBranch) {
+            bytecode.addOpcode(Opcode.GOTO);
+            alwaysBranch = breakList.size() == 0;
+        }
 
+        bytecode.addIndex(pc2 - bytecode.currentPc() + 1);
         patchGoto(breakList, bytecode.currentPc());
         patchGoto(continueList, pc3);
         continueList = prevContList;
@@ -471,7 +481,14 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         int pc = bytecode.currentPc();
         int pc2 = 0;
         if (expr != null) {
-            compileBooleanExpr(false, expr);
+            if (compileBooleanExpr(false, expr)) {
+                // in case of "for (...; false; ...)"
+                continueList = prevContList;
+                breakList = prevBreakList;
+                hasReturned = false;
+                return;
+            }
+
             pc2 = bytecode.currentPc();
             bytecode.addIndex(0);
         }
@@ -915,20 +932,23 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
     }
 
     public void atCondExpr(CondExpr expr) throws CompileError {
-        booleanExpr(false, expr.condExpr());
-        int pc = bytecode.currentPc();
-        bytecode.addIndex(0);   // correct later
-        expr.thenExpr().accept(this);
-        int dim1 = arrayDim;
-        bytecode.addOpcode(Opcode.GOTO);
-        int pc2 = bytecode.currentPc();
-        bytecode.addIndex(0);
-        bytecode.write16bit(pc, bytecode.currentPc() - pc + 1);
-        expr.elseExpr().accept(this);
-        if (dim1 != arrayDim)
-            throw new CompileError("type mismatch in ?:");
+        if (booleanExpr(false, expr.condExpr()))
+            expr.elseExpr().accept(this);
+        else {
+            int pc = bytecode.currentPc();
+            bytecode.addIndex(0);   // correct later
+            expr.thenExpr().accept(this);
+            int dim1 = arrayDim;
+            bytecode.addOpcode(Opcode.GOTO);
+            int pc2 = bytecode.currentPc();
+            bytecode.addIndex(0);
+            bytecode.write16bit(pc, bytecode.currentPc() - pc + 1);
+            expr.elseExpr().accept(this);
+            if (dim1 != arrayDim)
+                throw new CompileError("type mismatch in ?:");
 
-        bytecode.write16bit(pc2, bytecode.currentPc() - pc2 + 1);
+            bytecode.write16bit(pc2, bytecode.currentPc() - pc2 + 1);
+        }
     }
 
     static final int[] binOp = {
@@ -982,11 +1002,13 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         else {
             /* equation: &&, ||, ==, !=, <=, >=, <, >
             */
-            booleanExpr(true, expr);
-            bytecode.addIndex(7);
-            bytecode.addIconst(0);  // false
-            bytecode.addOpcode(Opcode.GOTO);
-            bytecode.addIndex(4);
+            if (!booleanExpr(true, expr)) {
+                bytecode.addIndex(7);
+                bytecode.addIconst(0);  // false
+                bytecode.addOpcode(Opcode.GOTO);
+                bytecode.addIndex(4);
+            }
+
             bytecode.addIconst(1);  // true
         }
     }
@@ -1088,9 +1110,10 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
     }
 
     /* Produces the opcode to branch if the condition is true.
-     * The oprand is not produced.
+     * The oprand (branch offset) is not produced.
      *
      * @return true if the compiled code is GOTO (always branch).
+     *                         GOTO is not produced.
      */
     private boolean booleanExpr(boolean branchIf, ASTree expr)
         throws CompileError
@@ -1105,22 +1128,31 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
             compareExpr(branchIf, bexpr.getOperator(), type1, bexpr);
         }
         else if (op == '!')
-            booleanExpr(!branchIf, ((Expr)expr).oprand1());
+            return booleanExpr(!branchIf, ((Expr)expr).oprand1());
         else if ((isAndAnd = (op == ANDAND)) || op == OROR) {
             BinExpr bexpr = (BinExpr)expr;
-            booleanExpr(!isAndAnd, bexpr.oprand1());
-            int pc = bytecode.currentPc();
-            bytecode.addIndex(0);       // correct later
-
-            booleanExpr(isAndAnd, bexpr.oprand2());
-            bytecode.write16bit(pc, bytecode.currentPc() - pc + 3);
-            if (branchIf != isAndAnd) {
-                bytecode.addIndex(6);   // skip GOTO instruction
-                bytecode.addOpcode(Opcode.GOTO);
+            if (booleanExpr(!isAndAnd, bexpr.oprand1())) {
+                exprType = BOOLEAN;
+                arrayDim = 0;
+                return true;
+            }
+            else {
+               int pc = bytecode.currentPc();
+               bytecode.addIndex(0);       // correct later
+               if (booleanExpr(isAndAnd, bexpr.oprand2()))
+                       bytecode.addOpcode(Opcode.GOTO);
+
+               bytecode.write16bit(pc, bytecode.currentPc() - pc + 3);
+               if (branchIf != isAndAnd) {
+                       bytecode.addIndex(6);   // skip GOTO instruction
+                       bytecode.addOpcode(Opcode.GOTO);
+               }
             }
         }
         else if (isAlwaysBranch(expr, branchIf)) {
-            bytecode.addOpcode(Opcode.GOTO);
+               // Opcode.GOTO is not added here.  The caller must add it.
+            exprType = BOOLEAN;
+            arrayDim = 0;
             return true;       // always branch
         }
         else {                          // others
@@ -1136,7 +1168,6 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         return false;
     }
 
-
     private static boolean isAlwaysBranch(ASTree expr, boolean branchIf) {
         if (expr instanceof Keyword) {
             int t = ((Keyword)expr).get();
@@ -1468,11 +1499,13 @@ public abstract class CodeGen extends Visitor implements Opcode, TokenId {
         else if (token == PLUSPLUS || token == MINUSMINUS)
             atPlusPlus(token, oprand, expr, true);
         else if (token == '!') {
-            booleanExpr(false, expr);
-            bytecode.addIndex(7);
-            bytecode.addIconst(1);
-            bytecode.addOpcode(Opcode.GOTO);
-            bytecode.addIndex(4);
+            if (!booleanExpr(false, expr)) {
+                bytecode.addIndex(7);
+                bytecode.addIconst(1);
+                bytecode.addOpcode(Opcode.GOTO);
+                bytecode.addIndex(4);
+            }
+
             bytecode.addIconst(0);
         }
         else if (token == CALL)         // method call

diff --git a/src/test/Test.java b/src/test/Test.java
index 51591cf960a4f87a9779979f619f794137178473..044b7d4e8407f786a11f2b261165e969309f0cb4 100644 (file)
--- a/src/test/Test.java
+++ b/src/test/Test.java
@@ -3,14 +3,18 @@ import javassist.*;
 public class Test {
     public static void main(String[] args) throws Exception {
         ClassPool cp = ClassPool.getDefault();
-        // ClassPool cp = new ClassPool();
-        cp.insertClassPath("./target/test-classes");
-        CtClass cc = cp.get("test4.JIRA207");
-        // cc.getClassFile().setMajorVersion(javassist.bytecode.ClassFile.JAVA_4);
-        CtMethod cm = cc.getDeclaredMethod("foo");
-        cm.insertBefore("throw new Exception();");
-        CtMethod cm2 = cc.getDeclaredMethod("run2");
-        cm2.insertBefore("throw new Exception();");
-        cc.writeFile();
+        CtClass newClass = cp.makeClass("test4.TestDeadcode");
+        addDeadCode(newClass, "public void evaluate5(){ boolean b = !false; b = false && b; b = true && true;"
+                            + "  b = true || b; b = b || false; }");
+
+        newClass.debugWriteFile();
+        Class<?> cClass = newClass.toClass();
+        Object o = cClass.newInstance();
+        java.lang.reflect.Method m = cClass.getMethod("evaluate5");
+        m.invoke(o);
+    }
+    private static void addDeadCode(CtClass cc, String meth) throws Exception {
+        CtMethod m = CtNewMethod.make(meth, cc);
+        cc.addMethod(m);
     }
 }

diff --git a/src/test/javassist/JvstTest4.java b/src/test/javassist/JvstTest4.java
index 73a1b08f1996b691d844b9ad180223cc6df734bd..b36e9b9f2e67178422552d8d0ab6cdf2cd38f9d4 100644 (file)
--- a/src/test/javassist/JvstTest4.java
+++ b/src/test/javassist/JvstTest4.java
@@ -994,4 +994,40 @@ public class JvstTest4 extends JvstTestRoot {
             }
         });
     }
+
+    // JIRA-230
+    public void testDeadcode() throws Exception {
+        CtClass newClass = sloader.makeClass("test4.TestDeadcode");
+        addDeadCode(newClass, "public void evaluate(){if (false) {int i = 0;}}");
+        addDeadCode(newClass, "public void evaluate2(){if (false == true) {int i = 0;}}");
+        addDeadCode(newClass, "public void evaluate3(){if (true) {} else {} int i = 0; if (false) {} else {} i++; }");
+        addDeadCode(newClass, "public void evaluate4(){ for (;false;); int i = false ? 1 : 0; while (true) { return; }}");
+        addDeadCode(newClass, "public void evaluate5(){ boolean b = !false; b = false && b; b = true && true;"
+                            + "  b = true || b; b = b || false; }");
+        addDeadCode(newClass, "public boolean evaluate6(){ return !false; }");
+        addDeadCode(newClass, "public boolean evaluate7(){ return !true; }");
+
+        newClass.debugWriteFile();
+        Class<?> cClass = newClass.toClass();
+        Object o = cClass.newInstance();
+        java.lang.reflect.Method m = cClass.getMethod("evaluate");
+        m.invoke(o);
+        m = cClass.getMethod("evaluate2");
+        m.invoke(o);
+        m = cClass.getMethod("evaluate3");
+        m.invoke(o);
+        m = cClass.getMethod("evaluate4");
+        m.invoke(o);
+        m = cClass.getMethod("evaluate6");
+        assertTrue((boolean)m.invoke(o));
+        m = cClass.getMethod("evaluate7");
+        assertFalse((boolean)m.invoke(o));
+        m = cClass.getMethod("evaluate5");
+        m.invoke(o);
+    }
+
+    private void addDeadCode(CtClass cc, String meth) throws Exception {
+        CtMethod m = CtNewMethod.make(meth, cc);
+        cc.addMethod(m);
+    }
 }