"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é/
