null-aware operators in the VM

Implement ??  ??= and ?. operators

BUG=23455
R=srdjan@google.com

Committed: https://github.com/dart-lang/sdk/commit/0535e37619995083321e22a3eda575920d36218c

[hausner, 2015-06-17 15:55:46]

hausner@google.com changed reviewers:
+ fschneider@google.com, srdjan@google.com

[hausner, 2015-06-17 15:57:25]

The changes in flow_graph_builder.cc need particular scrutiny.

[hausner, 2015-06-17 18:03:10]

Yes I have a CL out to add this to the VM. Thanks for the thorough tests!

[srdjan, 2015-06-17 18:15:36]

Please add tests that trigger optimized code for null-aware operators.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/ast.h#newcod...
runtime/vm/ast.h:1480: bool is_conditional)
use default value as in InstanceCall (consistency).

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1355: null_constant);
Fix indentation.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1460: null_constant);
Fix indentation.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:7133: const String& name) {
Fix indentation.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:10460: result = new BinaryOpNode(rhs->token_pos(),
new (Z)

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:11175: false);
Add comment what 'false' means:
     false /* is_conditional */);

Also below.

[hausner, 2015-06-17 22:24:57]

PTAL

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/ast.h#newcod...
runtime/vm/ast.h:1480: bool is_conditional)
On 2015/06/17 18:15:36, srdjan wrote:
> use default value as in InstanceCall (consistency).

Done.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1355: null_constant);
On 2015/06/17 18:15:36, srdjan wrote:
> Fix indentation.

Done.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1460: null_constant);
On 2015/06/17 18:15:36, srdjan wrote:
> Fix indentation.

Done.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:7133: const String& name) {
On 2015/06/17 18:15:36, srdjan wrote:
> Fix indentation.

Done.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:10460: result = new BinaryOpNode(rhs->token_pos(),
On 2015/06/17 18:15:36, srdjan wrote:
> new (Z)

Done.

https://codereview.chromium.org/1180903002/diff/40001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:11175: false);
On 2015/06/17 18:15:36, srdjan wrote:
> Add comment what 'false' means:
>      false /* is_conditional */);
> 
> Also below.

Done.

[srdjan, 2015-06-17 23:16:37]

lgtm

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/ast.h#newcod...
runtime/vm/ast.h:1453: bool is_conditional_;
const (also all below)

[Florian Schneider, 2015-06-18 14:09:00]

dbc:

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/flow_graph_b...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1445: // temp = left; temp === null ? right :
temp
Is this the same code as in EffectGraphVisitor::VisitBinaryNode?

I wonder if this can be expressed easier using a LetNode and a
ConditionalExprNode. This way, just one call to visit would build the graph for
you and it would result in less duplicated code.

[hausner, 2015-06-18 17:55:37]

Thank you for taking a look.

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/ast.h#newcod...
runtime/vm/ast.h:1453: bool is_conditional_;
On 2015/06/17 23:16:37, srdjan wrote:
> const (also all below)

Done.

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/flow_graph_b...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/1180903002/diff/60001/runtime/vm/flow_graph_b...
runtime/vm/flow_graph_builder.cc:1445: // temp = left; temp === null ? right :
temp
On 2015/06/18 14:09:00, Florian Schneider wrote:
> Is this the same code as in EffectGraphVisitor::VisitBinaryNode?
> 
> I wonder if this can be expressed easier using a LetNode and a
> ConditionalExprNode. This way, just one call to visit would build the graph
for
> you and it would result in less duplicated code.

There is some duplication here that I would like to clean up in a later checkin
if you don't mind.

I started out using ConditinoalExpr and Let nodes to implement this, but the
parser code to deal with assignments (turning right-hand expressions into
left-hand sides) got very ugly.

[hausner, 2015-06-18 18:05:33]

Committed patchset #5 (id:80001) manually as
0535e37619995083321e22a3eda575920d36218c (presubmit successful).