Reviewers: Michael Starzinger,
Message:
This is a reapplication of https://codereview.chromium.org/1085263003.
Patchset
1 is the original patchset as reviewed, which caused a regression on
CodeLoad.
Patchset 2 fixes the regression by restoring lazy parsing of sloppy eval
inside
script scopes. I didn't realize that lazy parsing of eval code was
important --
I guess to allow nested functions to themselves be lazily parsed? Anyway,
PTAL
:-)
Description:
Eagerly declare eval scopes, even for sloppy scopes
[email protected]
LOG=N
BUG=N
Please review this at https://codereview.chromium.org/1110473003/
Base URL: https://chromium.googlesource.com/v8/v8@master
Affected files (+20, -37 lines):
M src/parser.h
M src/parser.cc
M src/runtime/runtime-scopes.cc
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
0ea398ae2e70af74dea66b0006680f984fc24c35..20ef55bb4e5ffd62fd6f0e25fd5bced42bf48285
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -961,6 +961,9 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo*
info) {
DCHECK(scope_ == NULL);
DCHECK(target_stack_ == NULL);
+ Mode parsing_mode = FLAG_lazy && allow_lazy() ? PARSE_LAZILY :
PARSE_EAGERLY;
+ if (allow_natives() || extension_ != NULL) parsing_mode = PARSE_EAGERLY;
+
FunctionLiteral* result = NULL;
{
Scope* scope = NewScope(scope_, SCRIPT_SCOPE);
@@ -978,22 +981,17 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo*
info) {
original_scope_ = scope;
if (info->is_eval()) {
if (!scope->is_script_scope() || is_strict(info->language_mode())) {
- scope = NewScope(scope, EVAL_SCOPE);
+ parsing_mode = PARSE_EAGERLY;
}
+ scope = NewScope(scope, EVAL_SCOPE);
} else if (info->is_module()) {
scope = NewScope(scope, MODULE_SCOPE);
}
scope->set_start_position(0);
- // Compute the parsing mode.
- Mode mode = (FLAG_lazy && allow_lazy()) ? PARSE_LAZILY : PARSE_EAGERLY;
- if (allow_natives() || extension_ != NULL || scope->is_eval_scope()) {
- mode = PARSE_EAGERLY;
- }
- ParsingModeScope parsing_mode(this, mode);
-
- // Enters 'scope'.
+ // Enter 'scope' with the given parsing mode..
+ ParsingModeScope parsing_mode_scope(this, parsing_mode);
AstNodeFactory function_factory(ast_value_factory());
FunctionState function_state(&function_state_, &scope_, scope,
kNormalFunction, &function_factory);
@@ -1006,10 +1004,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo*
info) {
DCHECK(allow_harmony_modules());
ParseModuleItemList(body, &ok);
} else {
- Scope* eval_scope = nullptr;
- ParseStatementList(body, Token::EOS, info->is_eval(), &eval_scope,
&ok);
- if (eval_scope != nullptr)
- eval_scope->set_end_position(scanner()->peek_location().beg_pos);
+ ParseStatementList(body, Token::EOS, &ok);
}
// The parser will peek but not consume EOS. Our scope logically goes
all
@@ -1195,7 +1190,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate,
ParseInfo* info,
void* Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
- bool is_eval, Scope** eval_scope, bool*
ok) {
+ bool* ok) {
// StatementList ::
// (StatementListItem)* <end_token>
@@ -1267,23 +1262,6 @@ void*
Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
// Strong mode implies strict mode. If there are several "use
strict"
// / "use strong" directives, do the strict mode changes only
once.
if (is_sloppy(scope_->language_mode())) {
- // TODO(mstarzinger): Global strict eval calls, need their own
scope
- // as specified in ES5 10.4.2(3). The correct fix would be to
always
- // add this scope in DoParseProgram(), but that requires
adaptations
- // all over the code base, so we go with a quick-fix for now.
- // In the same manner, we have to patch the parsing mode.
- if (is_eval && !scope_->is_eval_scope()) {
- DCHECK(scope_->is_script_scope());
- Scope* scope = NewScope(scope_, EVAL_SCOPE);
- scope->set_start_position(scope_->start_position());
- scope->set_end_position(scope_->end_position());
- scope_ = scope;
- if (eval_scope != NULL) {
- // Caller will correct the positions of the ad hoc eval
scope.
- *eval_scope = scope;
- }
- mode_ = PARSE_EAGERLY;
- }
scope_->SetLanguageMode(static_cast<LanguageMode>(
scope_->language_mode() | STRICT_BIT));
}
@@ -1292,6 +1270,7 @@ void*
Parser::ParseStatementList(ZoneList<Statement*>* body, int end_token,
scope_->SetLanguageMode(static_cast<LanguageMode>(
scope_->language_mode() | STRONG_BIT));
}
+ if (scope_->is_eval_scope()) mode_ = PARSE_EAGERLY;
} else if (literal->raw_value()->AsString() ==
ast_value_factory()->use_asm_string() &&
token_loc.end_pos - token_loc.beg_pos ==
@@ -4271,7 +4250,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
yield, RelocInfo::kNoPosition), zone());
}
- ParseStatementList(body, Token::RBRACE, false, NULL, CHECK_OK);
+ ParseStatementList(body, Token::RBRACE, CHECK_OK);
if (IsGeneratorFunction(kind)) {
VariableProxy* get_proxy = factory()->NewVariableProxy(
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
b4c628ebb297fa90d86594c2656423dada3dc298..58253e1bb6919309a444864f10bfff904efb837f
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -910,8 +910,7 @@ class Parser : public ParserBase<ParserTraits> {
// which is set to false if parsing failed; it is unchanged otherwise.
// By making the 'exception handling' explicit, we are forced to check
// for failure at the call sites.
- void* ParseStatementList(ZoneList<Statement*>* body, int end_token,
- bool is_eval, Scope** ad_hoc_eval_scope, bool*
ok);
+ void* ParseStatementList(ZoneList<Statement*>* body, int end_token,
bool* ok);
Statement* ParseStatementListItem(bool* ok);
void* ParseModuleItemList(ZoneList<Statement*>* body, bool* ok);
Statement* ParseModuleItem(bool* ok);
Index: src/runtime/runtime-scopes.cc
diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc
index
c92f027264ce3edc2974d269eb2dc8cd91f2b083..7a523a7cac41b3783be5f99f381f313d76cdb3fa
100644
--- a/src/runtime/runtime-scopes.cc
+++ b/src/runtime/runtime-scopes.cc
@@ -240,12 +240,17 @@ RUNTIME_FUNCTION(Runtime_DeclareLookupSlot) {
// TODO(verwaest): This case should probably not be covered by this
function,
// but by DeclareGlobals instead.
- if ((attributes != ABSENT && holder->IsJSGlobalObject()) ||
- (context_arg->has_extension() &&
- context_arg->extension()->IsJSGlobalObject())) {
+ if (attributes != ABSENT && holder->IsJSGlobalObject()) {
return DeclareGlobals(isolate, Handle<JSGlobalObject>::cast(holder),
name,
value, attr, is_var, is_const, is_function);
}
+ if (context_arg->has_extension() &&
+ context_arg->extension()->IsJSGlobalObject()) {
+ Handle<JSGlobalObject> global(
+ JSGlobalObject::cast(context_arg->extension()), isolate);
+ return DeclareGlobals(isolate, global, name, value, attr, is_var,
is_const,
+ is_function);
+ }
if (attributes != ABSENT) {
// The name was declared before; check for conflicting re-declarations.
--
--
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.
