Implement sloppy-mode block-defined functions (Annex B 3.3)

src/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/parser.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 2241 matching lines @@
           ? CONST
           : (is_strict(language_mode()) || allow_harmony_sloppy_function()) &&
                     !scope_->is_declaration_scope()
                 ? LET
                 : VAR;
   VariableProxy* proxy = NewUnresolved(name, mode);
   Declaration* declaration =
       factory()->NewFunctionDeclaration(proxy, mode, fun, scope_, pos);
   Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
   if (names) names->Add(name, zone());
-  return factory()->NewEmptyStatement(RelocInfo::kNoPosition);
+  EmptyStatement* empty = factory()->NewEmptyStatement(RelocInfo::kNoPosition);
+  if (is_sloppy(language_mode()) && allow_harmony_sloppy_function() &&
+      !scope_->is_declaration_scope()) {
+    DelegateStatement* delegate =
+        factory()->NewDelegateStatement(RelocInfo::kNoPosition, empty, scope_);
+    scope_->DeclarationScope()->sloppy_block_function_map()->Declare(name,
+                                                                     delegate);
+    return delegate;
+  }
+  return empty;
 }
 
 
 Statement* Parser::ParseClassDeclaration(ZoneList<const AstRawString*>* names,
                                          bool* ok) {
   // ClassDeclaration ::
   //   'class' Identifier ('extends' LeftHandExpression)? '{' ClassBody '}'
   //
   // A ClassDeclaration
   //
@@ skipping 1997 matching lines @@
                       function_name_location, CHECK_OK);
     const bool allow_duplicate_parameters =
         is_sloppy(language_mode) && formals.is_simple && !IsConciseMethod(kind);
     ValidateFormalParameters(&formals_classifier, language_mode,
                              allow_duplicate_parameters, CHECK_OK);
 
     if (is_strict(language_mode)) {
       CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),
                               CHECK_OK);
     }
+    if (is_sloppy(language_mode) && allow_harmony_sloppy_function()) {
+      InsertSloppyBlockFunctionVarBindings(scope, CHECK_OK);
+    }
     if (is_strict(language_mode) || allow_harmony_sloppy()) {
       CheckConflictingVarDeclarations(scope, CHECK_OK);
     }
   }
 
   bool has_duplicate_parameters =
       !formals_classifier.is_valid_formal_parameter_list_without_duplicates();
   FunctionLiteral::ParameterFlag duplicate_parameters =
       has_duplicate_parameters ? FunctionLiteral::kHasDuplicateParameters
                                : FunctionLiteral::kNoDuplicateParameters;
@@ skipping 642 matching lines @@
     Scanner::Location location = position == RelocInfo::kNoPosition
         ? Scanner::Location::invalid()
         : Scanner::Location(position, position + 1);
     ParserTraits::ReportMessageAt(location, MessageTemplate::kVarRedeclaration,
                                   name);
     *ok = false;
   }
 }
 
 
+void Parser::InsertSloppyBlockFunctionVarBindings(Scope* scope, bool* ok) {

[adamk, 2015/09/11 15:42:56]

Can you add a DCHECK that the passed-in scope is a declaration scope?

Also, how does this interact with the case where a function has two var
declaration scopes (as it does for sloppy non-simple argument lists)?

[Dan Ehrenberg, 2015/09/17 17:44:10]

Done

+  // For each variable which is used as a function declaration in a sloppy
+  // block,
+  SloppyBlockFunctionMap* map = scope->sloppy_block_function_map();
+  for (ZoneHashMap::Entry* p = map->Start(); p != NULL; p = map->Next(p)) {

[adamk, 2015/09/11 15:42:56]

Nit: I think "nullptr" is preferred these days over "NULL"

[Dan Ehrenberg, 2015/09/17 17:44:10]

Done

+    AstRawString* name = reinterpret_cast<AstRawString*>(p->key);

[adamk, 2015/09/11 15:42:56]

Can this just be a static_cast? Also, please re-constify it:

static_cast<const AstRawString*>(p->key)

[Dan Ehrenberg, 2015/09/17 17:44:10]

Done

+    // If the variable wouldn't conflict with a lexical declaration,
+    Variable* var = scope->LookupLocal(name);
+    if (var == NULL || !IsLexicalVariableMode(var->mode())) {

[adamk, 2015/09/11 15:42:56]

s/NULL/nullptr/

[Dan Ehrenberg, 2015/09/17 17:44:10]

Done

+      // Declare a var-style binding for the function in the outer scope
+      VariableProxy* proxy = scope->NewUnresolved(factory(), name);
+      Declaration* declaration = factory()->NewVariableDeclaration(
+          proxy, VAR, scope, RelocInfo::kNoPosition);
+      Declare(declaration, DeclarationDescriptor::NORMAL, true, ok);

[adamk, 2015/09/11 15:42:56]

I'm a little bit confused: why do you use the passed-in scope above, but the
current scope_ here (since you don't pass the last argument)? Are |scope| and
|scope_| actually the same here?

[Dan Ehrenberg, 2015/09/17 17:44:10]

It happens to work because this is called when coming back out to the function
scope, but that's unclear, so I passed scope.

[adamk, 2015/09/17 18:44:36]

Thanks, this looks better. A later cleanup (not in this CL) should probably make
that argument non-optional and make Declare static, to avoid other member
accesses of scope_creeping in.

+      DCHECK(ok);  // Based on the preceding check, this should not fail
+      if (!ok) return;
+
+      // Write in assignments to var for each block-scoped function declaration
+      auto delegates =
+          reinterpret_cast<ZoneVector<DelegateStatement*>*>(p->value);

[adamk, 2015/09/11 15:42:56]

Same here, please static_cast instead of reinterpret_cast

[Dan Ehrenberg, 2015/09/17 17:44:09]

Done

+      for (auto stmt = delegates->begin(); stmt < delegates->end(); stmt++) {

[adamk, 2015/09/11 15:42:56]

You can use more C++11 here:

for (auto stmt : delegates)

That'll also make stmt actually a Statement*, instead of an iterator.

Also, "delegate" is probably a better name for the loop var, since you're
already using "statement" below.

[Dan Ehrenberg, 2015/09/17 17:44:09]

Done

+        // Read from the local lexical scope and write to the function scope
+        VariableProxy* to = scope->NewUnresolved(factory(), name);
+        VariableProxy* from = (*stmt)->scope()->NewUnresolved(factory(), name);
+        Expression* assignment = factory()->NewAssignment(
+            Token::ASSIGN, to, from, RelocInfo::kNoPosition);
+        Statement* statement = factory()->NewExpressionStatement(
+            assignment, RelocInfo::kNoPosition);
+        (*stmt)->set_statement(statement);
+      }
+    }
+  }
+}
+
+
 // ----------------------------------------------------------------------------
 // Parser support
 
 bool Parser::TargetStackContainsLabel(const AstRawString* label) {
   for (Target* t = target_stack_; t != NULL; t = t->previous()) {
     if (ContainsLabel(t->statement()->labels(), label)) return true;
   }
   return false;
 }
 
@@ skipping 1291 matching lines @@
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
 }
 }  // namespace internal
 }  // namespace v8