Add support for calculating a scopes maximum nested context chain.

src/interpreter/bytecode-generator.cc

Index: src/interpreter/bytecode-generator.cc
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
index 5f321201df4e0f9c76b6b853d446c96586e7d2a8..908674c93aef805d1779369fd2f0b56bbd591556 100644
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -152,11 +152,10 @@ Handle<BytecodeArray> BytecodeGenerator::MakeBytecode(CompilationInfo* info) {
 
   builder()->set_parameter_count(info->num_parameters_including_this());
   builder()->set_locals_count(scope()->num_stack_slots());
-  // TODO(rmcilroy): Set correct context count.
-  builder()->set_context_count(info->num_heap_slots() > 0 ? 1 : 0);
+  builder()->set_context_count(scope()->MaxNestedContextChainLength());

[adamk, 2015/10/15 07:38:44]

Is this exercised by any tests (I'm not familiar with the testing plan for the
interpreter)?

[rmcilroy, 2015/10/15 09:23:07]

This is exercised by test-bytecode-generator.cc::DeclareGlobals and
test-interpreter.cc::DeclareGlobals, but only for the inner function context
(just below) since we don't yet have support for any other nested contexts (this
change is required before we can add that support).

One thing I was trying to figure out - is there any way to trigger a normal
(non-with) block to require a nested context without calling eval (which we
don't support yet either)? If there is an easy way to do this which is supported
by the current interpreter functionality I could add a test for that to this CL.

[adamk, 2015/10/15 09:34:32]

Capturing a lexical variable from a block will force it to be heap allocated:

function f() {
  "use strict";
  let x = 1;
  {
    let y = 2;
    return function g() { return [x, y]; }
  }
}

in this case, g's context chain will include the block containing "y".

[rmcilroy, 2015/10/15 14:06:16]

Great, thanks for the pointer. It turns out this needs a bit more support than
the interpreter supported, but I've added the test to my followup CL at
https://codereview.chromium.org/1403943004/ (both in test-interpreter.cc and
test-bytecode-generator.cc).

 
   // Build function context only if there are context allocated variables.
-  if (info->num_heap_slots() > 0) {
+  if (scope()->NeedsContext()) {

[adamk, 2015/10/15 07:38:45]

I'd leave this as num_heap_slots() > 0 (see other comment).

[rmcilroy, 2015/10/15 09:23:07]

Commented on the other comment.

     // Push a new inner context scope for the function.
     VisitNewLocalFunctionContext();
     ContextScope top_context(this, true);