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

[Hannes Wallnoefer]

Looks good. One thing I'm noting is that String is currently handled as 
ObjectType (i.e. getType().isObject() will return true), which is true 
in Java terms, but in this context we should treat String as primitive, 
right?

Hannes

Am 2013-10-14 21:59, schrieb 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é/