Classes: Implement correct name binding

src/full-codegen.cc

Index: src/full-codegen.cc
diff --git a/src/full-codegen.cc b/src/full-codegen.cc
index e399a3496c342f536e92fc0d4aee84a5d83270a2..79771dbcc4d937c3f60998702f9124e1664369b0 100644
--- a/src/full-codegen.cc
+++ b/src/full-codegen.cc
@@ -1070,41 +1070,12 @@ void FullCodeGenerator::VisitBlock(Block* stmt) {
   NestedBlock nested_block(this, stmt);
   SetStatementPosition(stmt);
 
-  Scope* saved_scope = scope();
-  // Push a block context when entering a block with block scoped variables.
-  if (stmt->scope() == NULL) {
-    PrepareForBailoutForId(stmt->EntryId(), NO_REGISTERS);
-  } else {
-    scope_ = stmt->scope();
-    DCHECK(!scope_->is_module_scope());
-    { Comment cmnt(masm_, "[ Extend block context");
-      __ Push(scope_->GetScopeInfo());
-      PushFunctionArgumentForContextAllocation();
-      __ CallRuntime(Runtime::kPushBlockContext, 2);
-
-      // Replace the context stored in the frame.
-      StoreToFrameField(StandardFrameConstants::kContextOffset,
-                        context_register());
-      PrepareForBailoutForId(stmt->EntryId(), NO_REGISTERS);
-    }
-    { Comment cmnt(masm_, "[ Declarations");
-      VisitDeclarations(scope_->declarations());
-      PrepareForBailoutForId(stmt->DeclsId(), NO_REGISTERS);
-    }
-  }
-
-  VisitStatements(stmt->statements());
-  scope_ = saved_scope;
-  __ bind(nested_block.break_label());
-
-  // Pop block context if necessary.
-  if (stmt->scope() != NULL) {
-    LoadContextField(context_register(), Context::PREVIOUS_INDEX);
-    // Update local stack frame context field.
-    StoreToFrameField(StandardFrameConstants::kContextOffset,
-                      context_register());
+  {
+    EnterBlockScopeIfNeeded block_scope_state(
+        this, stmt->scope(), stmt->EntryId(), stmt->DeclsId(), stmt->ExitId());
+    VisitStatements(stmt->statements());
+    __ bind(nested_block.break_label());
   }
-  PrepareForBailoutForId(stmt->ExitId(), NO_REGISTERS);
 }
 
 
@@ -1605,26 +1576,38 @@ void FullCodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
 void FullCodeGenerator::VisitClassLiteral(ClassLiteral* lit) {
   Comment cmnt(masm_, "[ ClassLiteral");
 
-  if (lit->raw_name() != NULL) {
-    __ Push(lit->name());
-  } else {
-    __ Push(isolate()->factory()->undefined_value());
-  }
+  {
+    EnterBlockScopeIfNeeded block_scope_state(
+        this, lit->scope(), BailoutId::None(), BailoutId::None(),

[Dmitry Lomov (no reviews), 2014/11/13 21:10:37]

Nice!

+        BailoutId::None());
 
-  if (lit->extends() != NULL) {
-    VisitForStackValue(lit->extends());
-  } else {
-    __ Push(isolate()->factory()->the_hole_value());
-  }
+    if (lit->raw_name() != NULL) {
+      __ Push(lit->name());
+    } else {
+      __ Push(isolate()->factory()->undefined_value());
+    }
 
-  VisitForStackValue(lit->constructor());
+    if (lit->extends() != NULL) {
+      VisitForStackValue(lit->extends());
+    } else {
+      __ Push(isolate()->factory()->the_hole_value());
+    }
+
+    VisitForStackValue(lit->constructor());
 
-  __ Push(script());
-  __ Push(Smi::FromInt(lit->start_position()));
-  __ Push(Smi::FromInt(lit->end_position()));
+    __ Push(script());
+    __ Push(Smi::FromInt(lit->start_position()));
+    __ Push(Smi::FromInt(lit->end_position()));
 
-  __ CallRuntime(Runtime::kDefineClass, 6);
-  EmitClassDefineProperties(lit);
+    __ CallRuntime(Runtime::kDefineClass, 6);
+    EmitClassDefineProperties(lit);
+
+    if (lit->scope() != NULL) {
+      DCHECK_NOT_NULL(lit->class_variable_proxy());
+      EmitVariableAssignment(lit->class_variable_proxy()->var(),
+                             Token::INIT_CONST);
+    }
+  }
 
   context()->Plug(result_register());
 }
@@ -1792,6 +1775,49 @@ bool BackEdgeTable::Verify(Isolate* isolate, Code* unoptimized) {
 #endif  // DEBUG
 
 
+FullCodeGenerator::EnterBlockScopeIfNeeded::EnterBlockScopeIfNeeded(
+    FullCodeGenerator* codegen, Scope* scope, BailoutId entry_id,
+    BailoutId declarations_id, BailoutId exit_id)
+    : codegen_(codegen), scope_(scope), exit_id_(exit_id) {
+  saved_scope_ = codegen_->scope();
+
+  if (scope == NULL) {
+    codegen_->PrepareForBailoutForId(entry_id, NO_REGISTERS);

[Dmitry Lomov (no reviews), 2014/11/13 21:10:37]

This should crash under --always-opt when entry_id == BailoutId::kNone. You will
have to protect it here specifically.

While you are here, maybe add a DCHECK in PrepareForBailoutForId that the id it
not None.

[arv (Not doing code reviews), 2014/11/13 21:56:29]

Since ClassLiteral is not optimized this is fine.

https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/full-codege...

+  } else {
+    codegen_->scope_ = scope;
+    {
+      Comment cmnt(masm(), "[ Extend block context");
+      __ Push(scope->GetScopeInfo());
+      codegen_->PushFunctionArgumentForContextAllocation();
+      __ CallRuntime(Runtime::kPushBlockContext, 2);
+
+      // Replace the context stored in the frame.
+      codegen_->StoreToFrameField(StandardFrameConstants::kContextOffset,
+                                  codegen_->context_register());
+      codegen_->PrepareForBailoutForId(entry_id, NO_REGISTERS);
+    }
+    {
+      Comment cmnt(masm(), "[ Declarations");
+      codegen_->VisitDeclarations(scope->declarations());
+      codegen_->PrepareForBailoutForId(declarations_id, NO_REGISTERS);
+    }
+  }
+}
+
+
+FullCodeGenerator::EnterBlockScopeIfNeeded::~EnterBlockScopeIfNeeded() {
+  if (scope_ != NULL) {
+    codegen_->LoadContextField(codegen_->context_register(),
+                               Context::PREVIOUS_INDEX);
+    // Update local stack frame context field.
+    codegen_->StoreToFrameField(StandardFrameConstants::kContextOffset,
+                                codegen_->context_register());
+  }
+  codegen_->PrepareForBailoutForId(exit_id_, NO_REGISTERS);
+  codegen_->scope_ = saved_scope_;
+}
+
+
 #undef __