[v8-dev] [turbofan] Fix context chain extension for top-level code. (issue 1172013002 by [email protected])

Tue, 09 Jun 2015 06:12:07 -0700 

Reviewers: Benedikt Meurer,

Description:
[turbofan] Fix context chain extension for top-level code.

For top-level code the closure passed into context allocation methods
needs to be replaced with a sentined to canonicalize is to the empty
function object.

[email protected]
TEST=mjsunit/regress/regress-4169
BUG=v8:4169
LOG=N

Please review this at https://codereview.chromium.org/1172013002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+26, -11 lines):
  M src/compiler/ast-graph-builder.h
  M src/compiler/ast-graph-builder.cc
  A + test/mjsunit/regress/regress-4169.js


Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 7fddc4aafd549a0c00956dd7a891a14703333ee0..9bafb730a867182a385b501e0d98b0161dc15266 100644 
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -453,6 +453,21 @@ AstGraphBuilder::AstGraphBuilder(Zone* local_zone, CompilationInfo* info, 
 }


+Node* AstGraphBuilder::GetFunctionClosureForContext() {
+  Scope* declaration_scope = current_scope()->DeclarationScope();
+  if (declaration_scope->is_script_scope() ||
+      declaration_scope->is_module_scope()) {
+ // Contexts nested in the native context have a canonical empty function as 
+    // their closure, not the anonymous closure containing the global code.
+    // Pass a SMI sentinel and let the runtime look up the empty function.
+    return jsgraph()->SmiConstant(0);
+  } else {
+    DCHECK(declaration_scope->is_function_scope());
+    return GetFunctionClosure();
+  }
+}
+
+
 Node* AstGraphBuilder::GetFunctionClosure() {
   if (!function_closure_.is_set()) {
     const Operator* op = common()->Parameter(
@@ -1181,7 +1196,7 @@ void AstGraphBuilder::VisitWithStatement(WithStatement* stmt) { 
   VisitForValue(stmt->expression());
   Node* value = environment()->Pop();
   const Operator* op = javascript()->CreateWithContext();
-  Node* context = NewNode(op, value, GetFunctionClosure());
+  Node* context = NewNode(op, value, GetFunctionClosureForContext());
   PrepareFrameState(context, stmt->EntryId());
   ContextScope scope(this, stmt->scope(), context);
   Visit(stmt->statement());
@@ -1399,7 +1414,7 @@ void AstGraphBuilder::VisitTryCatchStatement(TryCatchStatement* stmt) { 
   Node* exception = try_control.GetExceptionNode();
   Unique<String> name = MakeUnique(stmt->variable()->name());
   const Operator* op = javascript()->CreateCatchContext(name);
-  Node* context = NewNode(op, exception, GetFunctionClosure());
+  Node* context = NewNode(op, exception, GetFunctionClosureForContext());
   PrepareFrameState(context, BailoutId::None());
   {
     ContextScope scope(this, stmt->scope(), context);
@@ -3054,12 +3069,12 @@ Node* AstGraphBuilder::BuildLocalFunctionContext(Node* context, 


 Node* AstGraphBuilder::BuildLocalScriptContext(Scope* scope) {
-  Node* closure = GetFunctionClosure();
+  DCHECK(scope->is_script_scope());

   // Allocate a new local context.
   const Operator* op = javascript()->CreateScriptContext();
   Node* scope_info = jsgraph()->Constant(scope->GetScopeInfo(isolate()));
-  Node* local_context = NewNode(op, closure, scope_info);
+  Node* local_context = NewNode(op, GetFunctionClosure(), scope_info);
   PrepareFrameState(local_context, BailoutId::FunctionEntry());

   return local_context;
@@ -3067,12 +3082,12 @@ Node* AstGraphBuilder::BuildLocalScriptContext(Scope* scope) { 


 Node* AstGraphBuilder::BuildLocalBlockContext(Scope* scope) {
-  Node* closure = GetFunctionClosure();
+  DCHECK(scope->is_block_scope());

   // Allocate a new local context.
   const Operator* op = javascript()->CreateBlockContext();
   Node* scope_info = jsgraph()->Constant(scope->GetScopeInfo(isolate()));
-  Node* local_context = NewNode(op, scope_info, closure);
+ Node* local_context = NewNode(op, scope_info, GetFunctionClosureForContext()); 

   return local_context;
 }
Index: src/compiler/ast-graph-builder.h
diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index 6d14c75d7801be922dfdcf2c71b440d9229eddf3..42fa53f6c7c9e055e9650b01b4db84ca7f68fb7c 100644 
--- a/src/compiler/ast-graph-builder.h
+++ b/src/compiler/ast-graph-builder.h
@@ -145,6 +145,7 @@ class AstGraphBuilder : public AstVisitor {
   void CreateFunctionContext(bool constant_context);

   // Get or create the node that represents the outer function closure.
+  Node* GetFunctionClosureForContext();
   Node* GetFunctionClosure();

   // Node creation helpers.
Index: test/mjsunit/regress/regress-4169.js
diff --git a/test/message/super-in-function.js b/test/mjsunit/regress/regress-4169.js 
similarity index 68%
copy from test/message/super-in-function.js
copy to test/mjsunit/regress/regress-4169.js
index edaa0e4eadc7befdb7ed28a3e06eaa827c4388f0..df2de039844fd748c9b83d61608cef5f8c09e181 100644 
--- a/test/message/super-in-function.js
+++ b/test/mjsunit/regress/regress-4169.js
@@ -1,10 +1,9 @@
 // Copyright 2015 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.
-//
-// Flags: --harmony-classes
-'use strict';

-function f() {
-  super.x();
+with ({}) {
+  eval("var x = 23");
+  assertEquals(23, x);
 }
+assertEquals(23, x);


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to