Eliminate temporary locals for some expressions

runtime/vm/parser.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/parser.h"
 
 #include "lib/invocation_mirror.h"
 #include "vm/bigint_operations.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
@@ skipping 98 matching lines @@
 }
 
 
 LocalVariable* ParsedFunction::CreateExpressionTempVar(intptr_t token_pos) {
   return new LocalVariable(token_pos,
                            Symbols::ExprTemp(),
                            Type::ZoneHandle(Type::DynamicType()));
 }
 
 
-LocalVariable* ParsedFunction::CreateArrayLiteralVar(intptr_t token_pos) {
-  return new LocalVariable(token_pos,
-                           Symbols::ArrayLiteralVar(),
-                           Type::ZoneHandle(Type::ArrayType()));
-}
-
-
 void ParsedFunction::SetNodeSequence(SequenceNode* node_sequence) {
   ASSERT(node_sequence_ == NULL);
   ASSERT(node_sequence != NULL);
   node_sequence_ = node_sequence;
 }
 
 
 LocalVariable* ParsedFunction::GetSavedArgumentsDescriptorVar() const {
   const int num_parameters = function().NumParameters();
   LocalScope* scope = node_sequence()->scope();
@@ skipping 645 matching lines @@
     case RawFunction::kConstImplicitGetter:
       node_sequence = parser.ParseStaticConstGetter(func);
       break;
     case RawFunction::kMethodExtractor:
       node_sequence = parser.ParseMethodExtractor(func);
       break;
     default:
       UNREACHABLE();
   }
 
-  parsed_function->set_array_literal_var(
-      ParsedFunction::CreateArrayLiteralVar(func.token_pos()));
-  node_sequence->scope()->AddVariable(parsed_function->array_literal_var());
-
   if (!HasReturnNode(node_sequence)) {
     // Add implicit return node.
     node_sequence->Add(new ReturnNode(func.end_token_pos()));
   }
   if (parsed_function->has_expression_temp_var()) {
     node_sequence->scope()->AddVariable(parsed_function->expression_temp_var());
   }
   if (parsed_function->has_saved_current_context_var()) {
     node_sequence->scope()->AddVariable(
         parsed_function->saved_current_context_var());
@@ skipping 597 matching lines @@
 }
 
 
 static const String& PrivateCoreLibName(const String& str) {
   const Library& core_lib = Library::Handle(Library::CoreLibrary());
   const String& private_name = String::ZoneHandle(core_lib.PrivateName(str));
   return private_name;
 }
 
 
