Revision: 16325
Author:   [email protected]
Date:     Mon Aug 26 13:05:10 2013 UTC
Log:      Rollback of r16286 in trunk branch.

Fix scoping of function declarations in eval inside non-trivial local scope

BUG=v8:2594
[email protected]

Review URL: https://codereview.chromium.org/23004022
http://code.google.com/p/v8/source/detail?r=16325

Deleted:
 /trunk/test/mjsunit/regress/regress-2594.js
Modified:
 /trunk/src/parser.cc
 /trunk/src/parser.h
 /trunk/src/scopes.cc
 /trunk/src/version.cc

=======================================
--- /trunk/test/mjsunit/regress/regress-2594.js Fri Aug 23 13:28:00 2013 UTC
+++ /dev/null
@@ -1,104 +0,0 @@
-// Copyright 2013 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
-//       with the distribution.
-//     * Neither the name of Google Inc. nor the names of its
-//       contributors may be used to endorse or promote products derived
-//       from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// In the assertions but the first, the ES5 spec actually requires 0, but
-// that is arguably a spec bug, and other browsers return 1 like us.
-// In ES6, all of those will presumably result in a ReferenceError.
-// Our main concern with this test is that we do not crash, though.
-
-function f1() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    eval("var h = function() { return XXX }")
-  }
-  return h()
-}
-assertEquals(1, f1())
-
-function f2() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    eval("function h(){ return XXX }")
-  }
-  return h()
-}
-assertEquals(1, f2())
-
-function f3() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    try { throw 2 } catch (y) {
-      eval("function h(){ return XXX }")
-    }
-  }
-  return h()
-}
-assertEquals(1, f3())
-
-function f4() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    with ({}) {
-      eval("function h(){ return XXX }")
-    }
-  }
-  return h()
-}
-assertEquals(1, f4())
-
-function f5() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    eval('eval("function h(){ return XXX }")')
-  }
-  return h()
-}
-assertEquals(1, f5())
-
-function f6() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    eval("var h = (function() { function g(){ return XXX } return g })()")
-  }
-  return h()
-}
-assertEquals(1, f6())
-
-function f7() {
-  var XXX = 0
-  try { throw 1 } catch (XXX) {
-    eval("function h() { var XXX=2; function g(){ return XXX } return g }")
-  }
-  return h()()
-}
-assertEquals(2, f7())  // !
-
-var XXX = 0
-try { throw 1 } catch (XXX) {
-  eval("function h(){ return XXX }")
-}
-assertEquals(1, h())
=======================================
--- /trunk/src/parser.cc        Fri Aug 23 13:28:00 2013 UTC
+++ /trunk/src/parser.cc        Mon Aug 26 13:05:10 2013 UTC
@@ -542,7 +542,6 @@
       scanner_(isolate_->unicode_cache()),
       reusable_preparser_(NULL),
       top_scope_(NULL),
-      original_scope_(NULL),
       current_function_state_(NULL),
       target_stack_(NULL),
       extension_(info->extension()),
@@ -623,7 +622,6 @@
     if (!info->context().is_null()) {
scope = Scope::DeserializeScopeChain(*info->context(), scope, zone());
     }
