Create multiple compilation jobs for ignition if compiling multiple literals

src/ast/collect-eager-literals.cc

Index: src/ast/collect-eager-literals.cc
diff --git a/src/ast/collect-eager-literals.cc b/src/ast/collect-eager-literals.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0b8a21e3dac81db4ef263f66f293e3190c7157c4
--- /dev/null
+++ b/src/ast/collect-eager-literals.cc
@@ -0,0 +1,43 @@
+// Copyright 2016 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/ast/collect-eager-literals.h"
+
+#include "src/ast/ast.h"
+#include "src/compilation-info.h"
+#include "src/parsing/parse-info.h"
+
+namespace v8 {
+namespace internal {
+
+CollectEagerLiterals::CollectEagerLiterals(CompilationInfo* info)
+    : AstTraversalVisitor(info->isolate(), info->parse_info()->literal()),
+      parse_info_(info->parse_info()) {}
+
+CollectEagerLiterals::~CollectEagerLiterals() {}
+
+void CollectEagerLiterals::VisitFunctionDeclaration(FunctionDeclaration* decl) {
+  i::Variable* variable = decl->proxy()->var();
+  DCHECK(variable->mode() == LET || variable->mode() == VAR);
+  if (variable->location() == VariableLocation::UNALLOCATED) {

[Michael Starzinger, 2016/10/17 13:16:03]

Why have this special handling here? Any FunctionDeclaration will evaluate the
function literal one way or the other. Wouldn't it be sufficient to just have
the default visitation function for function declarations and let everything
just go through VisitFunctionLiteral below?

[jochen (gone - plz use gerrit), 2016/10/17 13:19:10]

VisitFunctionLiteral doesn't visit the declarations of the function, so it's
possible that it won't see a function that is only declared in that scope, and
only used in a lazily parsed scope

[Michael Starzinger, 2016/10/17 13:37:48]

Not sure I fully understand your response,
AstTraversalVisitor::VisitFunctionLiteral _does_ visit scope->declarations()
AFAICT. Can you elaborate?

+    if (decl->fun()->ShouldEagerCompile()) {
+      eager_literals_.insert(decl->fun());
+    } else {
+      return;

[Michael Starzinger, 2016/10/17 13:16:03]

nit: Why return here? Shouldn't we always fall through to
AstTraversalVisitor::VisitFunctionDeclaration just to play it safe? If we ever
add sub-expressions of any kind to a FunctionLiteral it would stop working
otherwise.

[jochen (gone - plz use gerrit), 2016/10/17 13:19:10]

if a function doesn't eager compiler, it might not have a body(), however, the
AstTraversalVisitor blindly visits the body...

[Michael Starzinger, 2016/10/17 13:37:48]

Then this should go into VisitFunctionLiteral below and basically read "if
(fun->body() == nullptr) return" there IMHO. One could even argue that this is a
"bug" in AstTraversalVisitor in the first place and the handling should go
there. WDYT?

+    }
+  }
+  AstTraversalVisitor::VisitFunctionDeclaration(decl);
+}
+
+void CollectEagerLiterals::VisitFunctionLiteral(FunctionLiteral* fun) {
+  if (fun->ShouldEagerCompile()) {
+    if (fun != parse_info_->literal()) {
+      eager_literals_.insert(fun);
+    }
+    AstTraversalVisitor::VisitFunctionLiteral(fun);
+  }
+}
+
+}  // namespace internal
+}  // namespace v8