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é
Changeset: 03a68e7ca1d5
Author: lagergren
Date: 2013-10-09 17:53 +0200
URL:http://hg.openjdk.java.net/nashorn/jdk8/nashorn/rev/03a68e7ca1d5
8026137: Fix Issues with Binary Evaluation Order
Reviewed-by: hannesw, jlaskey
Contributed-by:marcus.lagergren at oracle.com
<http://mail.openjdk.java.net/mailman/listinfo/nashorn-dev>,attila.szegedi
at oracle.com
<http://mail.openjdk.java.net/mailman/listinfo/nashorn-dev>
! src/jdk/nashorn/internal/codegen/Attr.java
! src/jdk/nashorn/internal/codegen/BranchOptimizer.java
! src/jdk/nashorn/internal/codegen/CodeGenerator.java
! src/jdk/nashorn/internal/codegen/CompileUnit.java
! src/jdk/nashorn/internal/codegen/Compiler.java
! src/jdk/nashorn/internal/codegen/FinalizeTypes.java
! src/jdk/nashorn/internal/codegen/MethodEmitter.java
! src/jdk/nashorn/internal/codegen/WeighNodes.java
! src/jdk/nashorn/internal/codegen/types/BooleanType.java
! src/jdk/nashorn/internal/codegen/types/ObjectType.java
! src/jdk/nashorn/internal/codegen/types/Type.java
! src/jdk/nashorn/internal/ir/AccessNode.java
! src/jdk/nashorn/internal/ir/BaseNode.java
! src/jdk/nashorn/internal/ir/CallNode.java
! src/jdk/nashorn/internal/ir/IdentNode.java
! src/jdk/nashorn/internal/ir/IndexNode.java
! src/jdk/nashorn/internal/ir/LiteralNode.java
! src/jdk/nashorn/internal/ir/RuntimeNode.java
- src/jdk/nashorn/internal/ir/TypeOverride.java
! src/jdk/nashorn/internal/ir/UnaryNode.java
! src/jdk/nashorn/internal/ir/visitor/NodeOperatorVisitor.java
! src/jdk/nashorn/internal/parser/TokenType.java
! src/jdk/nashorn/internal/runtime/Context.java
! src/jdk/nashorn/internal/runtime/JSType.java
! src/jdk/nashorn/internal/runtime/arrays/JavaArrayIterator.java
! src/jdk/nashorn/internal/runtime/arrays/ReverseJavaArrayIterator.java
! src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java
+ test/script/basic/JDK-8026137.js
+ test/script/basic/JDK-8026137.js.EXPECTED
