1501155 - [BinAST] Use WTF-8 in string encoding

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

The string literal in binjs file is raw UTF-8 string which doesn't contain any escape sequence like \uXXXX, which means a lone surrogate should be representable there.
We can use WTF-8 as the string encoding in the binjs file.

related issue in binjs-ref https://github.com/binast/binjs-ref/issues/190

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Use WTF-8 as BinAST string encoding.

corresponds to https://github.com/binast/binjs-ref/pull/193
will ask review once binjs-ref is patched

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Add AtomizeWTF8Chars.

This adds API for WTF-8 [1] that is used by BinAST parser which is fixed in Part 2.
  * add WTF8Chars
  * add AtomizeWTF8Chars
  * templatize methods/classes in CharacterEncoding.cpp to accept both UTF8Chars and WTF8Chars

given AtomizeWTF8Chars needs the exact same optimization added by bug 1494942,
I've cloned them by template.

[1] https://simonsapin.github.io/wtf-8/

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Use WTF-8 as BinAST string encoding.

And just replaced the consumer in BinAST side.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 9025228 [details] [diff] [review]
Part 2: Use WTF-8 as BinAST string encoding.

Review of attachment 9025228 [details] [diff] [review]:
-----------------------------------------------------------------

Most complicated patch ever :)

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 9025227 [details] [diff] [review]
Part 1: Add AtomizeWTF8Chars.

Review of attachment 9025227 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/CharacterEncoding.cpp
@@ +210,2 @@
>   */
> +template <typename InputCharsT>

Use class for this, because the parameter *is* always a class.  Same for the other InputCharsT below.

@@ +214,2 @@
>  {
>      MOZ_ASSERT(1 <= utf8Length && utf8Length <= 4);

Add

  static_assert(std::is_same<InputCharsT, UTF8Chars>::value ||
                std::is_same<InputCharsT, WTF8Chars>::value,
                "must be either UTF-8 or WTF-8");

at top here.

@@ +404,2 @@
>  static void
> +CopyAndInflateUTF8IntoBuffer(JSContext* cx, const InputCharsT src, CharT *dst, size_t outlen, bool allASCII)

Put the * by CharT while you're touching.

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ba2f019ce6d420275d088749482cf77f35d4bf4
Bug 1501155 - Part 1: Add AtomizeWTF8Chars. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1a6e2052ea19b8fbf7fde7e3aa25629ff5163be
Bug 1501155 - Part 2: Use WTF-8 as BinAST string encoding. r=Yoric

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/4ba2f019ce6d
https://hg.mozilla.org/mozilla-central/rev/b1a6e2052ea1