Handle BOMs in dart2js.

sdk/lib/_internal/compiler/implementation/scanner/utf8_bytes_scanner.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of scanner;
 
 /**
  * Scanner that reads from a UTF-8 encoded list of bytes and creates tokens
  * that points to substrings.
  */
@@ skipping 51 matching lines @@
    * string text of the source file is decoded.
    *
    * The list of UTF-8 bytes [file.slowUtf8Bytes()] is expected to return an
    * array whose last element is '0' to signal the end of the file. If this
    * is not the case, the entire array is copied before scanning.
    */
   Utf8BytesScanner(SourceFile file, {bool includeComments: false})
       : bytes = file.slowUtf8Bytes(),
         super(file, includeComments) {
     ensureZeroTermination();
+    // Skip a leading BOM.
+    if (_containsBomAt(0)) byteOffset += 3;
   }
 
   /**
    * Creates a new Utf8BytesScanner from a list of UTF-8 bytes.
    *
    * The last element of the list is expected to be '0' to signal the end of
    * the file. If this is not the case, the entire array is copied before
    * scanning.
    */
   Utf8BytesScanner.fromBytes(this.bytes, {bool includeComments: false})
       : super(null, includeComments) {
     ensureZeroTermination();
   }
 
   void ensureZeroTermination() {
     if (bytes.isEmpty || bytes[bytes.length - 1] != 0) {
       // TODO(lry): abort instead of copying the array, or warn?
       var newBytes =  new Uint8List(bytes.length + 1);
       for (int i = 0; i < bytes.length; i++) {
         newBytes[i] = bytes[i];
       }
       newBytes[bytes.length] = 0;
       bytes = newBytes;
     }
   }
 
+  bool _containsBomAt(int offset) {
+    const BOM_UTF8 = const [0xEF, 0xBB, 0xBF];
+
+    assert(() {

[ahe, 2014/01/15 13:15:38]

I would probably add a unit test for this instead of an assert.

If you want a fast scanner, it should be so fast that it is done before an
optimizing JIT notices what is going on. This means that you should keep the
code simple, for example, by keeping the number of tokens low.

We know that dart2js performance is affected by how large the snapshot is
(because there will be less heap space available and more GCs).  By being frugal
about asserts, you help keep the snapshot smaller.

[floitsch, 2014/01/15 13:44:46]

There is already a test.
Removing the assert.

+      // Verify that BOM_UTF8 corresponds to the UTF-8 BOM character.
+      // The UTF-8 decoder strips leading BOMs. Add another character in front.
+      List<int> prefixedBom = [0x20]..addAll(BOM_UTF8);
+      String decoded = UTF8.decode(prefixedBom);
+      return decoded.codeUnitAt(1) == UNICODE_BOM_CHARACTER_RUNE;
+    });
+
+    return offset + 3 < bytes.length &&
+        bytes[offset] == BOM_UTF8[0] &&
+        bytes[offset + 1] == BOM_UTF8[1] &&
+        bytes[offset + 2] == BOM_UTF8[2];
+  }
+
   int advance() => bytes[++byteOffset];
 
   int peek() => bytes[byteOffset + 1];
 
   /**
    * Returns the unicode code point starting at the byte offset [startOffset]
    * with the byte [nextByte]. If [advance] is true the current [byteOffset]
    * is advanced to the last byte of the code point.
    */
   int nextCodePoint(int startOffset, int nextByte, bool advance) {
     // The number of 1s in the first byte indicate the number of bytes, at
     // least 2.
     int numBytes = 2;
     int bit = 0x20;
     while ((nextByte & bit) != 0) {
       numBytes++;
       bit >>= 1;
     }
     int end = startOffset + numBytes;
     if (advance) {
       byteOffset = end - 1;
     }
     // TODO(lry): measurably slow, decode creates first a Utf8Decoder and a
     // _Utf8Decoder instance. Also the sublist is eagerly allocated.
     String codePoint = UTF8.decode(bytes.sublist(startOffset, end));
+    if (codePoint.length == 0) {
+      // The UTF-8 decoder discards leading BOM characters.
+      // TODO(floitsch): don't just assume that removed characters were the
+      // BOM.
+      assert(_containsBomAt(startOffset));
+      codePoint = "\u{FEFF}";

[ahe, 2014/01/14 14:23:59]

Can you use UNICODE_BOM_CHARACTER_RUNE somehow?

[floitsch, 2014/01/14 18:37:58]

I don't think so.
I don't want to recreate a string every time. I could add an assert, though:
assert(codePoint == new String.fromCharCode(UNICODE_BOM_CHARACTER_RUNE));

should I?

[ahe, 2014/01/15 13:15:38]

I would expect that it is extremely rare if a file contains more than zero BOM.
I would suggest writing a test instead of using asserts.

[floitsch, 2014/01/15 13:44:46]

I will allocate a new string.
I prefer keeping the assert here. It is a code-path that is executed very
rarely.
Test already exists.

[ahe, 2014/01/15 13:54:23]

Yes. Keep the assert on line 146. That assert tests an invariant about code that
is out of your control.

Alternatively, turn it into an internal error.
My suggestion is this:

const String UNICODE_BOM_CHARACTER = "\u{FEFF}";

And then this test:

main() {
  Expect.stringEquals(new String.fromCharCode(UNICODE_BOM_CHARACTER_RUNE),
UNICODE_BOM_CHARACTER);
}

[floitsch, 2014/01/15 14:51:39]

I see.
As you say: there shouldn't be many BOMs inside the code. So keeping the simpler
new String.fromCharCode.

+    }
     if (codePoint.length == 1) {
       if (advance) {
         utf8Slack += (numBytes - 1);
         scanSlack = numBytes - 1;
         scanSlackOffset = byteOffset;
       }
       return codePoint.codeUnitAt(0);
     } else if (codePoint.length == 2) {
       if (advance) {
         utf8Slack += (numBytes - 2);
@@ skipping 53 matching lines @@
 
   void appendSubstringToken(PrecedenceInfo info, int start, bool asciiOnly,
                             [int extraOffset = 0]) {
     tail.next = new StringToken.fromUtf8Bytes(
         info, bytes, start, byteOffset + extraOffset, asciiOnly, tokenStart);
     tail = tail.next;
   }
 
   bool atEndOfFile() => byteOffset >= bytes.length - 1;
 }