Utf 16 columns

[tromey]

This series changes columns numbers to be reported in units of UTF-16 characters.

---

This change is 

[SimonSapin]

Looks good! A couple details:

---

Reviewed 1 of 1 files at r1, 2 of 2 files at r2, 2 of 2 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

src/tokenizer.rs, line 371 at r1 (raw file):

        debug_assert!(byte == b'\r' || byte == b'\n' || byte == b'\x0C');
        self.position += 1;
        if byte == b'\r' && self.next_byte() == Some(/* LF */ b'\n') {

Nit: this "LF" comment was to mirror the former is_cr argument, but now that we have \r instead, \n is consistent and the comment can be removed.

---

src/tokenizer.rs, line 705 at r2 (raw file):

            b'\xF0'...b'\xFF' => { tokenizer.consume_4byte_intro(); }
            _ => {
                tokenizer.advance(1);

Please add a comment here and in 4 other places with similar code below to indicate that the remaining cases are ASCII or leading bytes.

---

src/tokenizer.rs, line 378 at r3 (raw file):

        // This takes two UTF-16 characters to represent, so we
        // actually have an undercount.
        self.current_line_start_position = self.current_line_start_position.wrapping_sub(1);

This uses wrapping_sub in case the very first byte of the input starts a 4-bytes code point, right? If so, shouldn’t every other operation with current_line_start_position use wrapping arithmetic? (Or maybe replacing += 1 in consume_continuation_byte with wrapping_add is enough, since it will bring the position back over zero with input that is known to be well-formed UTF-8?) Either way, please add a test for that.

---

Comments from Reviewable

[tromey]

This uses wrapping_sub in case the very first byte of the input starts a 4-bytes code point, right? If so, shouldn’t every other operation with current_line_start_position use wrapping arithmetic? (Or maybe replacing += 1 in consume_continuation_byte with wrapping_add is enough, since it will bring the position back over zero with input that is known to be well-formed UTF-8?) Either way, please add a test for that.

Yes, nice catch. I believe the places where it is incremented must use wrapping_add; however, there's no need to use wrapping operations in current_source_location, because that should not be able to observe an "underflow" value (due to the UTF-8 encoding observation you point out).

[SimonSapin]

@bors-servo r+

---

Reviewed 1 of 2 files at r5, 2 of 2 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[bors-servo]

📌 Commit f721368 has been approved by SimonSapin

[bors-servo]

⌛ Testing commit f721368 with merge 7560c3a...

[bors-servo]

☀️ Test successful - status-travis
Approved by: SimonSapin
Pushing 7560c3a to master...