Compute source spans for IR nodes and J/K-elements

Compute source spans for IR nodes and J/K-elements

R=sigmund@google.com

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

[Johnni Winther, 2017-07-07 15:57:36]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2017-07-07 19:25:02]

lgtm

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/element_map_impl.dart (right):

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/element_map_impl.dart:129: ir.Location location;
Let's store here just the Uri, and only use location within the loop.

Ideally, I'd like to not use Location - but it is true that right now it is the
simplest way to get the fileUri. The alternative would be walk up the tree until
we are in a procedure/field/class/library which stores a fileUri.

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/element_map_impl.dart:158: return
_getSourceSpanFromTreeNode(data.node);
there is not a common interface for these in kernel, but my understanding is
that all 3: library, class, and members will have a fileUri directly on it and
most likely we don't need to walk up the tree from them.

[Johnni Winther, 2017-07-11 08:06:59]

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/element_map_impl.dart (right):

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/element_map_impl.dart:129: ir.Location location;
On 2017/07/07 19:25:02, Siggi Cherem (dart-lang) wrote:
> Let's store here just the Uri, and only use location within the loop.

Done.

> Ideally, I'd like to not use Location - but it is true that right now it is
the
> simplest way to get the fileUri. The alternative would be walk up the tree
until
> we are in a procedure/field/class/library which stores a fileUri.

Long-term we should use Location (i.e. change SourceSpan to take line/position
info). Added a TODO.

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/element_map_impl.dart:158: return
_getSourceSpanFromTreeNode(data.node);
On 2017/07/07 19:25:02, Siggi Cherem (dart-lang) wrote:
> there is not a common interface for these in kernel, but my understanding is
> that all 3: library, class, and members will have a fileUri directly on it and
> most likely we don't need to walk up the tree from them.

They might, but MemberData.node might be a function expression so we need to
search anyway.

[Johnni Winther, 2017-07-11 08:34:53]

Description was changed from

==========
Compute source spans for IR nodes and J/K-elements
==========

to

==========
Compute source spans for IR nodes and J/K-elements

R=sigmund@google.com

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

[Johnni Winther, 2017-07-11 08:34:54]

Committed patchset #2 (id:20001) manually as
ddad7cc116ffd1f6e75fe6e65d203ce0058d94b6 (presubmit successful).

[Siggi Cherem (dart-lang), 2017-07-11 19:04:55]

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/element_map_impl.dart (right):

https://codereview.chromium.org/2974663002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/element_map_impl.dart:129: ir.Location location;
On 2017/07/11 08:06:59, Johnni Winther wrote:
> On 2017/07/07 19:25:02, Siggi Cherem (dart-lang) wrote:
> > Let's store here just the Uri, and only use location within the loop.
> 
> Done.
> 
> > Ideally, I'd like to not use Location - but it is true that right now it is
> the
> > simplest way to get the fileUri. The alternative would be walk up the tree
> until
> > we are in a procedure/field/class/library which stores a fileUri.
> 
> Long-term we should use Location (i.e. change SourceSpan to take line/position
> info). Added a TODO.

I'd like to use package:source_span for this (maybe kernel should switch to use
that too :))

I especially don't want to compute line/position here, but defer that to the
source-span logic.