null-aware operators in the VM

runtime/vm/flow_graph_builder.cc

Index: runtime/vm/flow_graph_builder.cc
diff --git a/runtime/vm/flow_graph_builder.cc b/runtime/vm/flow_graph_builder.cc
index a90c9faa09c61c788dd5ff2d4c6519d563ed6deb..2f98cfc779350ab0180d947fa380644b2e1926be 100644
--- a/runtime/vm/flow_graph_builder.cc
+++ b/runtime/vm/flow_graph_builder.cc
@@ -1335,6 +1335,36 @@ void EffectGraphVisitor::VisitBinaryOpNode(BinaryOpNode* node) {
       }
     }
     return;
+  } else if (node->kind() == Token::kIFNULL) {
+    // left ?? right. This operation cannot be overloaded.
+    // temp = left; temp === null ? right : temp
+    ValueGraphVisitor for_left_value(owner());
+    node->left()->Visit(&for_left_value);
+    Append(for_left_value);
+    Do(BuildStoreExprTemp(for_left_value.value()));
+
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ_STRICT,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    ValueGraphVisitor for_right_value(owner());
+    node->right()->Visit(&for_right_value);
+    for_right_value.Do(BuildStoreExprTemp(for_right_value.value()));
+
+    ValueGraphVisitor for_temp(owner());
+    // Nothing to do, left value is already loaded into temp.
+
+    Join(for_test, for_right_value, for_temp);
+    return;
   }
   ValueGraphVisitor for_left_value(owner());
   node->left()->Visit(&for_left_value);
@@ -1410,7 +1440,39 @@ void ValueGraphVisitor::VisitBinaryOpNode(BinaryOpNode* node) {
     }
     ReturnDefinition(BuildLoadExprTemp());
     return;
+  } else if (node->kind() == Token::kIFNULL) {
+    // left ?? right. This operation cannot be overloaded.
+    // temp = left; temp === null ? right : temp

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

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]

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.

+    ValueGraphVisitor for_left_value(owner());
+    node->left()->Visit(&for_left_value);
+    Append(for_left_value);
+    Do(BuildStoreExprTemp(for_left_value.value()));
+
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ_STRICT,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    ValueGraphVisitor for_right_value(owner());
+    node->right()->Visit(&for_right_value);
+    for_right_value.Do(BuildStoreExprTemp(for_right_value.value()));
+
+    ValueGraphVisitor for_temp(owner());
+    // Nothing to do, left value is already loaded into temp.
+
+    Join(for_test, for_right_value, for_temp);
+    ReturnDefinition(BuildLoadExprTemp());
+    return;
   }
+
   EffectGraphVisitor::VisitBinaryOpNode(node);
 }
 
@@ -2703,22 +2765,58 @@ void EffectGraphVisitor::VisitInstanceCallNode(InstanceCallNode* node) {
   ValueGraphVisitor for_receiver(owner());
   node->receiver()->Visit(&for_receiver);
   Append(for_receiver);
-  PushArgumentInstr* push_receiver = PushArgument(for_receiver.value());
-  ZoneGrowableArray<PushArgumentInstr*>* arguments =
-      new(Z) ZoneGrowableArray<PushArgumentInstr*>(
-          node->arguments()->length() + 1);
-  arguments->Add(push_receiver);
+  if (node->is_conditional()) {
+    Do(BuildStoreExprTemp(for_receiver.value()));
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    EffectGraphVisitor for_true(owner());
+    EffectGraphVisitor for_false(owner());
+
+    StoreLocalNode* store_null =
+        new(Z) StoreLocalNode(Scanner::kNoSourcePos, temp_var, null_constant);
+    store_null->Visit(&for_true);
+
+    InstanceCallNode* call =
+        new(Z) InstanceCallNode(node->token_pos(),
+                                load_temp,
+                                node->function_name(),
+                                node->arguments());
+    StoreLocalNode* store_result =
+        new(Z) StoreLocalNode(Scanner::kNoSourcePos, temp_var, call);
+    store_result->Visit(&for_false);
+
+    Join(for_test, for_true, for_false);
+    ReturnDefinition(BuildLoadExprTemp());
+  } else {
+    PushArgumentInstr* push_receiver = PushArgument(for_receiver.value());
+    ZoneGrowableArray<PushArgumentInstr*>* arguments =
+        new(Z) ZoneGrowableArray<PushArgumentInstr*>(
+            node->arguments()->length() + 1);
+    arguments->Add(push_receiver);
 
-  BuildPushArguments(*node->arguments(), arguments);
-  InstanceCallInstr* call = new(Z) InstanceCallInstr(
-      node->token_pos(),
-      node->function_name(),
-      Token::kILLEGAL,
-      arguments,
-      node->arguments()->names(),
-      1,
-      owner()->ic_data_array());
-  ReturnDefinition(call);
+    BuildPushArguments(*node->arguments(), arguments);
+    InstanceCallInstr* call = new(Z) InstanceCallInstr(
+        node->token_pos(),
+        node->function_name(),
+        Token::kILLEGAL,
+        arguments,
+        node->arguments()->names(),
+        1,
+        owner()->ic_data_array());
+    ReturnDefinition(call);
+  }
 }
 
 
