Move the Location class into the AST Expression class as a member....

src/compiler.cc

Index: src/compiler.cc
===================================================================
--- src/compiler.cc	(revision 3174)
+++ src/compiler.cc	(working copy)
@@ -48,23 +48,24 @@
 
   CodeGenSelector()
       : has_supported_syntax_(true),
-        location_(Location::Uninitialized()) {
+        context_(Expression::kUninitialized) {
   }
 
   CodeGenTag Select(FunctionLiteral* fun);
 
  private:
+  // Visit an expression in a given expression context.
+  void ProcessExpression(Expression::Context context, Expression* expr) {

[William Hesse, 2009/10/29 15:15:25]

Could this be called VisitExpression?
I think the expression should be the first argument, and how you visit it should
be the second.

[Kevin Millikin (Chromium), 2009/10/29 16:36:10]

I will change the order of arguments, and I'm open to changing the name.

I don't want to call it "VisitExpression" because it's too easy to get it
confused with the other Visit<AST node type> functions, and it's subtly
different.

+    Expression::Context saved = context_;
+    context_ = context;
+    Visit(expr);
+    expr->set_context(context);
+    context_ = saved;
+  }
+
   void VisitDeclarations(ZoneList<Declaration*>* decls);
   void VisitStatements(ZoneList<Statement*>* stmts);
 
-  // Visit an expression in effect context with a desired location of
-  // nowhere.
-  void VisitAsEffect(Expression* expr);
-
-  // Visit an expression in value context with a desired location of
-  // temporary.
-  void VisitAsValue(Expression* expr);
-
   // AST node visit functions.
 #define DECLARE_VISIT(type) virtual void Visit##type(type* node);
   AST_NODE_LIST(DECLARE_VISIT)
@@ -72,8 +73,8 @@
 
   bool has_supported_syntax_;
 
-  // The desired location of the currently visited expression.
-  Location location_;
+  // The desired expression context of the currently visited expression.
+  Expression::Context context_;
 
   DISALLOW_COPY_AND_ASSIGN(CodeGenSelector);
 };
@@ -513,30 +514,6 @@
 }
 
 
-void CodeGenSelector::VisitAsEffect(Expression* expr) {
-  if (location_.is_effect()) {
-    Visit(expr);
-  } else {
-    Location saved = location_;
-    location_ = Location::Effect();
-    Visit(expr);
-    location_ = saved;
-  }
-}
-
-
-void CodeGenSelector::VisitAsValue(Expression* expr) {
-  if (location_.is_value()) {
-    Visit(expr);
-  } else {
-    Location saved = location_;
-    location_ = Location::Value();
-    Visit(expr);
-    location_ = saved;
-  }
-}
-
-
 void CodeGenSelector::VisitDeclaration(Declaration* decl) {
   Variable* var = decl->proxy()->var();
   if (!var->is_global() || var->mode() == Variable::CONST) {
@@ -551,7 +528,7 @@
 
 
 void CodeGenSelector::VisitExpressionStatement(ExpressionStatement* stmt) {
-  VisitAsEffect(stmt->expression());
+  ProcessExpression(Expression::kEffect, stmt->expression());
 }
 
 
@@ -576,7 +553,7 @@
 
 
 void CodeGenSelector::VisitReturnStatement(ReturnStatement* stmt) {
-  VisitAsValue(stmt->expression());
+  ProcessExpression(Expression::kValue, stmt->expression());
 }
 
 
@@ -634,7 +611,6 @@
   if (!expr->AllowsLazyCompilation()) {
     BAILOUT("FunctionLiteral does not allow lazy compilation");
   }
-  expr->set_location(location_);
 }
 
 
@@ -671,17 +647,16 @@
       BAILOUT("non-parameter/non-local slot reference");
     }
   }
-  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitLiteral(Literal* expr) {
-  expr->set_location(location_);
+  /* Nothing to do. */
 }
 
 
 void CodeGenSelector::VisitRegExpLiteral(RegExpLiteral* expr) {
-  expr->set_location(location_);
+  /* Nothing to do. */
 }
 
 
