Cleanup AstGraphBuilder::AddHomeObjectIfNeeded a bit.

src/compiler/ast-graph-builder.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/ast-graph-builder.h"
 
 #include "src/compiler.h"
 #include "src/compiler/ast-loop-assignment-analyzer.h"
 #include "src/compiler/control-builders.h"
 #include "src/compiler/linkage.h"
@@ skipping 1327 matching lines @@
     ObjectLiteral::Property* property = expr->properties()->at(i);
     environment()->Push(property->is_static() ? literal : proto);
 
     VisitForValue(property->key());
     environment()->Push(
         BuildToName(environment()->Pop(), expr->GetIdForProperty(i)));
     VisitForValue(property->value());
     Node* value = environment()->Pop();
     Node* key = environment()->Pop();
     Node* receiver = environment()->Pop();
+    BuildSetHomeObject(value, receiver, property->value());
+
     switch (property->kind()) {
       case ObjectLiteral::Property::CONSTANT:
       case ObjectLiteral::Property::MATERIALIZED_LITERAL:
       case ObjectLiteral::Property::PROTOTYPE:
         UNREACHABLE();
       case ObjectLiteral::Property::COMPUTED: {
         const Operator* op =
             javascript()->CallRuntime(Runtime::kDefineClassMethod, 3);
         NewNode(op, receiver, key, value);
         break;
       }
       case ObjectLiteral::Property::GETTER: {
         Node* attr = jsgraph()->Constant(DONT_ENUM);
         const Operator* op = javascript()->CallRuntime(
             Runtime::kDefineGetterPropertyUnchecked, 4);
         NewNode(op, receiver, key, value, attr);
         break;
       }
       case ObjectLiteral::Property::SETTER: {
         Node* attr = jsgraph()->Constant(DONT_ENUM);
         const Operator* op = javascript()->CallRuntime(
             Runtime::kDefineSetterPropertyUnchecked, 4);
         NewNode(op, receiver, key, value, attr);
         break;
       }
     }
-
-    AddHomeObjectIfNeeded(property->value(), value, receiver);
   }
 
   // Transform both the class literal and the prototype to fast properties.
   const Operator* op = javascript()->CallRuntime(Runtime::kToFastProperties, 1);
   NewNode(op, environment()->Pop());  // prototype
   NewNode(op, environment()->Pop());  // literal
 
   // Assign to class variable.
   if (expr->scope() != NULL) {
     DCHECK_NOT_NULL(expr->class_variable_proxy());
     Variable* var = expr->class_variable_proxy()->var();
     BuildVariableAssignment(var, literal, Token::INIT_CONST, BailoutId::None());
   }
 
   PrepareFrameState(literal, expr->id(), ast_context()->GetStateCombine());
   ast_context()->ProduceValue(literal);
 }
 
 
-void AstGraphBuilder::AddHomeObjectIfNeeded(Expression* expr, Node* function,
-                                            Node* home_object) {
-  if (FunctionLiteral::NeedsHomeObject(expr)) {
-    Unique<Name> name = MakeUnique(isolate()->factory()->home_object_symbol());
-    Node* store = NewNode(javascript()->StoreNamed(language_mode(), name),
-                          function, home_object);
-    PrepareFrameState(store, BailoutId::None());
-  }
-}
-
-
 void AstGraphBuilder::VisitNativeFunctionLiteral(NativeFunctionLiteral* expr) {
   UNREACHABLE();
 }
 
 
 void AstGraphBuilder::VisitConditional(Conditional* expr) {
   IfBuilder compare_if(this);
   VisitForTest(expr->condition());
   Node* condition = environment()->Pop();
   compare_if.If(condition);
@@ skipping 81 matching lines @@
         // contains computed properties with an uninitialized value.
         if (key->value()->IsInternalizedString()) {
           if (property->emit_store()) {
             VisitForValue(property->value());
             Node* value = environment()->Pop();
             Unique<Name> name = MakeUnique(key->AsPropertyName());
             Node* store =
                 NewNode(javascript()->StoreNamed(language_mode(), name),
                         literal, value);
             PrepareFrameState(store, key->id());
-
-            AddHomeObjectIfNeeded(property->value(), value, literal);
+            BuildSetHomeObject(value, literal, property->value());
           } else {
             VisitForEffect(property->value());
           }
           break;
         }
         environment()->Push(literal);  // Duplicate receiver.
         VisitForValue(property->key());
         VisitForValue(property->value());
         Node* value = environment()->Pop();
         Node* key = environment()->Pop();
         Node* receiver = environment()->Pop();
         if (property->emit_store()) {
           Node* language = jsgraph()->Constant(SLOPPY);
           const Operator* op =
               javascript()->CallRuntime(Runtime::kSetProperty, 4);
           NewNode(op, receiver, key, value, language);
-
-          AddHomeObjectIfNeeded(property->value(), value, receiver);
+          BuildSetHomeObject(value, receiver, property->value());
         }
         break;
       }
       case ObjectLiteral::Property::PROTOTYPE: {
         environment()->Push(literal);  // Duplicate receiver.
         VisitForValue(property->value());
         Node* value = environment()->Pop();
         Node* receiver = environment()->Pop();
         DCHECK(property->emit_store());
         const Operator* op =
@@ skipping 15 matching lines @@
         break;
     }
   }
 
   // Create nodes to define accessors, using only a single call to the runtime
   // for each pair of corresponding getters and setters.
   for (AccessorTable::Iterator it = accessor_table.begin();
        it != accessor_table.end(); ++it) {
     VisitForValue(it->first);
     VisitForValueOrNull(it->second->getter);
+    BuildSetHomeObject(environment()->Top(), literal, it->second->getter);
     VisitForValueOrNull(it->second->setter);
+    BuildSetHomeObject(environment()->Top(), literal, it->second->setter);
     Node* setter = environment()->Pop();
     Node* getter = environment()->Pop();
     Node* name = environment()->Pop();
-    AddHomeObjectIfNeeded(it->second->getter, getter, literal);
-    AddHomeObjectIfNeeded(it->second->setter, setter, literal);
     Node* attr = jsgraph()->Constant(NONE);
     const Operator* op =
         javascript()->CallRuntime(Runtime::kDefineAccessorPropertyUnchecked, 5);
     Node* call = NewNode(op, literal, name, getter, setter, attr);
     // This should not lazy deopt on a new literal.
     PrepareFrameState(call, BailoutId::None());
   }
 
   // Object literals have two parts. The "static" part on the left contains no
   // computed property names, and so we can compute its map ahead of time; see