-    original_scope_ = scope;
     if (info->is_eval()) {
if (!scope->is_global_scope() || info->language_mode() != CLASSIC_MODE) {
         scope = NewScope(scope, EVAL_SCOPE);
@@ -751,7 +749,6 @@
scope = Scope::DeserializeScopeChain(info()->closure()->context(), scope,
                                            zone());
     }
-    original_scope_ = scope;
     FunctionState function_state(this, scope, isolate());
ASSERT(scope->language_mode() != STRICT_MODE | | !info()->is_classic_mode());
     ASSERT(scope->language_mode() != EXTENDED_MODE ||
@@ -4282,38 +4279,10 @@
   // Function declarations are function scoped in normal mode, so they are
   // hoisted. In harmony block scoping mode they are block scoped, so they
   // are not hoisted.
-  //
-  // One tricky case are function declarations in a local sloppy-mode eval:
- // their declaration is hoisted, but they still see the local scope. E.g.,
-  //
-  // function() {
-  //   var x = 0
-  //   try { throw 1 } catch (x) { eval("function g() { return x }") }
-  //   return g()
-  // }
-  //
-  // needs to return 1. To distinguish such cases, we need to detect
-  // (1) whether a function stems from a sloppy eval, and
-  // (2) whether it actually hoists across the eval.
- // Unfortunately, we do not represent sloppy eval scopes, so we do not have - // either information available directly, especially not when lazily compiling
-  // a function like 'g'. We hence rely on the following invariants:
- // - (1) is the case iff the innermost scope of the deserialized scope chain - // under which we compile is _not_ a declaration scope. This holds because
-  //   in all normal cases, function declarations are fully hoisted to a
-  //   declaration scope and compiled relative to that.
- // - (2) is the case iff the current declaration scope is still the original
-  //   one relative to the deserialized scope chain. Otherwise we must be
- // compiling a function in an inner declaration scope in the eval, e.g. a
-  //   nested function, and hoisting works normally relative to that.
-  Scope* declaration_scope = top_scope_->DeclarationScope();
-  Scope* original_declaration_scope = original_scope_->DeclarationScope();
   Scope* scope =
- function_type == FunctionLiteral::DECLARATION && !is_extended_mode() &&
-      (original_scope_ == original_declaration_scope ||
-       declaration_scope != original_declaration_scope)
-          ? NewScope(declaration_scope, FUNCTION_SCOPE)
-          : NewScope(top_scope_, FUNCTION_SCOPE);
+ (function_type == FunctionLiteral::DECLARATION && !is_extended_mode())
+      ? NewScope(top_scope_->DeclarationScope(), FUNCTION_SCOPE)
+      : NewScope(top_scope_, FUNCTION_SCOPE);
   ZoneList<Statement*>* body = NULL;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
=======================================
--- /trunk/src/parser.h Fri Aug 23 13:28:00 2013 UTC
+++ /trunk/src/parser.h Mon Aug 26 13:05:10 2013 UTC
@@ -855,7 +855,6 @@
   Scanner scanner_;
   preparser::PreParser* reusable_preparser_;
   Scope* top_scope_;
-  Scope* original_scope_;  // for ES5 function declarations in sloppy eval
   FunctionState* current_function_state_;
   Target* target_stack_;  // for break, continue statements
   v8::Extension* extension_;
=======================================
--- /trunk/src/scopes.cc        Fri Aug 23 13:28:00 2013 UTC
+++ /trunk/src/scopes.cc        Mon Aug 26 13:05:10 2013 UTC
@@ -907,32 +907,26 @@
   PrintF("%d heap slots\n", num_heap_slots_); }

   // Print locals.
+  Indent(n1, "// function var\n");
   if (function_ != NULL) {
-    Indent(n1, "// function var:\n");
     PrintVar(n1, function_->proxy()->var());
   }

-  if (temps_.length() > 0) {
-    Indent(n1, "// temporary vars:\n");
-    for (int i = 0; i < temps_.length(); i++) {
-      PrintVar(n1, temps_[i]);
-    }
+  Indent(n1, "// temporary vars\n");
+  for (int i = 0; i < temps_.length(); i++) {
+    PrintVar(n1, temps_[i]);
   }

-  if (internals_.length() > 0) {
-    Indent(n1, "// internal vars:\n");
-    for (int i = 0; i < internals_.length(); i++) {
-      PrintVar(n1, internals_[i]);
-    }
+  Indent(n1, "// internal vars\n");
+  for (int i = 0; i < internals_.length(); i++) {
+    PrintVar(n1, internals_[i]);
   }

-  if (variables_.Start() != NULL) {
-    Indent(n1, "// local vars:\n");
-    PrintMap(n1, &variables_);
-  }
+  Indent(n1, "// local vars\n");
+  PrintMap(n1, &variables_);

+  Indent(n1, "// dynamic vars\n");
   if (dynamics_ != NULL) {
-    Indent(n1, "// dynamic vars:\n");
     PrintMap(n1, dynamics_->GetMap(DYNAMIC));
     PrintMap(n1, dynamics_->GetMap(DYNAMIC_LOCAL));
     PrintMap(n1, dynamics_->GetMap(DYNAMIC_GLOBAL));
=======================================
--- /trunk/src/version.cc       Fri Aug 23 13:53:01 2013 UTC
+++ /trunk/src/version.cc       Mon Aug 26 13:05:10 2013 UTC
@@ -35,7 +35,7 @@
 #define MAJOR_VERSION     3
 #define MINOR_VERSION     21
 #define BUILD_NUMBER      3
-#define PATCH_LEVEL       1
+#define PATCH_LEVEL       2
 // Use 1 for candidates and 0 otherwise.
 // (Boolean macro values are not supported by all preprocessors.)
 #define IS_CANDIDATE_VERSION 0

--
--
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/groups/opt_out.

Reply via email to