Fix bad canonicalization for strings whose utf32->utf16 conversion is lossy.

Fix bad canonicalization for strings whose utf32->utf16 conversion is lossy.

Changes String::Equals(utf32) from string.AsUtf32().Equals(utf32) to string.Equals(utf32.AsUtf16()).

Fixes #27433.

R=hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/1cba15030716c784a040047ce6e8514883134213

[rmacnak, 2016-10-06 21:31:42]

rmacnak@google.com changed reviewers:
+ asiva@google.com, hausner@google.com

[hausner, 2016-10-06 22:14:42]

LGTM

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object_test.cc
File runtime/vm/object_test.cc (right):

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object_test.cc#n...
runtime/vm/object_test.cc:4730: VM_TEST_CASE(String_EqualsUTF32) {
Maybe a short comment what is being tested here and a reference to the bug
number?

[rmacnak, 2016-10-06 23:16:29]

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object_test.cc
File runtime/vm/object_test.cc (right):

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object_test.cc#n...
runtime/vm/object_test.cc:4730: VM_TEST_CASE(String_EqualsUTF32) {
On 2016/10/06 22:14:41, hausner wrote:
> Maybe a short comment what is being tested here and a reference to the bug
> number?

Done.

[rmacnak, 2016-10-06 23:16:56]

Description was changed from

==========
Fix bad canonicalization for strings whose utf32->utf16 conversion is lossy.

Changes String::Equals(utf32) from string.AsUtf32().Equals(utf32) to
string.Equals(utf32.AsUtf16()).

Fixes #27433.
==========

to

==========
Fix bad canonicalization for strings whose utf32->utf16 conversion is lossy.

Changes String::Equals(utf32) from string.AsUtf32().Equals(utf32) to
string.Equals(utf32.AsUtf16()).

Fixes #27433.

R=hausner@google.com

Committed:
https://github.com/dart-lang/sdk/commit/1cba15030716c784a040047ce6e8514883134213
==========

[rmacnak, 2016-10-06 23:16:57]

Committed patchset #2 (id:20001) manually as
1cba15030716c784a040047ce6e8514883134213 (presubmit successful).

[siva, 2016-10-06 23:37:24]

DBC

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:20106: if (j + 1 >= Length()) return false;
((j + 1) >= ... to maintain our style.

https://codereview.chromium.org/2400523003/diff/1/runtime/vm/object.cc#newcod...
runtime/vm/object.cc:20114: return j == Length();
The compiler maybe doing this optimization but I was wondering it it might make
sense to cache Length() in a local variable at the top
and also would this check at the start be valid?
if ((len < 0) || (len > Length())) return false;