In the toplevel compiler, shift the responsibility of assigning a...

src/compiler.cc

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 28 matching lines @@
 #include "usage-analyzer.h"
 
 namespace v8 {
 namespace internal {
 
 
 class CodeGenSelector: public AstVisitor {
  public:
   enum CodeGenTag { NORMAL, FAST };
 
-  CodeGenSelector() : has_supported_syntax_(true) {}
+  CodeGenSelector()
+      : has_supported_syntax_(true),
+        location_(Location::Nowhere()) {

[fschneider, 2009/10/28 10:06:01]

I'm not sure: Should this match the default location for ast nodes(TEMP)?

[Kevin Millikin (Chromium), 2009/10/28 11:39:38]

Possibly.  I wanted it to be nowhere when not inside an expression (visiting a
statement or declaration).  Now that every AST node gets a location explicitly
assigned, it might make sense to introduce an INVALID or UNITIALIZED one as
expression default to catch failure to assign.

I would do that as a separate change.

+  }
 
   CodeGenTag Select(FunctionLiteral* fun);
 
  private:
   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)
 #undef DECLARE_VISIT
 
   bool has_supported_syntax_;
 
+  // The desired location of the currently visited expression.
+  Location location_;
+
   DISALLOW_COPY_AND_ASSIGN(CodeGenSelector);
 };
 
 
 static Handle<Code> MakeCode(FunctionLiteral* literal,
                              Handle<Script> script,
                              Handle<Context> context,
                              bool is_eval) {
   ASSERT(literal != NULL);
 
@@ skipping 418 matching lines @@
 
 
 void CodeGenSelector::VisitStatements(ZoneList<Statement*>* stmts) {
   for (int i = 0, len = stmts->length(); i < len; i++) {
     Visit(stmts->at(i));
     CHECK_BAILOUT;
   }
 }
 
 
+void CodeGenSelector::VisitAsEffect(Expression* expr) {
+  if (location_.is_nowhere()) {
+    Visit(expr);
+  } else {
+    Location saved = location_;
+    location_ = Location::Nowhere();
+    Visit(expr);
+    location_ = saved;
+  }
+}
+
+
+void CodeGenSelector::VisitAsValue(Expression* expr) {
+  if (location_.is_temporary()) {
+    Visit(expr);
+  } else {
+    Location saved = location_;
+    location_ = Location::Temporary();
+    Visit(expr);
+    location_ = saved;
+  }
+}
+
+
 void CodeGenSelector::VisitDeclaration(Declaration* decl) {
   Variable* var = decl->proxy()->var();
   if (!var->is_global() || var->mode() == Variable::CONST) {
     BAILOUT("Non-global declaration");
   }
 }
 
 
 void CodeGenSelector::VisitBlock(Block* stmt) {
   VisitStatements(stmt->statements());
 }
 
 
 void CodeGenSelector::VisitExpressionStatement(ExpressionStatement* stmt) {
-  Expression* expr = stmt->expression();
-  Visit(expr);
-  CHECK_BAILOUT;
-  expr->set_location(Location::Nowhere());
+  VisitAsEffect(stmt->expression());
 }
 
 
 void CodeGenSelector::VisitEmptyStatement(EmptyStatement* stmt) {
   // EmptyStatement is supported.
 }
 
 
 void CodeGenSelector::VisitIfStatement(IfStatement* stmt) {
   BAILOUT("IfStatement");
 }
 
 
 void CodeGenSelector::VisitContinueStatement(ContinueStatement* stmt) {
   BAILOUT("ContinueStatement");
 }
 
 
 void CodeGenSelector::VisitBreakStatement(BreakStatement* stmt) {
   BAILOUT("BreakStatement");
 }
 
 
 void CodeGenSelector::VisitReturnStatement(ReturnStatement* stmt) {
-  Visit(stmt->expression());
+  VisitAsValue(stmt->expression());
 }
 
 
 void CodeGenSelector::VisitWithEnterStatement(WithEnterStatement* stmt) {
   BAILOUT("WithEnterStatement");
 }
 
 
 void CodeGenSelector::VisitWithExitStatement(WithExitStatement* stmt) {
   BAILOUT("WithExitStatement");
@@ skipping 37 matching lines @@
 
 void CodeGenSelector::VisitDebuggerStatement(DebuggerStatement* stmt) {
   BAILOUT("DebuggerStatement");
 }
 
 
 void CodeGenSelector::VisitFunctionLiteral(FunctionLiteral* expr) {
   if (!expr->AllowsLazyCompilation()) {
     BAILOUT("FunctionLiteral does not allow lazy compilation");
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitFunctionBoilerplateLiteral(
     FunctionBoilerplateLiteral* expr) {
   BAILOUT("FunctionBoilerplateLiteral");
 }
 
 
 void CodeGenSelector::VisitConditional(Conditional* expr) {
   BAILOUT("Conditional");
 }
 
 
 void CodeGenSelector::VisitSlot(Slot* expr) {
-  Slot::Type type = expr->type();
-  if (type != Slot::PARAMETER && type != Slot::LOCAL) {
-    BAILOUT("non-parameter/non-local slot reference");
-  }
+  UNREACHABLE();
 }
 
 
 void CodeGenSelector::VisitVariableProxy(VariableProxy* expr) {
   Expression* rewrite = expr->var()->rewrite();
-  if (rewrite != NULL) Visit(rewrite);
+  // A rewrite of NULL indicates a global variable.
+  if (rewrite != NULL) {
+    Slot* slot = rewrite->AsSlot();
+    if (slot != NULL) {
+      Slot::Type type = slot->type();
+      if (type != Slot::PARAMETER && type != Slot::LOCAL) {
+        BAILOUT("non-parameter/non-local slot reference");
+      }

[William Hesse, 2009/10/28 10:28:54]

Control flow is tricky here.  Reversing sense of slot != NULL makes this an if,
else if that is easier to read.

[Kevin Millikin (Chromium), 2009/10/28 11:39:38]

I see what you mean.  I've changed it.

+    } else {
+      BAILOUT("non-global/non-slot variable reference");
+    }
+  }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitLiteral(Literal* expr) {
-  // Literals are supported.
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitRegExpLiteral(RegExpLiteral* expr) {
-  // RegexpLiterals are supported.
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitObjectLiteral(ObjectLiteral* expr) {
-  for (int i = 0; i < expr->properties()->length(); i++) {
-    ObjectLiteral::Property* property = expr->properties()->at(i);
-    Visit(property->key());
-    CHECK_BAILOUT;
-    Visit(property->value());
+  ZoneList<ObjectLiteral::Property*>* properties = expr->properties();
+
+  for (int i = 0, len = properties->length(); i < len; i++) {
+    ObjectLiteral::Property* property = properties->at(i);
+    if (property->IsCompileTimeValue()) continue;
+
+    switch (property->kind()) {
+      case ObjectLiteral::Property::CONSTANT:
+        UNREACHABLE();
+
+      // For (non-compile-time) materialized literals and computed
+      // properties with symbolic keys we will use an IC and therefore not
+      // generate code for the key.
+      case ObjectLiteral::Property::COMPUTED:  // Fall through.
+      case ObjectLiteral::Property::MATERIALIZED_LITERAL:
+        if (property->key()->handle()->IsSymbol()) {
+          break;
+        }
+        // Fall through.
+
+      // In all other cases we need the key's value on the stack
+      // for a runtime call.  (Relies on TEMP meaning STACK.)
+      case ObjectLiteral::Property::GETTER:  // Fall through.
+      case ObjectLiteral::Property::SETTER:  // Fall through.
+      case ObjectLiteral::Property::PROTOTYPE:
+        VisitAsValue(property->key());
+        CHECK_BAILOUT;
+        break;
+    }
+    VisitAsValue(property->value());
     CHECK_BAILOUT;
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitArrayLiteral(ArrayLiteral* expr) {
   ZoneList<Expression*>* subexprs = expr->values();
   for (int i = 0, len = subexprs->length(); i < len; i++) {
     Expression* subexpr = subexprs->at(i);
     if (subexpr->AsLiteral() != NULL) continue;
     if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue;
-    Visit(subexpr);
+    VisitAsValue(subexpr);
     CHECK_BAILOUT;
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitCatchExtensionObject(CatchExtensionObject* expr) {
   BAILOUT("CatchExtensionObject");
 }
 
 
 void CodeGenSelector::VisitAssignment(Assignment* expr) {
   // We support plain non-compound assignments to parameters and
   // non-context (stack-allocated) locals.
-  if (expr->starts_initialization_block()) BAILOUT("initialization block");
+  if (expr->starts_initialization_block() ||
+      expr->ends_initialization_block()) {
+    BAILOUT("initialization block start");
+  }
 
   Token::Value op = expr->op();
   if (op == Token::INIT_CONST) BAILOUT("initialize constant");
   if (op != Token::ASSIGN && op != Token::INIT_VAR) {
     BAILOUT("compound assignment");
   }
 
   Variable* var = expr->target()->AsVariableProxy()->AsVariable();
   if (var == NULL) BAILOUT("non-variable assignment");
 
   if (!var->is_global()) {
     ASSERT(var->slot() != NULL);
     Slot::Type type = var->slot()->type();
     if (type != Slot::PARAMETER && type != Slot::LOCAL) {
       BAILOUT("non-parameter/non-local slot assignment");
     }
   }
 
-  Visit(expr->value());
+  VisitAsValue(expr->value());
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitThrow(Throw* expr) {
   BAILOUT("Throw");
 }
 
 
 void CodeGenSelector::VisitProperty(Property* expr) {
   Visit(expr->obj());
   CHECK_BAILOUT;
   Visit(expr->key());

[fschneider, 2009/10/28 10:06:01]

Also set location here:

expr->set_location(location_);

[Kevin Millikin (Chromium), 2009/10/28 11:39:38]

You're right.  Svn update can't rewrite these :)

 }
 
 
 void CodeGenSelector::VisitCall(Call* expr) {
   Expression* fun = expr->expression();
   ZoneList<Expression*>* args = expr->arguments();
   Variable* var = fun->AsVariableProxy()->AsVariable();
 
   // Check for supported calls
   if (var != NULL && var->is_possibly_eval()) {
     BAILOUT("Call to a function named 'eval'");
   } else if (var != NULL && !var->is_this() && var->is_global()) {
     // ----------------------------------
     // JavaScript example: 'foo(1, 2, 3)'  // foo is global
     // ----------------------------------
   } else {
     BAILOUT("Call to a non-global function");
   }
-  // Check all arguments to the call
+  // Check all arguments to the call.  (Relies on TEMP meaning STACK.)
   for (int i = 0; i < args->length(); i++) {
-    Visit(args->at(i));
+    VisitAsValue(args->at(i));
     CHECK_BAILOUT;
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitCallNew(CallNew* expr) {
   Visit(expr->expression());
   CHECK_BAILOUT;
   ZoneList<Expression*>* args = expr->arguments();
   // Check all arguments to the call
   for (int i = 0; i < args->length(); i++) {
     Visit(args->at(i));
     CHECK_BAILOUT;
   }

[fschneider, 2009/10/28 10:06:01]

Also set location here: expr->set_location(location_);

 }
 
 
 void CodeGenSelector::VisitCallRuntime(CallRuntime* expr) {
   // In case of JS runtime function bail out.
-  if (expr->function() == NULL) BAILOUT("CallRuntime");
+  if (expr->function() == NULL) BAILOUT("call JS runtime function");
   // Check for inline runtime call
   if (expr->name()->Get(0) == '_' &&
       CodeGenerator::FindInlineRuntimeLUT(expr->name()) != NULL) {
-    BAILOUT("InlineRuntimeCall");
+    BAILOUT("inlined runtime call");
   }
+  // Check all arguments to the call.  (Relies on TEMP meaning STACK.)
   for (int i = 0; i < expr->arguments()->length(); i++) {
-    Visit(expr->arguments()->at(i));
+    VisitAsValue(expr->arguments()->at(i));
     CHECK_BAILOUT;
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitUnaryOperation(UnaryOperation* expr) {
   BAILOUT("UnaryOperation");
 }
 
 
 void CodeGenSelector::VisitCountOperation(CountOperation* expr) {
   BAILOUT("CountOperation");
 }
 
 
 void CodeGenSelector::VisitBinaryOperation(BinaryOperation* expr) {
   switch (expr->op()) {
     case Token::OR:
-      Visit(expr->left());
+      VisitAsValue(expr->left());
       CHECK_BAILOUT;
+      // The location for the right subexpression is the same as for the
+      // whole expression.
       Visit(expr->right());
       break;
 
     default:
       BAILOUT("Unsupported binary operation");
   }
+  expr->set_location(location_);
 }
 
 
 void CodeGenSelector::VisitCompareOperation(CompareOperation* expr) {
   BAILOUT("CompareOperation");
 }
 
 
 void CodeGenSelector::VisitThisFunction(ThisFunction* expr) {
   BAILOUT("ThisFunction");
 }
 
 #undef BAILOUT
 #undef CHECK_BAILOUT
 
 
 } }  // namespace v8::internal