hg: nashorn/jdk8/nashorn: 8026137: Fix Issues with Binary Evaluation Order

[André Bargull]

"primitive type" was a bad choice of words, I rather meant 
"side-effect-free".

When all operands are primitives, most operations won't trigger 
side-effects. Only most operations because in `0 - (0 instanceof 1)` all 
operands are primitives, but `(0 instanceof 1)` still needs to be 
executed before the subtraction. So I was thinking of the following 
change for Codegen (incomplete -> `instanceof` etc. not handled!):


     private static boolean isSideEffectFree(final Expression rhs) {
         if (rhs instanceof LiteralNode.PrimitiveLiteralNode) {
             return true;
         } else if (rhs instanceof IdentNode) {
             return !rhs.getSymbol().isScope() && !rhs.getType().isObject();
         } else if (rhs instanceof UnaryNode) {
             UnaryNode unary = (UnaryNode) rhs;
             return isSideEffectFree(unary.rhs());
         } else if (rhs instanceof BinaryNode) {
             BinaryNode binary = (BinaryNode) rhs;
             return isSideEffectFree(binary.lhs())
                     && isSideEffectFree(binary.rhs());
         } else if (rhs instanceof TernaryNode) {
             TernaryNode ternary = (TernaryNode) rhs;
             return isSideEffectFree(ternary.getTest())
                     && isSideEffectFree(ternary.getTrueExpression())
                     && isSideEffectFree(ternary.getFalseExpression());
         }
         return false;
     }

     private static boolean safeLiteral(final Expression rhs) {
//        return isSideEffectFree(rhs);
         return rhs instanceof LiteralNode && !(rhs instanceof 
ArrayLiteralNode);
     }


- André


> Side effects can still occur with primitive types. As an example, this
> script from the test case as a numeric right hand side:
>
> ({valueOf: function(){throw 0}}) - ({valueOf: function(){throw 1}} - 1)
>
> But maybe something like this (pseudo-code) could work in
> CodeGenerator#safeLiteral:
>
>     isPrimitiveLiteralNode || (isIdentNode && isPrimitiveType)
>
> An IdentNode can have side effects if it is a global with a getter, but
> if we know it's type that shouldn't be the case.
>
> Hannes
>
> Am 2013-10-09 18:44, schrieb André Bargull:
> >/  Quick question for this change set:
> />/
> />/  Why does CodeGenerator#safeLiteral(...) need to test for literal nodes
> />/  instead of using type information? When the right-hand-side is a
> />/  primitive type, side effects cannot occur, so the additional
> />/  swap/dup/pop instructions can be omitted safely.
> />/
> />/  For example `function f(a){var c = 1; return a - c }` now emits:
> />/      ALOAD 1
> />/      ILOAD 4
> />/      SWAP
> />/      INVOKESTATIC jdk/nashorn/internal/runtime/JSType.toNumber
> />/  (Ljava/lang/Object;)D
> />/      DUP2_X1
> />/      POP2
> />/      I2D
> />/      DSUB
> />/
> />/  Using type info it's possible to change the bytecode to:
> />/      ALOAD 1
> />/      INVOKESTATIC jdk/nashorn/internal/runtime/JSType.toNumber
> />/  (Ljava/lang/Object;)D
> />/      ILOAD 4
> />/      I2D
> />/      DSUB
> />/
> />/
> />/  (Also: That was one of the bug reports which worried me a bit, because
> />/  I expected the changes to be non-trivial. :-(
> />/
> />/
> />/  - André/