[interpreter] statement position always overwrites earlier positions.

[interpreter] statement position always overwrites earlier positions.

If a statement or expression does not produce any bytecode, it's position
should always be overwritten by a following statement position.

R=mstarzinger@chromium.org, vogelheim@chromium.org
BUG=v8:4680
LOG=N

Committed: https://crrev.com/1354b1bf3701d41e0393325cf4e040b0b1c8540e
Cr-Commit-Position: refs/heads/master@{#35252}

[vogelheim, 2016-04-04 16:14:32]

lgtm, but please have another look at the test.

https://codereview.chromium.org/1854113002/diff/1/test/unittests/interpreter/...
File test/unittests/interpreter/source-position-table-unittest.cc (right):

https://codereview.chromium.org/1854113002/diff/1/test/unittests/interpreter/...
test/unittests/interpreter/source-position-table-unittest.cc:40:
builder.AddStatementPosition(offsets[i], offsets[i]);
Hmm. Does this still do anything useful?

Each pos is added twice, so this stresses the 2nd-statement-wins logic. But
since it adds the same positions each time, the assertions in
ToSourcePositionTable cannot distinguish between an implementation where the 1st
or the 2nd statement wins.

Maybe:
  builder.AddStatementPosition(offsets[i], 2*i);
  builder.AddStatementPosition(offsets[i], 2*i + 1):

That way, the resulting table will be different, depending on what
AddStatementPosition actually does.

[Michael Starzinger, 2016-04-04 17:16:45]

LGTM with one question.

https://codereview.chromium.org/1854113002/diff/1/src/interpreter/source-posi...
File src/interpreter/source-position-table.cc (right):

https://codereview.chromium.org/1854113002/diff/1/src/interpreter/source-posi...
src/interpreter/source-position-table.cc:128: LOG_CODE_EVENT(isolate_,
CodeLinePosInfoAddPositionEvent(
Isn't this event now covered by the events fired in CommitEntry as well?

[Yang, 2016-04-05 08:50:03]

Thanks for the reviews. I changed the unit test to make a bit more sense.

https://codereview.chromium.org/1854113002/diff/1/src/interpreter/source-posi...
File src/interpreter/source-position-table.cc (right):

https://codereview.chromium.org/1854113002/diff/1/src/interpreter/source-posi...
src/interpreter/source-position-table.cc:128: LOG_CODE_EVENT(isolate_,
CodeLinePosInfoAddPositionEvent(
On 2016/04/04 17:16:45, Michael Starzinger wrote:
> Isn't this event now covered by the events fired in CommitEntry as well?

Correct. I forgot to remove this one.

[Yang, 2016-04-05 08:50:08]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-04-05 08:50:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/1854113002/#ps20001
(title: "fix")

[commit-bot: I haz the power, 2016-04-05 08:50:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1854113002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1854113002/20001

[commit-bot: I haz the power, 2016-04-05 09:13:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-05 09:14:24]

Description was changed from

==========
[interpreter] statement position always overwrites earlier positions.

If a statement or expression does not produce any bytecode, it's position
should always be overwritten by a following statement position.

R=mstarzinger@chromium.org, vogelheim@chromium.org
BUG=v8:4680
LOG=N
==========

to

==========
[interpreter] statement position always overwrites earlier positions.

If a statement or expression does not produce any bytecode, it's position
should always be overwritten by a following statement position.

R=mstarzinger@chromium.org, vogelheim@chromium.org
BUG=v8:4680
LOG=N

Committed: https://crrev.com/1354b1bf3701d41e0393325cf4e040b0b1c8540e
Cr-Commit-Position: refs/heads/master@{#35252}
==========

[commit-bot: I haz the power, 2016-04-05 09:14:25]

Patchset 2 (id:??) landed as
https://crrev.com/1354b1bf3701d41e0393325cf4e040b0b1c8540e
Cr-Commit-Position: refs/heads/master@{#35252}