Scanner for UTF-8 byte arrays

Scanner for UTF-8 byte arrays

Introduces a SourceFile based on UTF-8 byte arrays and a scanner for
such source files.

Substrings are computed lazily. This should save some memory because
substring tokens in classes that are not used by some program are

R=kasperl@google.com

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

[ahe, 2013-10-16 14:26:04]

Comments so far.

I'm thinking that it would be nice to restore ArrayBasedScanner to minimize the
diff.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
File sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:17: * The source can be represented
either as a [List<int>] of UTF-8 bytes or as
I don't think you can say List<int> inside [].

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:18: * a [String]. It is recommended to
return a UTF-8 encoded list of bytes because
Break before "It is recommended" and add "The following text is non-normative:".

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:186: String tp =
data.runtimeType.toString();
Please don't abbreviate. Also, don't use runtimeType, it is expensive in
dart2js.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:187: reportReadError("The
CompilerInputProvider returned a $tp.");
This error message should be something like:

"Expected a 'String' or a 'List<int>' from input provider, but got:
${Error.safeToString(data)}.".

This follows the recommendation from warnings.dart:

 * Generally, we want to provide messages that consists of three sentences:
 * 1. what is wrong, 2. why is it wrong, 3. how do I fix it. However, we
 * combine the first two in [template] and the last in [howToFix].

In addition, this should be reported as an exception in user code, not a
compile-time error.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart
(right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart:1553:
compiler.assembledCode);
Try to keep things together by wrapping this line as:

SourceFile compiledFile =
    new StringSourceFile(null, compiler.assembledCode);

[lukas, 2013-10-17 07:21:25]

I re-added the ArrayBasedScanner class, not sure if it helps for reviewing
though.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
File sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:17: * The source can be represented
either as a [List<int>] of UTF-8 bytes or as
On 2013/10/16 14:26:04, ahe wrote:
> I don't think you can say List<int> inside [].

Done.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:18: * a [String]. It is recommended to
return a UTF-8 encoded list of bytes because
On 2013/10/16 14:26:04, ahe wrote:
> Break before "It is recommended" and add "The following text is
non-normative:".

Done.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:186: String tp =
data.runtimeType.toString();
On 2013/10/16 14:26:04, ahe wrote:
> Please don't abbreviate. Also, don't use runtimeType, it is expensive in
> dart2js.

Done.

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:187: reportReadError("The
CompilerInputProvider returned a $tp.");
> In addition, this should be reported as an exception in user code, not a
> compile-time error.

How do I do that?

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart
(right):

https://codereview.chromium.org/27510003/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_emitter/code_emitter_task.dart:1553:
compiler.assembledCode);
On 2013/10/16 14:26:04, ahe wrote:
> Try to keep things together by wrapping this line as:
> 
> SourceFile compiledFile =
>     new StringSourceFile(null, compiler.assembledCode);

Done.

[kasperl, 2013-10-17 08:50:39]

LGTM, but I haven't looked at all the details of the scanner character handling
changes. I assume Peter will want to look closely at those.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:182: sourceFile = new
Utf8BytesSourceFile(resourceUri.toString(), data);
Consider doing resourceUri.toString only once.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/mirror_renamer/renamer.dart
(right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/mirror_renamer/renamer.dart:89:
sb.write('};\x00'); // Terminate the string with '0', see [StringScanner].
Would it be easier to read if you did sb.writeCharCode(0) on a separate line
instead?

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/scanner.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:151: /**
Documentation in subclass [ArrayBasedScanner] */
Terminate these comments /** Documentation in subclass [ArrayBasedScanner] */
with .

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:387: // The
following are non-ASCII characters.
Can the check for $NBSP be guarded by the next >= 128 condition? Not that it
matters for performance.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:519: // !== is
kept for user-friendly error reporting
Nit: I'd terminate the ... is kept ... comments with .

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:231: * The length
thershold above which substring tokens are computed lazily.
thershold -> threshold

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:238: static final
int lazyThreshold = 4;
static const int LAZY_THRESHOLD

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:240: var
valueOrSublist;
Sublist -> LazyList?

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:288: if
(valueOrSublist is LazySubstring) {
Should this be an assert instead?

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:310: // @lry
replace by hash set after merging from svn trunk
I guess this is done?

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:315: if
(canonicalize) {
I'd consider doing something like:

  if (!canonicalize) return s;
  ...

to get rid of some of the nesting.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:317: if (result ==
null) {
I'd consider:

  if (result != null) return result;

to get rid of even more nesting.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:331: s = new
String.fromCharCodes(data.getRange(start, end));
Maybe add a comment that getRange doesn't copy the data, but it gives an
iterable "view" of the range.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:333: // TODO(lry),
this is measurably slow. Also sublist is allocated eagerly.
TODO(lry), -> TODO(lry):. Instead of allocated you might use a word like copy or
something.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart
(right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:158: 
That's a lot of whitespace.

[kasperl, 2013-10-17 11:55:03]

LGTM if you address my comments. I'd go ahead and submit this now and address
any feedback from Peter in another CL.

[lukas, 2013-10-17 17:49:34]

included the feedback from kasper, fixed the compiler tests (--compiler=none -r
vm dart2js).

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:182: sourceFile = new
Utf8BytesSourceFile(resourceUri.toString(), data);
On 2013/10/17 08:50:39, kasperl wrote:
> Consider doing resourceUri.toString only once.

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/mirror_renamer/renamer.dart
(right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/mirror_renamer/renamer.dart:89:
sb.write('};\x00'); // Terminate the string with '0', see [StringScanner].
On 2013/10/17 08:50:39, kasperl wrote:
> Would it be easier to read if you did sb.writeCharCode(0) on a separate line
> instead?

Sure.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/scanner.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:151: /**
Documentation in subclass [ArrayBasedScanner] */
On 2013/10/17 08:50:39, kasperl wrote:
> Terminate these comments /** Documentation in subclass [ArrayBasedScanner] */
> with .

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:387: // The
following are non-ASCII characters.
On 2013/10/17 08:50:39, kasperl wrote:
> Can the check for $NBSP be guarded by the next >= 128 condition? Not that it
> matters for performance.

Actually we can just remove the check for >= 128.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:519: // !== is
kept for user-friendly error reporting
On 2013/10/17 08:50:39, kasperl wrote:
> Nit: I'd terminate the ... is kept ... comments with .

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:231: * The length
thershold above which substring tokens are computed lazily.
On 2013/10/17 08:50:39, kasperl wrote:
> thershold -> threshold

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:238: static final
int lazyThreshold = 4;
On 2013/10/17 08:50:39, kasperl wrote:
> static const int LAZY_THRESHOLD

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:240: var
valueOrSublist;
On 2013/10/17 08:50:39, kasperl wrote:
> Sublist -> LazyList?

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:288: if
(valueOrSublist is LazySubstring) {
On 2013/10/17 08:50:39, kasperl wrote:
> Should this be an assert instead?

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:310: // @lry
replace by hash set after merging from svn trunk
On 2013/10/17 08:50:39, kasperl wrote:
> I guess this is done?

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:315: if
(canonicalize) {
On 2013/10/17 08:50:39, kasperl wrote:
> I'd consider doing something like:
> 
>   if (!canonicalize) return s;
>   ...
> 
> to get rid of some of the nesting.

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:317: if (result ==
null) {
On 2013/10/17 08:50:39, kasperl wrote:
> I'd consider:
> 
>   if (result != null) return result;
> 
> to get rid of even more nesting.

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:331: s = new
String.fromCharCodes(data.getRange(start, end));
On 2013/10/17 08:50:39, kasperl wrote:
> Maybe add a comment that getRange doesn't copy the data, but it gives an
> iterable "view" of the range.

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:333: // TODO(lry),
this is measurably slow. Also sublist is allocated eagerly.
On 2013/10/17 08:50:39, kasperl wrote:
> TODO(lry), -> TODO(lry):. Instead of allocated you might use a word like copy
or
> something.

Done.

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart
(right):

https://codereview.chromium.org/27510003/diff/17001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:158: 
On 2013/10/17 08:50:39, kasperl wrote:
> That's a lot of whitespace.

Done.

[lukas, 2013-10-18 06:41:37]

Committed patchset #3 manually as r28843 (presubmit successful).

[kasperl, 2013-10-18 06:44:56]

Still LGTM!

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/de...
File tests/compiler/dart2js/deferred_load_graph_segmentation_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/de...
tests/compiler/dart2js/deferred_load_graph_segmentation_test.dart:40:
deferredClasses
Does the where invocation fit on the same line as the deferredClasses reference
now?

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mi...
File tests/compiler/dart2js/mirrors_used_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mi...
tests/compiler/dart2js/mirrors_used_test.dart:82: if (library ==
compiler.mainApp
Fits on one line?

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/si...
File tests/compiler/dart2js/simple_inferrer_callers_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/si...
tests/compiler/dart2js/simple_inferrer_callers_test.dart:48: var fieldB =
classB.lookupLocalMember('field');;
;; -> ;

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/so...
File tests/compiler/dart2js/source_mapping_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/so...
tests/compiler/dart2js/source_mapping_test.dart:45: codeWithMarkers);
Fix indentation.

[lukas, 2013-10-18 08:39:45]

thanks, i fixed those and will commit them in some upcoming cl.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/de...
File tests/compiler/dart2js/deferred_load_graph_segmentation_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/de...
tests/compiler/dart2js/deferred_load_graph_segmentation_test.dart:40:
deferredClasses
On 2013/10/18 06:44:57, kasperl wrote:
> Does the where invocation fit on the same line as the deferredClasses
reference
> now?

Done.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mi...
File tests/compiler/dart2js/mirrors_used_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mi...
tests/compiler/dart2js/mirrors_used_test.dart:82: if (library ==
compiler.mainApp
On 2013/10/18 06:44:57, kasperl wrote:
> Fits on one line?

Done.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/si...
File tests/compiler/dart2js/simple_inferrer_callers_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/si...
tests/compiler/dart2js/simple_inferrer_callers_test.dart:48: var fieldB =
classB.lookupLocalMember('field');;
On 2013/10/18 06:44:57, kasperl wrote:
> ;; -> ;

Done.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/so...
File tests/compiler/dart2js/source_mapping_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/so...
tests/compiler/dart2js/source_mapping_test.dart:45: codeWithMarkers);
On 2013/10/18 06:44:57, kasperl wrote:
> Fix indentation.

Done.

[ngeoffray, 2013-10-18 10:19:36]

That was one hell of a change!

I think you forgot to upload the latest revision of the CL before submitting:
files that changed in the inferrer were not listed here.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:179: .then((data) {
Fits in previous line?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:639:
compiler.findHelper('TypeVariable');
Fits in one line.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart
(right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:99: *
Notifies on [$LF] characters in multi-line commends or strings.
commends -> comments

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:125:
* Appends a token that begins a ends group, represented by [value].
a ends -> an end

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:197:
* We call this method to discard '<' from the "grouping" stack
Replace 'We' by the actual callers.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/scanner.dart (right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:52: final
List<int> lineStarts = [0];
<int>[0]

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:58: 
Extra line.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:87: * Invoking
[currentAsUnicode] multiple times is safe, i.e.,
i.e. -> that is

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:166: /**
Documentation in subclass [ArrayBasedScanner]. */
So do the following methods only apply to the ArrayBasedScanner?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:858: * [next] is
the first character after the qoute.
qoute -> quote

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:932: next =
advance(); // Advance past the quote
Missing .

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:955: int next =
advance(); // Advance past the (last) quote (of three)
Missing .

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:1051: // as
errors. However, the rest of the parser assume the groups
assume -> assumes

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/scanner_task.dart
(right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/scanner_task.dart:43: * value
('\x00'). If [source] does not with '0', the string is copied before
not *end* with

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/string_scanner.dart
(right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/string_scanner.dart:45: 
Extra line.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:106: * For
[StringToken]s the value includes the quotes, explicit escapes, etc.
the value -> [value]

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:112: * For symbol
and keyword tokens, returns the string value reprenseted by this
represented

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:143: * see
implementaiton in [KeywordToken].
implementation

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:174: * A symbol
token represents the symbol in its precendence info.
symbol token -> [SymbolToken]

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:193: * A
[BeginGroupToken] reprsents a symbol that may be the beginning of
represents

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:202: : super(info,
charOffset);
Fits in one line?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:226: * number
literals, comments and error tokens, using the corresponding
comments, and ...

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:240: var
valueOrLazySubstring;
You could put the union type of this field in comments, eg:

var /* String or LazySubstring */ valueOrLazySubstring;

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:249: [bool
canonicalize = false])
Make it a named parameter? Easier when reading call sites.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:258: int
charOffset, [bool canonicalize = false])
ditto

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:263: canonicalize);
indentation.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:311: new HashSet();
HashSet<String>()

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:337: * lazily. The
substring can either originate in a string or in a [:List<int>:]
originate in -> originate from?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:348: * If this
substring is based on a String, the boolean indicates wheter the
the boolean -> [boolValue]

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:351: * For
substrings based on a byte array, the boolean value is true if the
ditto

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:376: * of 1M.
[length] has 9 bits, which covers 512 characters.
1M -> 1MB.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:378: * The file
html_dart2js.dart is currently around 1M.
1M -> 1MB

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart
(right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:1: //
Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
2011 -> 2013

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:16: *
Points to the offset of the byte last returned by [advance].
byte last -> last byte?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:140:
throw "Invalid UTF-8 byte sequence: ${bytes.sublist(startOffset, end)}";
Will users face this error? If yes, we should throw differently.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:187: 
Extra line,

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/source_file.dart (right):

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/source_file.dart:37: set length(v);
int v?

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/source_file.dart:60: set lineStarts(v)
=> lineStartsCache = v;
v -> List<int> v.

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/source_file.dart:176: set length(v) =>
lengthCache = v;
int v

https://codereview.chromium.org/27510003/diff/28001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/source_file.dart:187: set length(v) {
}
int v

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mo...
File tests/compiler/dart2js/mock_compiler.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/mo...
tests/compiler/dart2js/mock_compiler.dart:390: mainApp.entryCompilationUnit);
Fits in one line?

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/pa...
File tests/compiler/dart2js/patch_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/pa...
tests/compiler/dart2js/patch_test.dart:740: 'method', compiler.coreLibrary, 0);
One line?

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/pa...
tests/compiler/dart2js/patch_test.dart:743:
cls.implementation.lookupLocalMember('method');
ditto

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/pa...
tests/compiler/dart2js/patch_test.dart:750: 'clear', compiler.coreLibrary, 0);
ditto

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/re...
File tests/compiler/dart2js/resolver_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/re...
tests/compiler/dart2js/resolver_test.dart:209:
fooElement.lookupLocalMember("foo");
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/re...
tests/compiler/dart2js/resolver_test.dart:232:
fooElement.lookupLocalMember("foo");
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/re...
tests/compiler/dart2js/resolver_test.dart:625: compiler.mainApp.find(className);
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/re...
tests/compiler/dart2js/resolver_test.dart:629: new
Selector.callConstructor(constructor,
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/ty...
File tests/compiler/dart2js/type_checker_test.dart (right):

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/ty...
tests/compiler/dart2js/type_checker_test.dart:474:
library.find("ClassWithMethods");
One line?

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/ty...
tests/compiler/dart2js/type_checker_test.dart:1100:
classTest.lookupLocalMember("test");
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/ty...
tests/compiler/dart2js/type_checker_test.dart:1142:
classTest.lookupLocalMember("test");
One line.

https://codereview.chromium.org/27510003/diff/28001/tests/compiler/dart2js/ty...
tests/compiler/dart2js/type_checker_test.dart:1166:
classTest.lookupLocalMember("test");
One line.

[sra1, 2013-10-22 19:52:30]

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:238: static const
int LAZY_THRESHOLD = 4;
How did you calculate this threshold?

[lukas, 2013-10-23 07:11:00]

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:238: static const
int LAZY_THRESHOLD = 4;
On 2013/10/22 19:52:31, sra1 wrote:
> How did you calculate this threshold?

Short strings have a smaller footprint than a CompactLazySubstring.
The actual value is taken from benchmarking the scanner and the parser.

[ahe, 2013-10-23 13:39:44]

Lukas, I think you made a number of understandable mistakes in this code review
process, and I'll discuss those with you in person.

For the record so others don't need to repeat this: since you made changes
outside dart2js, you should have CC'ed the "owners" of those files.

I have identified the following changes outside dart2js:

https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/pkg/...
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/...
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/...
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/...
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/util...

Each of these links contains links to the files' history, which should help you
identify who might benefit from hearing about this change. I have done my best
to add those people.

[nweiz, 2013-10-23 17:48:50]

Pub change lgtm.

[Alan Knight, 2013-10-23 17:49:52]

docgen change lgtm

[lukas, 2013-10-24 16:48:35]

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:179: .then((data) {
Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:639:
compiler.findHelper('TypeVariable');
Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart
(right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:99: *
Notifies on [$LF] characters in multi-line commends or strings.
Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:125:
* Appends a token that begins a ends group, represented by [value].
On 2013/10/18 10:19:37, ngeoffray wrote:
> a ends -> an end

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/array_based_scanner.dart:197:
* We call this method to discard '<' from the "grouping" stack
On 2013/10/18 10:19:37, ngeoffray wrote:
> Replace 'We' by the actual callers.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/scanner.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:52: final
List<int> lineStarts = [0];
On 2013/10/18 10:19:37, ngeoffray wrote:
> <int>[0]

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:58: 
On 2013/10/18 10:19:37, ngeoffray wrote:
> Extra line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:87: * Invoking
[currentAsUnicode] multiple times is safe, i.e.,
On 2013/10/18 10:19:37, ngeoffray wrote:
> i.e. -> that is

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:166: /**
Documentation in subclass [ArrayBasedScanner]. */
On 2013/10/18 10:19:37, ngeoffray wrote:
> So do the following methods only apply to the ArrayBasedScanner?

I just decided to put the documentation together with the actual implementation.
Technically the implementations could go here, but Peter preferred to separate
scanning code from token building code.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:858: * [next] is
the first character after the qoute.
On 2013/10/18 10:19:37, ngeoffray wrote:
> qoute -> quote

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:932: next =
advance(); // Advance past the quote
On 2013/10/18 10:19:37, ngeoffray wrote:
> Missing .

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:955: int next =
advance(); // Advance past the (last) quote (of three)
On 2013/10/18 10:19:37, ngeoffray wrote:
> Missing .

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner.dart:1051: // as
errors. However, the rest of the parser assume the groups
On 2013/10/18 10:19:37, ngeoffray wrote:
> assume -> assumes

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/scanner_task.dart
(right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/scanner_task.dart:43: * value
('\x00'). If [source] does not with '0', the string is copied before
On 2013/10/18 10:19:37, ngeoffray wrote:
> not *end* with

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/string_scanner.dart
(right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/string_scanner.dart:45: 
On 2013/10/18 10:19:37, ngeoffray wrote:
> Extra line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/token.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:106: * For
[StringToken]s the value includes the quotes, explicit escapes, etc.
On 2013/10/18 10:19:37, ngeoffray wrote:
> the value -> [value]

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:112: * For symbol
and keyword tokens, returns the string value reprenseted by this
On 2013/10/18 10:19:37, ngeoffray wrote:
> represented

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:143: * see
implementaiton in [KeywordToken].
On 2013/10/18 10:19:37, ngeoffray wrote:
> implementation

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:174: * A symbol
token represents the symbol in its precendence info.
On 2013/10/18 10:19:37, ngeoffray wrote:
> symbol token -> [SymbolToken]

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:193: * A
[BeginGroupToken] reprsents a symbol that may be the beginning of
On 2013/10/18 10:19:37, ngeoffray wrote:
> represents

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:202: : super(info,
charOffset);
On 2013/10/18 10:19:37, ngeoffray wrote:
> Fits in one line?

No :)

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:226: * number
literals, comments and error tokens, using the corresponding
On 2013/10/18 10:19:37, ngeoffray wrote:
> comments, and ...

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:240: var
valueOrLazySubstring;
On 2013/10/18 10:19:37, ngeoffray wrote:
> You could put the union type of this field in comments, eg:
> 
> var /* String or LazySubstring */ valueOrLazySubstring;

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:249: [bool
canonicalize = false])
On 2013/10/18 10:19:37, ngeoffray wrote:
> Make it a named parameter? Easier when reading call sites.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:258: int
charOffset, [bool canonicalize = false])
On 2013/10/18 10:19:37, ngeoffray wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:263: canonicalize);
On 2013/10/18 10:19:37, ngeoffray wrote:
> indentation.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:311: new HashSet();
On 2013/10/18 10:19:37, ngeoffray wrote:
> HashSet<String>()

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:337: * lazily. The
substring can either originate in a string or in a [:List<int>:]
On 2013/10/18 10:19:37, ngeoffray wrote:
> originate in -> originate from?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:348: * If this
substring is based on a String, the boolean indicates wheter the
On 2013/10/18 10:19:37, ngeoffray wrote:
> the boolean -> [boolValue]

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:351: * For
substrings based on a byte array, the boolean value is true if the
On 2013/10/18 10:19:37, ngeoffray wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:376: * of 1M.
[length] has 9 bits, which covers 512 characters.
On 2013/10/18 10:19:37, ngeoffray wrote:
> 1M -> 1MB.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/token.dart:378: * The file
html_dart2js.dart is currently around 1M.
On 2013/10/18 10:19:37, ngeoffray wrote:
> 1M -> 1MB

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart
(right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:1: //
Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
On 2013/10/18 10:19:37, ngeoffray wrote:
> 2011 -> 2013

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:16: *
Points to the offset of the byte last returned by [advance].
On 2013/10/18 10:19:37, ngeoffray wrote:
> byte last -> last byte?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:140:
throw "Invalid UTF-8 byte sequence: ${bytes.sublist(startOffset, end)}";
On 2013/10/18 10:19:37, ngeoffray wrote:
> Will users face this error? If yes, we should throw differently.

That could only happen in a file with wrong UTF-8 encoding, in which case
UTF8.decode would probably fail earlier. I could also just remove the check.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart:187: 
On 2013/10/18 10:19:37, ngeoffray wrote:
> Extra line,

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/source_file.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/source_file.dart:37: set length(v);
On 2013/10/18 10:19:37, ngeoffray wrote:
> int v?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/source_file.dart:60: set lineStarts(v)
=> lineStartsCache = v;
On 2013/10/18 10:19:37, ngeoffray wrote:
> v -> List<int> v.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/source_file.dart:176: set length(v) =>
lengthCache = v;
On 2013/10/18 10:19:37, ngeoffray wrote:
> int v

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/source_file.dart:187: set length(v) {
}
On 2013/10/18 10:19:37, ngeoffray wrote:
> int v

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
File tests/compiler/dart2js/mock_compiler.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/mock_compiler.dart:390: mainApp.entryCompilationUnit);
On 2013/10/18 10:19:37, ngeoffray wrote:
> Fits in one line?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
File tests/compiler/dart2js/patch_test.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/patch_test.dart:740: 'method', compiler.coreLibrary, 0);
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/patch_test.dart:743:
cls.implementation.lookupLocalMember('method');
On 2013/10/18 10:19:37, ngeoffray wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/patch_test.dart:750: 'clear', compiler.coreLibrary, 0);
On 2013/10/18 10:19:37, ngeoffray wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
File tests/compiler/dart2js/resolver_test.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/resolver_test.dart:209:
fooElement.lookupLocalMember("foo");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/resolver_test.dart:232:
fooElement.lookupLocalMember("foo");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/resolver_test.dart:625: compiler.mainApp.find(className);
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/resolver_test.dart:629: new
Selector.callConstructor(constructor,
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
File tests/compiler/dart2js/type_checker_test.dart (right):

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/type_checker_test.dart:474:
library.find("ClassWithMethods");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line?

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/type_checker_test.dart:1100:
classTest.lookupLocalMember("test");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/type_checker_test.dart:1142:
classTest.lookupLocalMember("test");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.

https://chromiumcodereview.appspot.com/27510003/diff/28001/tests/compiler/dar...
tests/compiler/dart2js/type_checker_test.dart:1166:
classTest.lookupLocalMember("test");
On 2013/10/18 10:19:37, ngeoffray wrote:
> One line.

Done.