@@ -711,14 +686,13 @@
       case ObjectLiteral::Property::GETTER:  // Fall through.
       case ObjectLiteral::Property::SETTER:  // Fall through.
       case ObjectLiteral::Property::PROTOTYPE:
-        VisitAsValue(property->key());
+        ProcessExpression(Expression::kValue, property->key());
         CHECK_BAILOUT;
         break;
     }
-    VisitAsValue(property->value());
+    ProcessExpression(Expression::kValue, property->value());
     CHECK_BAILOUT;
   }
-  expr->set_location(location_);
 }
 
 
@@ -728,10 +702,9 @@
     Expression* subexpr = subexprs->at(i);
     if (subexpr->AsLiteral() != NULL) continue;
     if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue;
-    VisitAsValue(subexpr);
+    ProcessExpression(Expression::kValue, subexpr);
     CHECK_BAILOUT;
   }
-  expr->set_location(location_);
 }
 
 
@@ -765,8 +738,7 @@
     }
   }
 
-  VisitAsValue(expr->value());
-  expr->set_location(location_);
+  ProcessExpression(Expression::kValue, expr->value());
 }
 
 
@@ -776,10 +748,9 @@
 
 
 void CodeGenSelector::VisitProperty(Property* expr) {
-  VisitAsValue(expr->obj());
+  ProcessExpression(Expression::kValue, expr->obj());
   CHECK_BAILOUT;
-  VisitAsValue(expr->key());
-  expr->set_location(location_);
+  ProcessExpression(Expression::kValue, expr->key());
 }
 
 
@@ -800,23 +771,21 @@
   }
   // Check all arguments to the call.  (Relies on TEMP meaning STACK.)
   for (int i = 0; i < args->length(); i++) {
-    VisitAsValue(args->at(i));
+    ProcessExpression(Expression::kValue, args->at(i));
     CHECK_BAILOUT;
   }
-  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitCallNew(CallNew* expr) {
-  VisitAsValue(expr->expression());
+  ProcessExpression(Expression::kValue, expr->expression());
   CHECK_BAILOUT;
   ZoneList<Expression*>* args = expr->arguments();
   // Check all arguments to the call
   for (int i = 0; i < args->length(); i++) {
-    VisitAsValue(args->at(i));
+    ProcessExpression(Expression::kValue, args->at(i));
     CHECK_BAILOUT;
   }
-  expr->set_location(location_);
 }
 
 
@@ -830,10 +799,9 @@
   }
   // Check all arguments to the call.  (Relies on TEMP meaning STACK.)
   for (int i = 0; i < expr->arguments()->length(); i++) {
-    VisitAsValue(expr->arguments()->at(i));
+    ProcessExpression(Expression::kValue, expr->arguments()->at(i));
     CHECK_BAILOUT;
   }
-  expr->set_location(location_);
 }
 
 
@@ -850,17 +818,15 @@
 void CodeGenSelector::VisitBinaryOperation(BinaryOperation* expr) {
   switch (expr->op()) {
     case Token::COMMA:
-      VisitAsEffect(expr->left());
+      ProcessExpression(Expression::kEffect, expr->left());
       CHECK_BAILOUT;
-      Visit(expr->right());  // Location is the same as the parent location.
+      ProcessExpression(context_, expr->right());
       break;
 
     case Token::OR:
-      VisitAsValue(expr->left());
+      ProcessExpression(Expression::kValue, expr->left());
       CHECK_BAILOUT;
-      // The location for the right subexpression is the same as for the
-      // whole expression so we call Visit directly.
-      Visit(expr->right());
+      ProcessExpression(context_, expr->right());
       break;
 
     case Token::ADD:
@@ -874,15 +840,14 @@
     case Token::SHL:
     case Token::SHR:
     case Token::SAR:
-      VisitAsValue(expr->left());
+      ProcessExpression(Expression::kValue, expr->left());
       CHECK_BAILOUT;
-      VisitAsValue(expr->right());
+      ProcessExpression(Expression::kValue, expr->right());
       break;
 
     default:
       BAILOUT("Unsupported binary operation");
   }
-  expr->set_location(location_);
 }