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.
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.
