[parser] Skipping inner funcs: produce the same scopes / variables for sloppy block funcs.

src/ast/scopes.cc

Index: src/ast/scopes.cc
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 1324f43b04d3b6fe14878d9e503c20c5a1ba702b..983d4c9f465ab7809a646115292db0604de54dfb 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -66,8 +66,8 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope,
   return reinterpret_cast<Variable*>(p->value);
 }
 
-void VariableMap::DeclareName(Zone* zone, const AstRawString* name,
-                              VariableMode mode) {
+Variable* VariableMap::DeclareName(Zone* zone, const AstRawString* name,
+                                   VariableMode mode) {
   Entry* p =
       ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->hash(),
                                   ZoneAllocationPolicy(zone));
@@ -77,6 +77,7 @@ void VariableMap::DeclareName(Zone* zone, const AstRawString* name,
     p->value =
         mode == VAR ? kDummyPreParserVariable : kDummyPreParserLexicalVariable;
   }
+  return reinterpret_cast<Variable*>(p->value);
 }
 
 void VariableMap::Remove(Variable* var) {
@@ -508,7 +509,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
       }
     }
 
-    bool var_created = false;
+    Variable* created_variable = nullptr;
 
     // Write in assignments to var for each block-scoped function declaration
     auto delegates = static_cast<SloppyBlockFunctionMap::Delegate*>(p->value);
@@ -543,8 +544,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
       if (!should_hoist) continue;
 
       // Declare a var-style binding for the function in the outer scope
-      if (!var_created) {
-        var_created = true;
+      if (created_variable == nullptr) {
         if (factory) {
           VariableProxy* proxy =
               factory->NewVariableProxy(name, NORMAL_VARIABLE);
@@ -554,12 +554,12 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
           // allow_harmony_restrictive_generators and
           // sloppy_mode_block_scope_function_redefinition.
           bool ok = true;
-          DeclareVariable(declaration, VAR,
-                          Variable::DefaultInitializationFlag(VAR), false,
-                          nullptr, &ok);
+          created_variable = DeclareVariable(
+              declaration, VAR, Variable::DefaultInitializationFlag(VAR), false,
+              nullptr, &ok);
           CHECK(ok);  // Based on the preceding check, this should not fail
         } else {
-          DeclareVariableName(name, VAR);
+          created_variable = DeclareVariableName(name, VAR);
         }
       }
 
@@ -570,6 +570,9 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
         Statement* statement =
             factory->NewExpressionStatement(assignment, kNoSourcePosition);
         delegate->set_statement(statement);
+      } else if (created_variable != kDummyPreParserVariable &&
+                 created_variable != kDummyPreParserLexicalVariable) {
+        created_variable->set_maybe_assigned();

[marja, 2017/02/02 11:56:17]

The Variable passing is needed to be able to do this

[vogelheim, 2017/02/02 19:00:42]

I can't parse that sentence. Care to explain?

[marja, 2017/02/03 21:03:01]

I need to return Variable* and transmit it up until here so that I can do
set_maybe_assigned.

       }

[vogelheim, 2017/02/02 19:00:42]

I don't really understand what this whole block does... I have a hunch it might
be a bit easier to read if "if (factory).." becomes the outer condition. Not
sure, though.

[marja, 2017/02/03 21:03:01]

This creates the assignmentstatement which assigns the lazy sloppy block
function into a variable in the outer scope. The variable gets created only
once, but if there are multiple sloppy block functions with the same name, an
assignment is created for each of them.

And an assignment naturally sets maybe_assigned.

When preparsing, AssignmentStatements are not created, but I still need to set
the relevant variable as maybe_assigned.

You're right, this is clearer if I move if (factory) out; I also added some
DCHECKs to make it clear when we're parsing and when we're preparsing.

An additional complication is that we're only creating variables in the
preparser when FLAG_preparser_scope_analysis is on. Oh well. Another possibility
would be to pass the maybe-assignedness-boolean to DeclareVariableName but I
didn't want to make that decision yet... let's see how this develops.

     }
   }
@@ -1063,7 +1066,8 @@ Variable* Scope::DeclareVariable(
   return var;
 }
 
-void Scope::DeclareVariableName(const AstRawString* name, VariableMode mode) {
+Variable* Scope::DeclareVariableName(const AstRawString* name,
+                                     VariableMode mode) {
   DCHECK(IsDeclaredVariableMode(mode));
   DCHECK(!already_resolved_);
   DCHECK(GetDeclarationScope()->is_being_lazily_parsed());
@@ -1094,8 +1098,9 @@ void Scope::DeclareVariableName(const AstRawString* name, VariableMode mode) {
       var->set_maybe_assigned();
     }
     var->set_is_used();
+    return var;
   } else {
-    variables_.DeclareName(zone(), name, mode);
+    return variables_.DeclareName(zone(), name, mode);
   }
 }