Always immediately propagate flags outwards rather than relying on PropagateScopeInfo

src/ast/scopes.h

 // 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.
 
 #ifndef V8_AST_SCOPES_H_
 #define V8_AST_SCOPES_H_
 
 #include "src/ast/ast.h"
 #include "src/base/hashmap.h"
 #include "src/globals.h"
@@ skipping 203 matching lines @@
   // the given list. This is used to catch patterns like
   // `try{}catch(e){let e;}`,
   // which is an error even though the two 'e's are declared in different
   // scopes.
   Declaration* CheckLexDeclarationsConflictingWith(
       const ZoneList<const AstRawString*>& names);
 
   // ---------------------------------------------------------------------------
   // Scope-specific info.
 
-  // Inform the scope that the corresponding code contains an eval call.
-  void RecordEvalCall() { scope_calls_eval_ = true; }
+  // Inform the scope and outer scopes that the corresponding code contains an
+  // eval call.
+  void RecordEvalCall() {
+    scope_calls_eval_ = true;
+    for (Scope* scope = this; scope != nullptr; scope = scope->outer_scope()) {
+      scope->inner_scope_calls_eval_ = true;
+    }
+  }
 

[neis, 2016/08/22 13:21:47]

Do I understand correctly that you are setting inner_scope_calls_eval_ for the
original scope too in order to shorten some conditionals elsewhere?

[Toon Verwaest, 2016/08/22 14:13:28]

Yes. All uses of inner_scope_calls_eval_ always did inner_scope_calls_eval_ ||
scope_calls_eval_

   // Inform the scope that the corresponding code uses "super".
   void RecordSuperPropertyUsage() { scope_uses_super_property_ = true; }
 
   // Set the language mode flag (unless disabled by a global flag).
   void SetLanguageMode(LanguageMode language_mode) {
     DCHECK(!is_module_scope() || is_strict(language_mode));
     set_language_mode(language_mode);
   }
 
   // Inform the scope that the scope may execute declarations nonlinearly.
@@ skipping 123 matching lines @@
 
   // Result of variable allocation.
   int num_stack_slots() const { return num_stack_slots_; }
   int num_heap_slots() const { return num_heap_slots_; }
   int num_global_slots() const { return num_global_slots_; }
 
   int StackLocalCount() const;
   int ContextLocalCount() const;
   int ContextGlobalCount() const;
 
-  // Make sure this scope and all outer scopes are eagerly compiled.
-  void ForceEagerCompilation()  { force_eager_compilation_ = true; }
-
   // Determine if we can parse a function literal in this scope lazily.
   bool AllowsLazyParsing() const;
 
-  // Determine if we can use lazy compilation for this scope.
-  bool AllowsLazyCompilation() const;
-
-  // Determine if we can use lazy compilation for this scope without a context.
-  bool AllowsLazyCompilationWithoutContext() const;
-
   // The number of contexts between this and scope; zero if this == scope.
   int ContextChainLength(Scope* scope) const;
 
   // The number of contexts between this and the outermost context that has a
   // sloppy eval call. One if this->calls_sloppy_eval().
   int ContextChainLengthUntilOutermostSloppyEval() const;
 
   // The maximum number of nested contexts required for this scope and any inner
   // scopes.
   int MaxNestedContextChainLength();
@@ skipping 113 matching lines @@
   // This scope uses "super" property ('super.foo').
   bool scope_uses_super_property_ : 1;
   // This scope has a parameter called "arguments".
   bool has_arguments_parameter_ : 1;
   // This scope's declarations might not be executed in order (e.g., switch).
   bool scope_nonlinear_ : 1;
   bool is_hidden_ : 1;
   // Temporary workaround that allows masking of 'this' in debug-evalute scopes.
   bool is_debug_evaluate_scope_ : 1;
 
-  // Computed via PropagateScopeInfo.
   bool inner_scope_calls_eval_ : 1;
