Rollback of r16286 in trunk branch.
Fix scoping of function declarations in eval inside non-trivial local scope
BUG=v8:2594
R=jkummerow@chromium.org
Review URL: https://codereview.chromium.org/23004022
git-svn-id: http://v8.googlecode.com/svn/trunk@16325 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
diff --git a/src/parser.cc b/src/parser.cc
index ccab7ec..4947790 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -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;
diff --git a/src/parser.h b/src/parser.h
index 2f26b84..68a74b7 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -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_;
diff --git a/src/scopes.cc b/src/scopes.cc
index e38e903..e631332 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -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));
diff --git a/src/version.cc b/src/version.cc
index 1d28f6c..0f682f6 100644
--- a/src/version.cc
+++ b/src/version.cc
@@ -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
diff --git a/test/mjsunit/regress/regress-2594.js b/test/mjsunit/regress/regress-2594.js
deleted file mode 100644
index 60720cb..0000000
--- a/test/mjsunit/regress/regress-2594.js
+++ /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())