Truffle: [TypeSystemCodeGenerator + ShortCircuit] generate a buggy code

[christian.humer at gmail.com]

Hi,

Thanks for the report. This is indeed broken. I fixed it and it will 
arrive in the OpenJDK repository sometime today.

Some further remarks about your code.

1) The order attribute declarations in your specializations have no 
effect anymore. You can safely remove them. The method declaration order 
is now used to define the order in which specializations are 
specialized.

2) In our current languages we tend to implement short circuiting or 
nodes like this without specializations:

@NodeInfo(shortName = "or")
public abstract static class OrNode extends ExpressionNode {

@Child private ExpressionNode leftNode;
@Child private ExpressionNode rightNode;

@Child private YesNode booleanCast = YesNodeFactory.create(null);

public OrNode(ExpressionNode leftNode, ExpressionNode rightNode) {
     this.leftNode = leftNode;
     this.rightNode = rightNode;
}

public Object execute(VirtualFrame frame) {
     Object left = leftNode.execute(frame);
     if (booleanCast.executeBoolean(frame, left)) {
         return rightNode.execute(frame);
     }
     return left;
}
}

This would also be a valid workaround for the bug you have reported.

Thanks again, and have fun.

- Christian Humer




------ Original Message ------
From: "Mohaned Y Qunaibit" <mqunaibi at uci.edu>
To: graal-dev at openjdk.java.net
Sent: 05.06.2015 09:19:50
Subject: Truffle: [TypeSystemCodeGenerator + ShortCircuit] generate a 
buggy code

>Hi,
>
>When we have a binary node that doesn't always execute both nodes, 
>(like
>AND, OR), TypeSystemCodeGenerator generate a type specialized node but 
>with
>the assumption that it can read both two children types.
>Example:
>Evaluate: "2 or 0"
>
>     @NodeInfo(shortName = "or")
>     @GenerateNodeFactory
>     public abstract static class OrNode extends BinaryOpNode {
>
>         @Child protected YesNode booleanCast;
>         public OrNode() {
>             this.booleanCast = 
>YesNodeFactory.create(EmptyNode.create());
>         }
>
>         @ShortCircuit("rightNode")
>         public boolean needsRightNode(VirtualFrame frame, Object left) 
>{
>             return !booleanCast.executeBoolean(frame, left);
>         }
>
>         @Specialization(order = 0)
>         public boolean doBoolean(boolean left, boolean needsRight, 
>boolean
>right) {
>             return needsRight ? right : left;
>         }
>
>         @Specialization(order = 1)
>         public int doInteger(int left, boolean needsRight, int right) {
>             return needsRight ? right : left;
>         }
>
>     }
>
>Generated node:
>
>         @GeneratedBy(OrNode.class)
>         public static final class OrNodeGen extends OrNode implements
>SpecializedNode {
>         ...
>                 public Object execute(VirtualFrame frameValue) {
>                     Object leftNodeValue_ = 
>executeLeftNode_(frameValue);
>                     boolean hasRightNodeValue =
>root.needsRightNode(frameValue, leftNodeValue_); // return false
>                     Object *rightNodeValue_ = null*;
>                     if (hasRightNodeValue) {
>                         rightNodeValue_ = 
>executeRightNode_(frameValue);
>                     }
>                     return execute_(frameValue, leftNodeValue_,
>hasRightNodeValue, *rightNodeValue_*);
>                 }
>
>             ...
>             @GeneratedBy(methodName = "doInteger(int, boolean, int)", 
>value
>= OrNode.class)
>             private static final class IntegerNode_ extends BaseNode_ {
>
>                 private final Class<?> leftNodeImplicitType;
>                 private final Class<?> rightNodeImplicitType;
>
>                 IntegerNode_(OrNodeGen root, Object leftNodeValue, 
>Object
>*rightNodeValue*) { // rightNodeValue = null
>                     super(root, 2);
>                     this.leftNodeImplicitType =
>LanguageTypesGen.getImplicitIntegerClass(leftNodeValue);
>                     this.rightNodeImplicitType =
>*LanguageTypesGen.getImplicitIntegerClass(rightNodeValue);
>// Throws exception*
>                 }
>                 ...
>             }
>         ...
>         }
>
>Here after the executing of the uninitialized node, *rightNodeValue_* 
>will
>carry a *null* value all the way until it gets replaced with
>*IntegerNode_* which
>will call *getImplicitIntegerClass* and throw an exception because of 
>the
>*null* value.
>
>The suggested fix:
>TypeSystemCodeGenerator.java:53
>-        builder.startStaticCall(createTypeSystemGen(typeSystem),
>getImplicitClass(typeSystem,
>type)).tree(value);
>*+        builder.startParantheses().tree(value).string(" ==
>").nullLiteral().end().string(" ? ").nullLiteral().string(" : ")*
>*+
>   .startStaticCall(createTypeSystemGen(typeSystem),
>getImplicitClass(typeSystem,
>type)).tree(value);*
>
>
>
>Best,
>Mohaned Qunaibit