[parser] Skipping inner funcs: produce the same scopes / variables for (some) loops.

test/cctest/test-parsing.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 8817 matching lines @@
 
 namespace v8 {
 namespace internal {
 
 class ScopeTestHelper {
  public:
   static bool MustAllocateInContext(Variable* var) {
     return var->scope()->MustAllocateInContext(var);
   }
 
+  // True if the scope is hidden and all scopes in its subscope tree are hidden

[vogelheim, 2017/02/02 18:28:56]

super nitpick:
 // True if this scope and its entire subscope tree are hidden.

Fits in one line. :)

[marja, 2017/02/03 06:48:58]

Done.

+  // too.
+  static bool IsHiddenLeafScope(Scope* scope) {

[vogelheim, 2017/02/02 18:28:56]

naming nitpick: I'm not super happy with the name, since the scope for which
this is called isn't really a leaf scope. Maybe: AllInnerScopesAreHidden? Or
ScopeTreeIsHidden? Or ContainsVisibleScope (then with reversed return value)?

[marja, 2017/02/03 06:48:58]

Done.

+    if (!scope->is_hidden()) {
+      return false;
+    }
+    for (Scope* inner = scope->inner_scope(); inner != nullptr;
+         inner = inner->sibling()) {
+      if (!IsHiddenLeafScope(inner)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data,
                                  size_t& index) {
     CHECK_EQ(data->backing_store_[index++], scope->scope_type());
     CHECK_EQ(data->backing_store_[index++], scope->start_position());
     CHECK_EQ(data->backing_store_[index++], scope->end_position());
 
     int inner_scope_count = 0;
     for (Scope* inner = scope->inner_scope(); inner != nullptr;
          inner = inner->sibling()) {
-      if (!inner->is_hidden()) {
+      // FIXME(marja): This is probably not the right condition for knowing what
+      // scopes are present in the preparse data.
+      if (!IsHiddenLeafScope(inner)) {
         ++inner_scope_count;
       }
     }
     CHECK_EQ(data->backing_store_[index++], inner_scope_count);
 
     int variable_count = 0;
     for (Variable* local : scope->locals_) {
       if (local->mode() == VAR || local->mode() == LET ||
           local->mode() == CONST) {
         ++variable_count;
@@ skipping 13 matching lines @@
           CHECK_EQ(data->backing_store_[index++], local_name->raw_data()[i]);
         }
 #endif
         CHECK_EQ(data->backing_store_[index++], local->location());
         CHECK_EQ(data->backing_store_[index++], local->maybe_assigned());
       }
     }
 
     for (Scope* inner = scope->inner_scope(); inner != nullptr;
          inner = inner->sibling()) {
-      if (!inner->is_hidden()) {
+      if (!IsHiddenLeafScope(inner)) {
         CompareScopeToData(inner, data, index);
       }
     }
   }
 };
 }  // namespace internal
 }  // namespace v8
 
 // Test that lazily parsed inner functions don't result in overly pessimistic
 // context allocations.
@@ skipping 412 matching lines @@
       {"", "function f1() { f1; } f1 = 3;"},
       {"", "function arguments() {} arguments = 8"},
       {"", "function f1() {} f1 = 3; function f1() {}"},
 
       {"", "var var1; eval('');"},
       {"", "var var1; function f1() { eval(''); }"},
       {"", "let var1; eval('');"},
       {"", "let var1; function f1() { eval(''); }"},
       {"", "const var1 = 10; eval('');"},
       {"", "const var1 = 10; function f1() { eval(''); }"},
+
+      {"", "for (var var1 = 0; var1 < 10; ++var1) { }"},
+      {"", "for (let var1 = 0; var1 < 10; ++var1) { }"},
+      {"", "for (const var1 = 0; var1 < 10; ++var1) { }"},
+
+      // FIXME(marja): make the corresponding cases work when foo is a sloppy
+      // block function.
+      {"",
+       "'use strict'; for (var var1 = 0; var1 < 10; ++var1) { function foo() { "
+       "var1; } }"},
+      {"",
+       "'use strict'; for (let var1 = 0; var1 < 10; ++var1) { function foo() { "
+       "var1; } }"},
+      {"",
+       "'use strict'; for (const var1 = 0; var1 < 10; ++var1) { function foo() "
+       "{ var1; } }"},
   };
 
   for (unsigned i = 0; i < arraysize(inners); ++i) {
     // First compile with the lazy inner function and extract the scope data.
     const char* inner_function = lazy_inner;
     int inner_function_len = Utf8LengthHelper(inner_function) - 4;
 
     int params_len = Utf8LengthHelper(inners[i].params);
     int source_len = Utf8LengthHelper(inners[i].source);
     int len =
@@ skipping 49 matching lines @@
         eager_info.literal()->scope()->inner_scope()->inner_scope();
     DCHECK_NOT_NULL(scope);
     DCHECK_NULL(scope->sibling());
     DCHECK(scope->is_function_scope());
 
     size_t index = 0;
     i::ScopeTestHelper::CompareScopeToData(
         scope, lazy_info.preparsed_scope_data(), index);
   }
 }