@@ -3063,20 +3161,55 @@ void EffectGraphVisitor::VisitInstanceGetterNode(InstanceGetterNode* node) {
   ValueGraphVisitor for_receiver(owner());
   node->receiver()->Visit(&for_receiver);
   Append(for_receiver);
-  PushArgumentInstr* push_receiver = PushArgument(for_receiver.value());
-  ZoneGrowableArray<PushArgumentInstr*>* arguments =
-      new(Z) ZoneGrowableArray<PushArgumentInstr*>(1);
-  arguments->Add(push_receiver);
-  const String& name =
-      String::ZoneHandle(Z, Field::GetterSymbol(node->field_name()));
-  InstanceCallInstr* call = new(Z) InstanceCallInstr(
-      node->token_pos(),
-      name,
-      Token::kGET,
-      arguments, Object::null_array(),
-      1,
-      owner()->ic_data_array());
-  ReturnDefinition(call);
+  if (node->is_conditional()) {
+    Do(BuildStoreExprTemp(for_receiver.value()));
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    EffectGraphVisitor for_true(owner());
+    EffectGraphVisitor for_false(owner());
+
+    StoreLocalNode* store_null =
+        new(Z) StoreLocalNode(Scanner::kNoSourcePos, temp_var, null_constant);
+    store_null->Visit(&for_true);
+
+    InstanceGetterNode* getter =
+        new(Z) InstanceGetterNode(node->token_pos(),
+                                  load_temp,
+                                  node->field_name());
+    StoreLocalNode* store_getter =
+        new(Z) StoreLocalNode(Scanner::kNoSourcePos, temp_var, getter);
+    store_getter->Visit(&for_false);
+
+    Join(for_test, for_true, for_false);
+    ReturnDefinition(BuildLoadExprTemp());
+  } else {
+    PushArgumentInstr* push_receiver = PushArgument(for_receiver.value());
+    ZoneGrowableArray<PushArgumentInstr*>* arguments =
+        new(Z) ZoneGrowableArray<PushArgumentInstr*>(1);
+    arguments->Add(push_receiver);
+    const String& name =
+        String::ZoneHandle(Z, Field::GetterSymbol(node->field_name()));
+    InstanceCallInstr* call = new(Z) InstanceCallInstr(
+        node->token_pos(),
+        name,
+        Token::kGET,
+        arguments, Object::null_array(),
+        1,
+        owner()->ic_data_array());
+    ReturnDefinition(call);
+  }
 }
 
 
@@ -3104,6 +3237,37 @@ void EffectGraphVisitor::BuildInstanceSetterArguments(
 
 
 void EffectGraphVisitor::VisitInstanceSetterNode(InstanceSetterNode* node) {
+  if (node->is_conditional()) {
+    ValueGraphVisitor for_receiver(owner());
+    node->receiver()->Visit(&for_receiver);
+    Append(for_receiver);
+    Do(BuildStoreExprTemp(for_receiver.value()));
+
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    EffectGraphVisitor for_true(owner());
+    EffectGraphVisitor for_false(owner());
+
+    InstanceSetterNode* setter =
+        new(Z) InstanceSetterNode(node->token_pos(),
+                                  load_temp,
+                                  node->field_name(),
+                                  node->value());
+    setter->Visit(&for_false);
+    Join(for_test, for_true, for_false);
+    return;
+  }
   ZoneGrowableArray<PushArgumentInstr*>* arguments =
       new(Z) ZoneGrowableArray<PushArgumentInstr*>(2);
   BuildInstanceSetterArguments(node, arguments, kResultNotNeeded);
@@ -3122,6 +3286,42 @@ void EffectGraphVisitor::VisitInstanceSetterNode(InstanceSetterNode* node) {
 
 
 void ValueGraphVisitor::VisitInstanceSetterNode(InstanceSetterNode* node) {
+  if (node->is_conditional()) {
+    ValueGraphVisitor for_receiver(owner());
+    node->receiver()->Visit(&for_receiver);
+    Append(for_receiver);
+    Do(BuildStoreExprTemp(for_receiver.value()));
+
+    LocalVariable* temp_var = owner()->parsed_function().expression_temp_var();
+    LoadLocalNode* load_temp =
+        new(Z) LoadLocalNode(Scanner::kNoSourcePos, temp_var);
+    LiteralNode* null_constant =
+        new(Z) LiteralNode(Scanner::kNoSourcePos, Object::null_instance());
+    ComparisonNode* check_is_null =
+        new(Z) ComparisonNode(Scanner::kNoSourcePos,
+                              Token::kEQ,
+                              load_temp,
+                              null_constant);
+    TestGraphVisitor for_test(owner(), Scanner::kNoSourcePos);
+    check_is_null->Visit(&for_test);
+
+    ValueGraphVisitor for_true(owner());
+    null_constant->Visit(&for_true);
+    for_true.Do(BuildStoreExprTemp(for_true.value()));
+
+    ValueGraphVisitor for_false(owner());
+    InstanceSetterNode* setter =
+        new(Z) InstanceSetterNode(node->token_pos(),
+                                  load_temp,
+                                  node->field_name(),
+                                  node->value());
+    setter->Visit(&for_false);
+    for_false.Do(BuildStoreExprTemp(for_false.value()));
+
+    Join(for_test, for_true, for_false);
+    ReturnDefinition(BuildLoadExprTemp());
+    return;
+  }
   ZoneGrowableArray<PushArgumentInstr*>* arguments =
       new(Z) ZoneGrowableArray<PushArgumentInstr*>(2);
   BuildInstanceSetterArguments(node, arguments, kResultNeeded);