Record statement positions for the debugger in the fast code generator.

Record statement positions for the debugger in the fast code generator.



Committed: http://code.google.com/p/v8/source/detail?r=3075

[Kevin Millikin (Chromium), 2009-10-15 14:54:27]

LGTM, with some style issues.

http://codereview.chromium.org/271102/diff/1/10
File src/arm/codegen-arm.h (right):

http://codereview.chromium.org/271102/diff/1/10#newcode158
Line 158: 
Too much whitespace?

http://codereview.chromium.org/271102/diff/1/10#newcode170
Line 170: static void RecordPositions(MacroAssembler* masm, int pos);
There's a big comment in codegen.h listing all the functions that are required
to be in the platform-specific CodeGenerator classes (because they're needed by
clients).  You should probably add this function.

http://codereview.chromium.org/271102/diff/1/9
File src/arm/fast-codegen-arm.cc (right):

http://codereview.chromium.org/271102/diff/1/9#newcode54
Line 54: // ARM does NOT call CodeForFunctionPosition
Period at the end of the comment.  We should talk to Søren when he gets back
about what ARM *should* do.

http://codereview.chromium.org/271102/diff/1/2
File src/fast-codegen.h (right):

http://codereview.chromium.org/271102/diff/1/2#newcode52
Line 52: // These member function use CodeGenerator::recordPositions
No need for this comment about the implementation. It will just have to be kept
up to date whenever we change the implementation.

http://codereview.chromium.org/271102/diff/1/2#newcode54
Line 54: void CodeForFunctionPosition(FunctionLiteral* fun);
Since we're building a new code generator, it's probably a good time to give
these better names.  They don't emit code, so I think "SetFunctionPosition" is
better.

[fschneider, 2009-10-15 15:21:00]

On 2009/10/15 14:54:27, Kevin Millikin wrote:
> LGTM, with some style issues.
> 
> http://codereview.chromium.org/271102/diff/1/10
> File src/arm/codegen-arm.h (right):
> 
> http://codereview.chromium.org/271102/diff/1/10#newcode158
> Line 158: 
> Too much whitespace?
>
DONE.
 
> http://codereview.chromium.org/271102/diff/1/10#newcode170
> Line 170: static void RecordPositions(MacroAssembler* masm, int pos);
> There's a big comment in codegen.h listing all the functions that are required
> to be in the platform-specific CodeGenerator classes (because they're needed
by
> clients).  You should probably add this function.
>

DONE.
 
> http://codereview.chromium.org/271102/diff/1/9
> File src/arm/fast-codegen-arm.cc (right):
> 
> http://codereview.chromium.org/271102/diff/1/9#newcode54
> Line 54: // ARM does NOT call CodeForFunctionPosition
> Period at the end of the comment.  We should talk to Søren when he gets back
> about what ARM *should* do.
> 

DONE.
Yes, we should confirm this. For now I just did the same thing as the normal
compiler.

> http://codereview.chromium.org/271102/diff/1/2
> File src/fast-codegen.h (right):
> 
> http://codereview.chromium.org/271102/diff/1/2#newcode52
> Line 52: // These member function use CodeGenerator::recordPositions
> No need for this comment about the implementation. It will just have to be
kept
> up to date whenever we change the implementation.
> 

DONE.

> http://codereview.chromium.org/271102/diff/1/2#newcode54
> Line 54: void CodeForFunctionPosition(FunctionLiteral* fun);
> Since we're building a new code generator, it's probably a good time to give
> these better names.  They don't emit code, so I think "SetFunctionPosition" is
> better.

DONE.