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

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 6f51d82e643d3e94625bc0505255f173c05efdac..8a217adec3d416a609627fbcf38a9d5c3b32338b 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -2259,7 +2259,16 @@ Statement* Parser::ParseFunctionDeclaration(
       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;
 }
 
 
@@ -4277,6 +4286,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
       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);
     }
@@ -4939,6 +4951,41 @@ void Parser::CheckConflictingVarDeclarations(Scope* scope, bool* ok) {
 }
 
 
+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