-LocalVariable* Parser::BuildArrayTempLocal(intptr_t token_pos) {
-  char name[64];
-  OS::SNPrint(name, 64, ":arrlit%"Pd, token_pos);
-  LocalVariable* temp =
-      new LocalVariable(token_pos,
-                        String::ZoneHandle(Symbols::New(name)),
-                        Type::ZoneHandle(Type::ArrayType()));
-  current_block_->scope->AddVariable(temp);
-  return temp;
-}
-
-
 StaticCallNode* Parser::BuildInvocationMirrorAllocation(
     intptr_t call_pos,
     const String& function_name,
     const ArgumentListNode& function_args) {
   const intptr_t args_pos = function_args.token_pos();
   // Build arguments to the call to the static
   // InvocationMirror._allocateInvocationMirror method.
   ArgumentListNode* arguments = new ArgumentListNode(args_pos);
   // The first argument is the original function name.
   arguments->Add(new LiteralNode(args_pos, function_name));
   // The second argument is the arguments descriptor of the original function.
   const Array& args_descriptor =
       Array::ZoneHandle(ArgumentsDescriptor::New(function_args.length(),
                                                  function_args.names()));
   arguments->Add(new LiteralNode(args_pos, args_descriptor));
   // The third argument is an array containing the original function arguments,
   // including the receiver.
-  ArrayNode* args_array = new ArrayNode(
-      args_pos, Type::ZoneHandle(Type::ArrayType()),
-      *BuildArrayTempLocal(call_pos));
+  ArrayNode* args_array =
+      new ArrayNode(args_pos, Type::ZoneHandle(Type::ArrayType()));
   for (intptr_t i = 0; i < function_args.length(); i++) {
     args_array->AddElement(function_args.NodeAt(i));
   }
   arguments->Add(args_array);
   // Lookup the static InvocationMirror._allocateInvocationMirror method.
   const Class& mirror_class =
       Class::Handle(LookupCoreClass(Symbols::InvocationMirror()));
   ASSERT(!mirror_class.IsNull());
   const Function& allocation_function = Function::ZoneHandle(
       mirror_class.LookupStaticFunction(
@@ skipping 6298 matching lines @@
       return left;
     }
     ASSERT(selector != NULL);
     left = selector;
   }
 }
 
 
 AstNode* Parser::ParsePostfixExpr() {
   TRACE_PARSER("ParsePostfixExpr");
-  const intptr_t postfix_expr_pos = TokenPos();
+  const intptr_t token_pos = TokenPos();

[hausner, 2013/05/28 17:00:47]

Why are you renaming this? The old name expresses what is at that particular
token position.

[Florian Schneider, 2013/05/30 09:29:31]

I found it too long so that it caused some ugly line breaks, but I can undo
renaming it.

   AstNode* postfix_expr = ParsePrimary();
   postfix_expr = ParseSelectors(postfix_expr, false);
   if (IsIncrementOperator(CurrentToken())) {
     TRACE_PARSER("IncrementOperator");
     Token::Kind incr_op = CurrentToken();
     if (!IsAssignableExpr(postfix_expr)) {
       ErrorMsg("expression is not assignable");
     }
     ConsumeToken();
     // Not prefix.
+    if (postfix_expr->IsInstanceGetterNode()) {

[hausner, 2013/05/28 17:00:47]

I think this special case needs to be handled in
PrepareCompoundAssignmentNodes(), not here. If you do it just here, you only
handle o.x++, but not o.x += 1, nor ++o.x, and neither cascades.

[Florian Schneider, 2013/05/30 09:29:31]

Yes, I'll handle this and the other cases in PrepareCompoundAssignmentNodes().

+      InstanceGetterNode* getter = postfix_expr->AsInstanceGetterNode();
+      Token::Kind binary_op =
+          (incr_op == Token::kINCR) ? Token::kADD : Token::kSUB;
+      if (!IsSimpleLocalOrLiteralNode(getter->receiver())) {
+        LetNode* result = new LetNode(token_pos, 2);
+        result->SetTempExpression(0, getter->receiver());
+        result->SetTempExpression(
+            1,
+            new InstanceGetterNode(
+                token_pos,
+                new LoadLocalNode(token_pos, result->temp(0)),
+                getter->field_name()));
+        BinaryOpNode* add = new BinaryOpNode(
+            token_pos,
+            binary_op,
+            new LoadLocalNode(token_pos, result->temp(1)),
+            new LiteralNode(token_pos, Smi::ZoneHandle(Smi::New(1))));
+        InstanceSetterNode* store = new InstanceSetterNode(
+            token_pos,
+            new LoadLocalNode(token_pos, result->temp(0)),
+            getter->field_name(),
+            add);
+        CommaNode* body = new CommaNode(
+            token_pos,
+            store,
+            new LoadLocalNode(token_pos, result->temp(1)));  // Result value.
+        result->set_body(body);
+        return result;
+      } else {
+        LetNode* result = new LetNode(token_pos, 1);
+        result->SetTempExpression(
+            0,
+            new InstanceGetterNode(
+                token_pos,
+                getter->receiver(),
+                getter->field_name()));
+        BinaryOpNode* add = new BinaryOpNode(
+            token_pos,
+            binary_op,
+            new LoadLocalNode(token_pos, result->temp(0)),
+            new LiteralNode(token_pos, Smi::ZoneHandle(Smi::New(1))));
+        InstanceSetterNode* store = new InstanceSetterNode(
+            token_pos,
+            getter->receiver(),
+            getter->field_name(),
+            add);
+        CommaNode* body = new CommaNode(
+            token_pos,
+            store,
+            new LoadLocalNode(token_pos, result->temp(0)));  // Result value.
+        result->set_body(body);
+        return result;
+      }
+    }
     AstNode* left_expr = PrepareCompoundAssignmentNodes(&postfix_expr);
     const LocalVariable* temp = GetIncrementTempLocal();

[hausner, 2013/05/28 17:00:47]

Can this temporary local variable be handled with your new LetNode construct? If
so, please at least add TODOs where simplification is possible.

[Florian Schneider, 2013/05/30 09:29:31]

Yes, definitely. The plan is to replace it here as well.

     AstNode* save =
-        new StoreLocalNode(postfix_expr_pos, temp, postfix_expr);
+        new StoreLocalNode(token_pos, temp, postfix_expr);
     Token::Kind binary_op =
         (incr_op == Token::kINCR) ? Token::kADD : Token::kSUB;
     BinaryOpNode* add = new BinaryOpNode(
-        postfix_expr_pos,
+        token_pos,
         binary_op,
         save,
-        new LiteralNode(postfix_expr_pos, Smi::ZoneHandle(Smi::New(1))));
+        new LiteralNode(token_pos, Smi::ZoneHandle(Smi::New(1))));
     AstNode* store = CreateAssignmentNode(left_expr, add);
     // The result is a pair of the (side effects of the) store followed by
     // the (value of the) initial value temp variable load.
     return new CommaNode(
-        postfix_expr_pos,
+        token_pos,
         store,
-        new LoadLocalNode(postfix_expr_pos, temp));
+        new LoadLocalNode(token_pos, temp));
   }
   return postfix_expr;
 }
 
 
 // Resolve the given type and its type arguments from the given scope class
 // according to the given type finalization mode.
 // If the given scope class is null, use the current library, but do not try to
 // resolve type parameters.
 // Not all involved type classes may get resolved yet, but at least the type
