Improve handling of locations not from the uris the source map is for.

Improve handling of locations not from the uris the source map is for.

Make `MappingBundle` distinguish between locations from a file with source maps that do not have a source map entry and locations that are from a file that does not have a source map. This enables more graceful generation of stack traces with mixed Dart and JS stack frames.


Support a new source map bundle format useful for the Dart Dev Compiler. A source map bundle is a JSON array where each entry is a source map.

BUG=
R=sigmund@google.com

Committed: https://github.com/dart-lang/source_maps/commit/6783522f7af6e6abe94b3cb3be52a0624ab13aeb

[Jacob, 2016-12-09 01:56:27]

jacobr@google.com changed reviewers:
+ nweiz@google.com, sigmund@google.com

[Jacob, 2016-12-09 01:56:27]

Based on discussion of source_map_stack_trace cl.
This changes the default behavior to be good for Natalie's
case and my case.

[Siggi Cherem (dart-lang), 2016-12-09 16:38:38]

https://codereview.chromium.org/2564683003/diff/20001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2564683003/diff/20001/CHANGELOG.md#newcode3
CHANGELOG.md:3: * Improve handling of locations not from the uris the source map
is for.
nit, rephrase it to make it a bit more specific about where this change is and
how it can happen. For example:

* Extend`MappingBundle.spanFor` to accept requests for output files that don't
have source-maps data.

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart
File lib/parser.dart (right):

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart#newcode217
lib/parser.dart:217: // the input location as well as possible. Fake the file
offset because
nits:
- input and output is a bit confusing here. it's the input to this function, but
normally we refer to is as the output file.
- consider splitting the discussion of fake offsets to a separate paragraph.

One possible suggestion:

// Note: when there is no source map for an uri, this behaves like an
// identity function, returning the requested location as the result.

// Create a mock offset for the output location. We compute it in terms
// of the input line and column to minimize the chances that two different
// line and column locations are mapped to the same offset.

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart#newcode220
lib/parser.dart:220: var offset = line * 1000000000 + column;
wow, 1 billion! that's a very long line :)

Can we make it smaller? I think the chances of this being an issue are pretty
low if you use something like 1000. It would be nice to use a number that is
likely going to fit in 32 bit.

[Jacob, 2016-12-09 17:18:03]

ptal. applied the suggested comment tweaks and kept fake offsets under 32 bits.

https://codereview.chromium.org/2564683003/diff/20001/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2564683003/diff/20001/CHANGELOG.md#newcode3
CHANGELOG.md:3: * Improve handling of locations not from the uris the source map
is for.
On 2016/12/09 16:38:38, Siggi Cherem (dart-lang) wrote:
> nit, rephrase it to make it a bit more specific about where this change is and
> how it can happen. For example:
> 
> * Extend`MappingBundle.spanFor` to accept requests for output files that don't
> have source-maps data.
Done. Was trying to keep the changelog entries high level rather
than having them be implementation detail specific but perhaps I want too far.

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart
File lib/parser.dart (right):

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart#newcode217
lib/parser.dart:217: // the input location as well as possible. Fake the file
offset because
On 2016/12/09 16:38:38, Siggi Cherem (dart-lang) wrote:
> nits:
> - input and output is a bit confusing here. it's the input to this function,
but
> normally we refer to is as the output file.
> - consider splitting the discussion of fake offsets to a separate paragraph.
> 
> One possible suggestion:
> 
> // Note: when there is no source map for an uri, this behaves like an
> // identity function, returning the requested location as the result.
> 
> // Create a mock offset for the output location. We compute it in terms
> // of the input line and column to minimize the chances that two different
> // line and column locations are mapped to the same offset.

Done.

https://codereview.chromium.org/2564683003/diff/20001/lib/parser.dart#newcode220
lib/parser.dart:220: var offset = line * 1000000000 + column;
On 2016/12/09 16:38:38, Siggi Cherem (dart-lang) wrote:
> wow, 1 billion! that's a very long line :)
> 
> Can we make it smaller? I think the chances of this being an issue are pretty
> low if you use something like 1000. It would be nice to use a number that is
> likely going to fit in 32 bit.
Switched to a million. Note that SourceMapSpan does identity comparisons based
on just the offset so it is a bit scary that there could be collisions.

[Siggi Cherem (dart-lang), 2016-12-09 17:51:16]

lgtm, thanks Jacob

[Jacob, 2016-12-09 17:56:14]

Description was changed from

==========
Improve handling of locations not from the uris the source map is for.

Make `MappingBundle` distinguish between locations from a file with source maps
that do not have a source map entry and locations that are from a file that does
not have a source map. This enables more graceful generation of stack traces
with mixed Dart and JS stack frames.


Support a new source map bundle format useful for the Dart Dev Compiler. A
source map bundle is a JSON array where each entry is a source map.

BUG=
==========

to

==========
Improve handling of locations not from the uris the source map is for.

Make `MappingBundle` distinguish between locations from a file with source maps
that do not have a source map entry and locations that are from a file that does
not have a source map. This enables more graceful generation of stack traces
with mixed Dart and JS stack frames.


Support a new source map bundle format useful for the Dart Dev Compiler. A
source map bundle is a JSON array where each entry is a source map.

BUG=
R=sigmund@google.com

Committed:
https://github.com/dart-lang/source_maps/commit/6783522f7af6e6abe94b3cb3be52a...
==========

[Jacob, 2016-12-09 17:56:16]

Committed patchset #3 (id:40001) manually as
6783522f7af6e6abe94b3cb3be52a0624ab13aeb (presubmit successful).