Eagerly declare eval scopes, even for sloppy scopes

R=marja@chromium.org, mstarzinger@chromium.org
LOG=N
BUG=N

Review URL: https://codereview.chromium.org/1085263003

Cr-Commit-Position: refs/heads/master@{#28027}

src/parser.cc

@@ -977,9 +977,7 @@ 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);
-      }
+      scope = NewScope(scope, EVAL_SCOPE);
     } else if (info->is_module()) {
       scope = NewScope(scope, MODULE_SCOPE);
     }
@@ -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));
           }
@@ -4244,7 +4222,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(

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);

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.