On 2015/07/06 10:18:02, ulan wrote:
On 2015/07/06 10:05:08, Yang wrote:
> Replies inline.
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/arm/full-codegen-arm.cc
> File src/arm/full-codegen-arm.cc (right):
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/arm/full-codegen-arm.cc#newcode2130
> src/arm/full-codegen-arm.cc:2130: SetExpressionPosition(expr);
> On 2015/07/06 09:24:18, ulan wrote:
> > Why we don't insert break here?
>
> Because EmitReturnSequence emits a return, which is considered a
statement
and
> can be used to set a break point.
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/arm/full-codegen-arm.cc#newcode2832
> src/arm/full-codegen-arm.cc:2832: SetExpressionPosition(expr);
> On 2015/07/06 09:24:18, ulan wrote:
> > Why we don't insert break here?
>
> Actually we don't need this at all. The caller already emits a position.
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.cc
> File src/full-codegen.cc (right):
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.cc#newcode410
> src/full-codegen.cc:410: bool RecordPositions(MacroAssembler* masm, int
pos,
> bool statement = true) {
> On 2015/07/06 09:24:18, ulan wrote:
> > Please use enum instead of bool and avoid implicit.
>
> Done.
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.cc#newcode783
> src/full-codegen.cc:783: SetStatementPosition(stmt, SKIP_BREAK);
> On 2015/07/06 09:24:18, ulan wrote:
> > Please add a comment that the break is added below.
>
> Done.
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.cc#newcode850
> src/full-codegen.cc:850: SetStatementPosition(stmt, SKIP_BREAK);
> On 2015/07/06 09:24:18, ulan wrote:
> > Please add a comment that the break is added below (here and in other
> functions
> > too).
>
> Done.
>
> https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.h
> File src/full-codegen.h (right):
>
>
https://codereview.chromium.org/1218493005/diff/40001/src/full-codegen.h#newcode665
> src/full-codegen.h:665: void SetStatementPosition(Statement* stmt,
> On 2015/07/06 09:24:19, ulan wrote:
> > Can we make the insert_break explicit in SetStatementPosition and
> > SetExpressionPosition?
> >
> > Since they are using opposite defaults call-sites become confusing.
Also
> > SetReturnPosition and SetFunctionPosition do not record slots at all,
but
> > SetExpressionAsStatementPosition does.
> >
> > SetExpressionAsStatementPosition should be named
> > SetExpressionAsStatementPositionAndInsertBreak.
>
> The background here is that we generally break at statements, but not at
> expressions. Expression positions are mostly just used for stack
traces, not
> break points.
>
> That's why the default is that statement --> break, expression --> no
break.
Not
> having the default would mean that I would have to add a lot of
boilerplate
at
> all call sites except for a few exceptions.
>
> Can I simply add a comment here?
I see. Just comment would be ok.
Thanks. Is there anything left do to for me?
https://codereview.chromium.org/1218493005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