@@ skipping 11 matching lines @@
     VisitForValue(property->key());
     environment()->Push(BuildToName(environment()->Pop(),
                                     expr->GetIdForProperty(property_index)));
     // TODO(mstarzinger): For ObjectLiteral::Property::PROTOTYPE the key should
     // not be on the operand stack while the value is being evaluated. Come up
     // with a repro for this and fix it. Also find a nice way to do so. :)
     VisitForValue(property->value());
     Node* value = environment()->Pop();
     Node* key = environment()->Pop();
     Node* receiver = environment()->Pop();
-
-    AddHomeObjectIfNeeded(property->value(), value, receiver);
+    BuildSetHomeObject(value, receiver, property->value());
 
     switch (property->kind()) {
       case ObjectLiteral::Property::CONSTANT:
       case ObjectLiteral::Property::COMPUTED:
       case ObjectLiteral::Property::MATERIALIZED_LITERAL: {
         Node* attr = jsgraph()->Constant(NONE);
         const Operator* op =
             javascript()->CallRuntime(Runtime::kDefineDataPropertyUnchecked, 4);
         Node* call = NewNode(op, receiver, key, value, attr);
         PrepareFrameState(call, BailoutId::None());
@@ skipping 1196 matching lines @@
 Node* AstGraphBuilder::BuildToName(Node* input, BailoutId bailout_id) {
   // TODO(turbofan): Possible optimization is to NOP on name constants. But the
   // same caveat as with BuildToBoolean applies, and it should be factored out
   // into a JSOperatorReducer.
   Node* name = NewNode(javascript()->ToName(), input);
   PrepareFrameState(name, bailout_id);
   return name;
 }
 
 
+Node* AstGraphBuilder::BuildSetHomeObject(Node* value, Node* home_object,
+                                          Expression* expr) {
+  if (!FunctionLiteral::NeedsHomeObject(expr)) return value;
+  Unique<Name> name = MakeUnique(isolate()->factory()->home_object_symbol());
+  const Operator* op = javascript()->StoreNamed(language_mode(), name);
+  Node* store = NewNode(op, value, home_object);
+  PrepareFrameState(store, BailoutId::None());
+  return store;

[arv (Not doing code reviews), 2015/02/16 20:09:56]

What is the return value used for here?

[Michael Starzinger, 2015/02/16 20:30:35]

It's not used, but for symmetry all builder methods return what they are
building. There are similar builder methods (e.g. BuildVariableAssignment) where
this is the case as well.

The main purpose of the naming behind the "BuildFoo" methods is to clearly
distinguish them from the "VisitFoo" family of methods, which return values via
the environment (and hence mutate the environment).

+}
+
+
 Node* AstGraphBuilder::BuildThrowReferenceError(Variable* variable,
                                                 BailoutId bailout_id) {
   // TODO(mstarzinger): Should be unified with the VisitThrow implementation.
   Node* variable_name = jsgraph()->Constant(variable->name());
   const Operator* op =
       javascript()->CallRuntime(Runtime::kThrowReferenceError, 1);
   Node* call = NewNode(op, variable_name);
   PrepareFrameState(call, bailout_id);
   return call;
 }
@@ skipping 362 matching lines @@
     Node* dead_node = graph()->NewNode(common()->Dead());
     dead_control_.set(dead_node);
     return dead_node;
   }
   return dead_control_.get();
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8