Fix setting breakpoint setting in await statements

Fix setting breakpoint setting in await statements

CL https://codereview.chromium.org/2626753002 introduced a “real” token
position for the synthetic code that re-throws an exception returned
by an await’ed expression. This interferes with setting a breakpoint
in a line that contains an await, since the synthetic code happens to
be at the lowest compiled code address and will thus be picked as the
breakpoint location.

This CL makes the re-throw a synthetic token position again, but
includes synthetic token positions in stack traces. This is an alternative
fix for bug #28325.

BUG=#28770
R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/408a64756e166b884840482194600169d39784fb

[hausner, 2017-03-02 01:10:46]

Description was changed from

==========
bill bailey Tinselworm
Fix setting breakpoint setting in await statements

CL https://codereview.chromium.org/2626753002 introduced a “real” token
position for the synthetic code that re-throws an exception returned
by an await’ed expression. This interferes with setting a breakpoint
in a line that contains an await, since the synthetic code happens to
be at the lowest compiled code address and will thus be picked as the
breakpoint location.

This CL makes the re-throw a synthetic token position again, but
includes synthetic token positions in stack traces. This is an alternative
fix for bug #28325.

BUG=#28770
==========

to

==========
Fix setting breakpoint setting in await statements

CL https://codereview.chromium.org/2626753002 introduced a “real” token
position for the synthetic code that re-throws an exception returned
by an await’ed expression. This interferes with setting a breakpoint
in a line that contains an await, since the synthetic code happens to
be at the lowest compiled code address and will thus be picked as the
breakpoint location.

This CL makes the re-throw a synthetic token position again, but
includes synthetic token positions in stack traces. This is an alternative
fix for bug #28325.

BUG=#28770
==========

[hausner, 2017-03-02 01:11:16]

hausner@google.com changed reviewers:
+ fschneider@google.com

[hausner, 2017-03-02 01:14:29]

hausner@google.com changed reviewers:
+ johnmccutchan@google.com

[hausner, 2017-03-02 01:15:02]

John may have an opinion on the changes to class TokenPosition.

[Florian Schneider, 2017-03-02 02:44:13]

Lgtm!

https://codereview.chromium.org/2727803002/diff/1/runtime/vm/token_position.h
File runtime/vm/token_position.h (right):

https://codereview.chromium.org/2727803002/diff/1/runtime/vm/token_position.h...
runtime/vm/token_position.h:137: bool IsSourcePosition() const { return IsReal()
|| IsSynthetic(); }
This seems better. Was this another bug that NoSource was considered a soource
position before?

[hausner, 2017-03-02 06:14:00]

Thanks

https://codereview.chromium.org/2727803002/diff/1/runtime/vm/token_position.h
File runtime/vm/token_position.h (right):

https://codereview.chromium.org/2727803002/diff/1/runtime/vm/token_position.h...
runtime/vm/token_position.h:137: bool IsSourcePosition() const { return IsReal()
|| IsSynthetic(); }
On 2017/03/02 02:44:13, Florian Schneider wrote:
> This seems better. Was this another bug that NoSource was considered a soource
> position before?

This function was only used in one place. It wasn't a bug per se, but I think
this makes more sense. I took the liberty to change it for this CL.

[hausner, 2017-03-02 20:21:47]

Description was changed from

==========
Fix setting breakpoint setting in await statements

CL https://codereview.chromium.org/2626753002 introduced a “real” token
position for the synthetic code that re-throws an exception returned
by an await’ed expression. This interferes with setting a breakpoint
in a line that contains an await, since the synthetic code happens to
be at the lowest compiled code address and will thus be picked as the
breakpoint location.

This CL makes the re-throw a synthetic token position again, but
includes synthetic token positions in stack traces. This is an alternative
fix for bug #28325.

BUG=#28770
==========

to

==========
Fix setting breakpoint setting in await statements

CL https://codereview.chromium.org/2626753002 introduced a “real” token
position for the synthetic code that re-throws an exception returned
by an await’ed expression. This interferes with setting a breakpoint
in a line that contains an await, since the synthetic code happens to
be at the lowest compiled code address and will thus be picked as the
breakpoint location.

This CL makes the re-throw a synthetic token position again, but
includes synthetic token positions in stack traces. This is an alternative
fix for bug #28325.

BUG=#28770
R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/408a64756e166b884840482194600169d39784fb
==========

[hausner, 2017-03-02 20:21:48]

Committed patchset #1 (id:1) manually as
408a64756e166b884840482194600169d39784fb (presubmit successful).

[Cutch, 2017-03-06 16:47:16]

On 2017/03/02 20:21:48, hausner wrote:
> Committed patchset #1 (id:1) manually as
> 408a64756e166b884840482194600169d39784fb (presubmit successful).

LGTM but it would be nice if a service test as added based on the bug report so
we can catch regressions.