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