1496863 - Various UTF-8-to-UTF-16 inflation mini-fixes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Jeff Walden [:Waldo]]

Noticed these while looking at the changes bug 1495571 made.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Add a constexpr char16_t unicode::REPLACEMENT_CHARACTER, and change the two (!) separate const char16_t variables of the same value in CharacterEncoding.cpp with it

(lol)

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Use precise capture-lists for the various lambdas in CharacterEncoding.cpp, rather than using [&] and capturing everything willy-nilly by reference

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Remove an obsolete comment

Comment 4

[Tom S. (please needinfo tschuster)]

Comment on attachment 9014972 [details] [diff] [review]
Add a constexpr char16_t unicode::REPLACEMENT_CHARACTER, and change the two (!) separate const char16_t variables of the same value in CharacterEncoding.cpp with it

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

(stealing)

Comment 5

[Tom S. (please needinfo tschuster)]

Comment on attachment 9014973 [details] [diff] [review]
Use precise capture-lists for the various lambdas in CharacterEncoding.cpp, rather than using [&] and capturing everything willy-nilly by reference

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

::: js/src/vm/CharacterEncoding.cpp
@@ +420,4 @@
>              ? OnUTF8Error::InsertQuestionMark
>              : OnUTF8Error::InsertReplacementCharacter;
>          size_t j = 0;
> +        auto push = [dst, &j](char16_t c) -> LoopDisposition {

No &dst? Doesn't this mean copy?

Comment 6

[Tom S. (please needinfo tschuster)]

Comment on attachment 9014973 [details] [diff] [review]
Use precise capture-lists for the various lambdas in CharacterEncoding.cpp, rather than using [&] and capturing everything willy-nilly by reference

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

Oh dst is a pointer obviously.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed81d578d775
Add a constexpr char16_t unicode::REPLACEMENT_CHARACTER, and change the two (!) separate const char16_t variables of the same value in CharacterEncoding.cpp with it.  r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/da12058b40be
Use precise capture-lists for the various lambdas in CharacterEncoding.cpp, rather than using [&] and capturing everything willy-nilly by reference.  r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/d912f4d60ea4
Remove an obsolete comment.  r=evilpie

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/ed81d578d775
https://hg.mozilla.org/mozilla-central/rev/da12058b40be
https://hg.mozilla.org/mozilla-central/rev/d912f4d60ea4