-  bool force_eager_compilation_ : 1;
   bool force_context_allocation_ : 1;
 
   // True if it holds 'var' declarations.
   bool is_declaration_scope_ : 1;
 
   // Create a non-local variable with a given name.
   // These variables are looked up dynamically at runtime.
   Variable* NonLocal(const AstRawString* name, VariableMode mode);
 
   // Variable resolution.
@@ skipping 129 matching lines @@
   }
 
   FunctionKind function_kind() const { return function_kind_; }
 
   bool is_arrow_scope() const {
     return is_function_scope() && IsArrowFunction(function_kind_);
   }
 
   bool NeedsHomeObject() const {
     return scope_uses_super_property_ ||
-           ((scope_calls_eval_ || inner_scope_calls_eval_) &&
-            (IsConciseMethod(function_kind()) ||
-             IsAccessorFunction(function_kind()) ||
-             IsClassConstructor(function_kind())));
+           (inner_scope_calls_eval_ && (IsConciseMethod(function_kind()) ||
+                                        IsAccessorFunction(function_kind()) ||
+                                        IsClassConstructor(function_kind())));
   }
 
   bool asm_module() const { return asm_module_; }
   void set_asm_module() { asm_module_ = true; }
   bool asm_function() const { return asm_function_; }
   void set_asm_function() { asm_module_ = true; }
 
   void DeclareThis(AstValueFactory* ast_value_factory);
   void DeclareDefaultFunctionVariables(AstValueFactory* ast_value_factory);
 
@@ skipping 146 matching lines @@
   // discard the Scope for lazily compiled functions. In particular, this
   // records variables which cannot be resolved inside the Scope (we don't yet
   // know what they will resolve to since the outer Scopes are incomplete) and
   // migrates them into migrate_to.
   void AnalyzePartially(DeclarationScope* migrate_to,
                         AstNodeFactory* ast_node_factory);
 
   Handle<StringSet> CollectNonLocals(ParseInfo* info,
                                      Handle<StringSet> non_locals);
 
+  // Determine if we can use lazy compilation for this scope.
+  bool AllowsLazyCompilation() const;
+
+  // Determine if we can use lazy compilation for this scope without a context.
+  bool AllowsLazyCompilationWithoutContext() const;
+
+  // Make this closure and all outer closures are eagerly compiled.

[neis, 2016/08/22 13:21:47]

There's a word missing here.

[Toon Verwaest, 2016/08/22 14:13:28]

Done.

+  void ForceEagerCompilation() {

[neis, 2016/08/22 13:21:47]

Can you add something like DCHECK(IsClosureScope) here? Or move the
GetClosureScope into the beginning of the loop.

[Toon Verwaest, 2016/08/22 14:13:29]

Done.

+    for (DeclarationScope* s = this; !s->is_script_scope();
+         s = s->outer_scope()->GetClosureScope()) {
+      force_eager_compilation_ = true;

[neis, 2016/08/22 13:21:47]

s->force_eager_compilation_

Should this have caused test failures?

+    }
+  }
+
 #ifdef DEBUG
   void PrintParameters();
 #endif
 
   void AllocateLocals();
   void AllocateParameterLocals();
   void AllocateReceiver();
 
  private:
   void AllocateParameter(Variable* var, int index);
 
   void SetDefaults();
 
   // If the scope is a function scope, this is the function kind.
   const FunctionKind function_kind_;
 
   bool has_simple_parameters_ : 1;
   // This scope contains an "use asm" annotation.
   bool asm_module_ : 1;
   // This scope's outer context is an asm module.
   bool asm_function_ : 1;
+  bool force_eager_compilation_ : 1;
 
   // Info about the parameter list of a function.
   int arity_;
   int rest_index_;
   Variable* rest_parameter_;
   // Compiler-allocated (user-invisible) temporaries.
   ZoneList<Variable*> temps_;
   // Parameter list in source order.
   ZoneList<Variable*> params_;
   // Map of function names to lists of functions defined in sloppy blocks
@@ skipping 25 matching lines @@
   void AllocateModuleVariables();
 
  private:
   ModuleDescriptor* module_descriptor_;
 };
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_AST_SCOPES_H_