Revision: 16286
Author: [email protected]
Date: Fri Aug 23 09:25:37 2013 UTC
Log: Fix scoping of function declarations in eval inside non-trivial
local scope
[email protected]
BUG=v8:2594
Review URL: https://codereview.chromium.org/22901010
http://code.google.com/p/v8/source/detail?r=16286
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-2594.js
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/scopes.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-2594.js Fri Aug 23
09:25:37 2013 UTC
@@ -0,0 +1,104 @@
+// 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())
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Aug 20 06:39:04 2013 UTC
+++ /branches/bleeding_edge/src/parser.cc Fri Aug 23 09:25:37 2013 UTC
@@ -542,6 +542,7 @@
scanner_(isolate_->unicode_cache()),
reusable_preparser_(NULL),
top_scope_(NULL),
+ original_scope_(NULL),
current_function_state_(NULL),
target_stack_(NULL),
extension_(info->extension()),
@@ -622,6 +623,7 @@
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);
@@ -749,6 +751,7 @@
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 ||
@@ -4279,10 +4282,38 @@
// 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())
- ? NewScope(top_scope_->DeclarationScope(), FUNCTION_SCOPE)
- : NewScope(top_scope_, FUNCTION_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);
ZoneList<Statement*>* body = NULL;
int materialized_literal_count = -1;
int expected_property_count = -1;
=======================================
--- /branches/bleeding_edge/src/parser.h Fri Jul 19 09:57:35 2013 UTC
+++ /branches/bleeding_edge/src/parser.h Fri Aug 23 09:25:37 2013 UTC
@@ -855,6 +855,7 @@
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_;
=======================================
--- /branches/bleeding_edge/src/scopes.cc Wed Jul 17 15:29:00 2013 UTC
+++ /branches/bleeding_edge/src/scopes.cc Fri Aug 23 09:25:37 2013 UTC
@@ -907,26 +907,32 @@
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());
}
- Indent(n1, "// temporary vars\n");
- for (int i = 0; i < temps_.length(); i++) {
- PrintVar(n1, temps_[i]);
+ if (temps_.length() > 0) {
+ Indent(n1, "// temporary vars:\n");
+ for (int i = 0; i < temps_.length(); i++) {
+ PrintVar(n1, temps_[i]);
+ }
}
- Indent(n1, "// internal vars\n");
- for (int i = 0; i < internals_.length(); i++) {
- PrintVar(n1, internals_[i]);
+ if (internals_.length() > 0) {
+ Indent(n1, "// internal vars:\n");
+ for (int i = 0; i < internals_.length(); i++) {
+ PrintVar(n1, internals_[i]);
+ }
}
- Indent(n1, "// local vars\n");
- PrintMap(n1, &variables_);
+ if (variables_.Start() != NULL) {
+ 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));
--
--
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.
