[Fasta] include source code in dill

[Fasta] include source code in dill

At least for now, only when passing --include-source

BUG=
R=ahe@google.com

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

[jensj, 2017-03-03 10:08:31]

jensj@google.com changed reviewers:
+ ahe@google.com

[ahe, 2017-03-03 12:30:32]

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/dill/dill_target.dart (right):

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/dill/dill_target.dart:34: void
addSourceInformation(Uri uri, List<int> lineStarts, String sourceCode) {
This should probably be List<int>, not String.

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:109: void
addSourceInformation(Uri uri, List<int> lineStarts, String sourceCode) {
List<int>

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
File pkg/front_end/lib/src/fasta/source/source_loader.dart (right):

https://codereview.chromium.org/2729913005/diff/1/pkg/front_end/lib/src/fasta...
pkg/front_end/lib/src/fasta/source/source_loader.dart:70: var source =
includeSource ? new String.fromCharCodes(bytes) : "";
This should be UTF8.encode(bytes), but I really think it should be List<int>.

[jensj, 2017-03-03 13:02:21]

Done.

[ahe, 2017-03-03 13:14:51]

lgtm

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_loader.dart (right):

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_loader.dart:47: bool get includeSource
=> CompilerContext.current.options.includeSource;
This should be set when constructing this object:

final includeSource = CompilerContext.current.options.includeSource;

Looking up options may be rather slow.

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_loader.dart:70: var source =
includeSource ? bytes : const <int>[];
var -> List<int>

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
File pkg/kernel/lib/binary/ast_from_binary.dart (right):

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
pkg/kernel/lib/binary/ast_from_binary.dart:77: List<int> readRawString() {
Change name to readUtf8Bytes, readUint8List, or something like that?

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
pkg/kernel/lib/binary/ast_from_binary.dart:78: return new
List<int>.generate(readUInt(), (i) => readByte());
How about:

List<int> bytes = new Uint8List(readUInt());
bytes.setRange(0, bytes.length, _bytes, _byteIndex);
_byteIndex += bytes.length;

[jensj, 2017-03-06 07:38:36]

Comments addressed.
I noticed, though, that the data is 0-terminated, which we don't want to include
as the actual source (it leaves an ugly box-like-character in the display in
observatory).
I've done a "bytes.sublist(0, bytes.length - 1)" for now, if you have a better
idea, I'm all ears!

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_loader.dart (right):

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_loader.dart:47: bool get includeSource
=> CompilerContext.current.options.includeSource;
On 2017/03/03 13:14:50, ahe wrote:
> This should be set when constructing this object:
> 
> final includeSource = CompilerContext.current.options.includeSource;
> 
> Looking up options may be rather slow.

Done.

https://codereview.chromium.org/2729913005/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_loader.dart:70: var source =
includeSource ? bytes : const <int>[];
On 2017/03/03 13:14:50, ahe wrote:
> var -> List<int>

Done.

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
File pkg/kernel/lib/binary/ast_from_binary.dart (right):

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
pkg/kernel/lib/binary/ast_from_binary.dart:77: List<int> readRawString() {
On 2017/03/03 13:14:50, ahe wrote:
> Change name to readUtf8Bytes, readUint8List, or something like that?

Done.

https://codereview.chromium.org/2729913005/diff/20001/pkg/kernel/lib/binary/a...
pkg/kernel/lib/binary/ast_from_binary.dart:78: return new
List<int>.generate(readUInt(), (i) => readByte());
On 2017/03/03 13:14:51, ahe wrote:
> How about:
> 
> List<int> bytes = new Uint8List(readUInt());
> bytes.setRange(0, bytes.length, _bytes, _byteIndex);
> _byteIndex += bytes.length;

Done.

[ahe, 2017-03-07 11:33:58]

On 2017/03/06 07:38:36, jensj wrote:
> Comments addressed.
> I noticed, though, that the data is 0-terminated, which we don't want to
include
> as the actual source (it leaves an ugly box-like-character in the display in
> observatory).
> I've done a "bytes.sublist(0, bytes.length - 1)" for now, if you have a better
> idea, I'm all ears!

if (bytes is Uint8List) {
  var sublist = new Uint8List(bytes.buffer, bytes.offsetInBytes, bytes.length -
1);
}

[ahe, 2017-03-07 11:34:31]

On 2017/03/07 11:33:58, ahe wrote:
> On 2017/03/06 07:38:36, jensj wrote:
> > Comments addressed.
> > I noticed, though, that the data is 0-terminated, which we don't want to
> include
> > as the actual source (it leaves an ugly box-like-character in the display in
> > observatory).
> > I've done a "bytes.sublist(0, bytes.length - 1)" for now, if you have a
better
> > idea, I'm all ears!
> 
> if (bytes is Uint8List) {
>   var sublist = new Uint8List(bytes.buffer, bytes.offsetInBytes, bytes.length
-
> 1);
> }

Sorry, should be 

var sublist = new Uint8List.view(bytes.buffer, bytes.offsetInBytes, bytes.length
- 1);

[jensj, 2017-03-07 14:41:09]

Patchset #4 (id:60001) has been deleted

[jensj, 2017-03-07 14:44:58]

Patchset #4 (id:80001) has been deleted

[jensj, 2017-03-07 14:47:51]

Thanks for the view hint. I was looking for something like that but couldn't
find it.
Landing.

[jensj, 2017-03-07 14:48:31]

Description was changed from

==========
[Fasta] include source code in dill

At least for now, only when passing --include-source

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

to

==========
[Fasta] include source code in dill

At least for now, only when passing --include-source

BUG=
R=ahe@google.com

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

[jensj, 2017-03-07 14:48:33]

Committed patchset #4 (id:100001) manually as
a58fe4898dc95ab8e59f917a962fc243a7c22a74 (presubmit successful).