Properly recognize closures when setting breakpoints

Properly recognize closures when setting breakpoints

Local functions and function literals are unknown to the VM
until the enclosing function is compiled. This change introduces
a list of known closures per class, and a function to look up
the innermost closure at a given token index.

The debugger now checks whether there are newly discovered
closures when a function containing a pending breakpoint is
compiled. If necessary, the breakpoint is set to the inner
function.
Committed: https://code.google.com/p/dart/source/detail?r=5599

[regis, 2012-03-16 22:04:01]

LGTM with comments.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:944: if (func.IsImplicitClosureFunction()) {
I do not understand why you changed this test. The comment below applies to all
closures, not just implicit ones.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:948: lookup_function = func.parent_function();
ASSERT(!lookup_function.IsNull());?

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:953: // Check if the breakpoint is inside a closure or
local function
Indentation

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/parser.cc#oldcode...
runtime/vm/parser.cc:3977: ASSERT(current_class().is_finalized());
I would not remove this assert, but just change the comment:

Local functions are registered in the enclosing class, but ignored during class
finalization. The enclosing class has already been finalized.	
 3977   ASSERT(current_class().is_finalized());

[hausner, 2012-03-16 22:20:58]

Thank you.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:944: if (func.IsImplicitClosureFunction()) {
On 2012/03/16 22:04:01, regis wrote:
> I do not understand why you changed this test. The comment below applies to
all
> closures, not just implicit ones.
Updated the comment.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:948: lookup_function = func.parent_function();
On 2012/03/16 22:04:01, regis wrote:
> ASSERT(!lookup_function.IsNull());?

Done.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/debugger.cc#newco...
runtime/vm/debugger.cc:953: // Check if the breakpoint is inside a closure or
local function
On 2012/03/16 22:04:01, regis wrote:
> Indentation

Done.

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/parser.cc
File runtime/vm/parser.cc (left):

http://codereview.chromium.org/9716004/diff/5001/runtime/vm/parser.cc#oldcode...
runtime/vm/parser.cc:3977: ASSERT(current_class().is_finalized());
On 2012/03/16 22:04:01, regis wrote:
> I would not remove this assert, but just change the comment:
> 
> Local functions are registered in the enclosing class, but ignored during
class
> finalization. The enclosing class has already been finalized.	
>  3977   ASSERT(current_class().is_finalized());

Done.