Fix source locations for inlined and patched functions.

Fix source locations for inlined and patched functions.

R=ahe@google.com,floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=12108

[floitsch, 2012-09-06 16:44:55]

LGTM, but if possible wait for Peter's comments too.
Please make sure it is not too slow (although, afaics, the SourceLocations were
already created before and shouldn't slow anything down).

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1478: if (element is
FunctionElement && element.dynamic.patch != null) {
don't use 'dynamic'.
Either use "as", or create a new local.
I think most people here prefer a new local.

[ahe, 2012-09-08 08:55:50]

LGTM!

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/code_buffer.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/code_buffer.dart:97: final sourcePosition;
What is a sourcePosition?

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/source_map_builder.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/source_map_builder.dart:145: class _Entry {
Why is this class private?

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/source_map_builder.dart:171: if (token.kind ===
IDENTIFIER_TOKEN) return token.slowToString();
This should be isIdentifier(token) which is defined in parser.dart. Feel free to
move it to token.dart or something so it is exported by scannerlib.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1478: if (element is
FunctionElement && element.dynamic.patch != null) {
On 2012/09/06 16:44:55, floitsch wrote:
> don't use 'dynamic'.
> Either use "as", or create a new local.
> I think most people here prefer a new local.

Yes. Please don't use "as".

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1479: element =
element.dynamic.patch;
This is a hack and you should add a TODO for Johnni. He is currently working on
something that should fix this.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:21: Element sourceElement =
element.patch == null ? element : element.patch;
Another TODO for Johnni.

https://chromiumcodereview.appspot.com/10915122/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/source_mapping_test.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/source_mapping_test.dart:33:
buffer.forEachSourceLocation((int offset, var sourcePosition) {
What is the offset?

[podivilov, 2012-09-10 11:13:23]

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/code_buffer.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/code_buffer.dart:97: final sourcePosition;
On 2012/09/08 08:55:50, ahe wrote:
> What is a sourcePosition?

sourcePosition is SourceFileLocation. CodeBuffer doesn't actually depend on
SourceFileLocation so I decided to omit the type. Maybe add a comment?

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/source_map_builder.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/source_map_builder.dart:145: class _Entry {
On 2012/09/08 08:55:50, ahe wrote:
> Why is this class private?

Done.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/source_map_builder.dart:171: if (token.kind ===
IDENTIFIER_TOKEN) return token.slowToString();
On 2012/09/08 08:55:50, ahe wrote:
> This should be isIdentifier(token) which is defined in parser.dart. Feel free
to
> move it to token.dart or something so it is exported by scannerlib.

Done.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1478: if (element is
FunctionElement && element.dynamic.patch != null) {
On 2012/09/08 08:55:50, ahe wrote:
> On 2012/09/06 16:44:55, floitsch wrote:
> > don't use 'dynamic'.
> > Either use "as", or create a new local.
> > I think most people here prefer a new local.
> 
> Yes. Please don't use "as".

Done.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1479: element =
element.dynamic.patch;
On 2012/09/08 08:55:50, ahe wrote:
> This is a hack and you should add a TODO for Johnni. He is currently working
on
> something that should fix this.

Done.

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:21: Element sourceElement =
element.patch == null ? element : element.patch;
On 2012/09/08 08:55:50, ahe wrote:
> Another TODO for Johnni.

Done.

https://chromiumcodereview.appspot.com/10915122/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/source_mapping_test.dart (right):

https://chromiumcodereview.appspot.com/10915122/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/source_mapping_test.dart:33:
buffer.forEachSourceLocation((int offset, var sourcePosition) {
On 2012/09/08 08:55:50, ahe wrote:
> What is the offset?

It's an offset in the buffer. Renamed to targetOffset.

[ngeoffray, 2012-09-10 14:00:24]

Hi Pavel,

This change broke dart2js:
http://build.chromium.org/p/client.dart/builders/dart2js-linux-release-checke...

Are you working on a fix?