@@ skipping 1054 matching lines @@
     }
     factory_type_args = factory_type_args.Canonicalize();
     ArgumentListNode* factory_param = new ArgumentListNode(literal_pos);
     if (element_list.length() == 0) {
       // TODO(srdjan): Use Object::empty_array once issue 9871 has been fixed.
       Array& empty_array = Array::ZoneHandle(Object::empty_array().raw());
       LiteralNode* empty_array_literal =
           new LiteralNode(TokenPos(), empty_array);
       factory_param->Add(empty_array_literal);
     } else {
-      const LocalVariable& temp_local = *BuildArrayTempLocal(type_pos);
-      ArrayNode* list =
-          new ArrayNode(TokenPos(), type, temp_local, element_list);
+      ArrayNode* list = new ArrayNode(TokenPos(), type, element_list);
       factory_param->Add(list);
     }
     return CreateConstructorCallNode(literal_pos,
                                      factory_type_args,
                                      factory_method,
                                      factory_param);
   }
 }
 
 
 ConstructorCallNode* Parser::CreateConstructorCallNode(
     intptr_t token_pos,
     const AbstractTypeArguments& type_arguments,
     const Function& constructor,
     ArgumentListNode* arguments) {
   if (!type_arguments.IsNull() && !type_arguments.IsInstantiated()) {
     EnsureExpressionTemp();
   }
-  LocalVariable* allocated =
-      CreateTempConstVariable(token_pos, "alloc");
   return new ConstructorCallNode(token_pos,
                                  type_arguments,
                                  constructor,
-                                 arguments,
-                                 allocated);
+                                 arguments);
 }
 
 
 static void AddKeyValuePair(GrowableArray<AstNode*>* pairs,
                             bool is_const,
                             AstNode* key,
                             AstNode* value) {
   if (is_const) {
     ASSERT(key->IsLiteralNode());
     ASSERT(key->AsLiteralNode()->literal().IsString());
@@ skipping 177 matching lines @@
       factory_type_args = factory_type.arguments();
       ASSERT(factory_type_args.Length() == factory_class.NumTypeArguments());
     }
     factory_type_args = factory_type_args.Canonicalize();
     ArgumentListNode* factory_param = new ArgumentListNode(literal_pos);
     // The kv_pair array is temporary and of element type dynamic. It is passed
     // to the factory to initialize a properly typed map.
     ArrayNode* kv_pairs = new ArrayNode(
         TokenPos(),
         Type::ZoneHandle(Type::ArrayType()),
-        *BuildArrayTempLocal(type_pos),
         kv_pairs_list);
     factory_param->Add(kv_pairs);
     return CreateConstructorCallNode(literal_pos,
                                      factory_type_args,
                                      factory_method,
                                      factory_param);
   }
 }
 
 
@@ skipping 366 matching lines @@
       values_list.Add(expr);
     }
   }
   if (is_compiletime_const) {
     primary = new LiteralNode(literal_start, Interpolate(values_list));
   } else {
     ArgumentListNode* interpolate_arg = new ArgumentListNode(TokenPos());
     ArrayNode* values = new ArrayNode(
         TokenPos(),
         Type::ZoneHandle(Type::ArrayType()),
-        *BuildArrayTempLocal(TokenPos()),
         values_list);
     interpolate_arg->Add(values);
     primary = MakeStaticCall(Symbols::StringBase(),
                              PrivateCoreLibName(Symbols::Interpolate()),
                              interpolate_arg);
   }
   return primary;
 }
 
 
@@ skipping 500 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart