Implement case mapping using the Unicode default case mapping algorithm.

Implement case mapping using the Unicode default case mapping algorithm.

BUG=5425968
Committed: https://code.google.com/p/dart/source/detail?r=616

[Ivan Posva, 2011-10-18 22:01:13]

LGTM otherwise.

-Ivan

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

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.cc#newcode5734
runtime/vm/object.cc:5734: ASSERT(!str.IsNull());
Please add a TODO in this function that outlines some of the optimizations we
could attempt to implement here.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h#newcode2306
runtime/vm/object.h:2306: virtual void SetCharAt(intptr_t index, int32_t value)
const;
This should only be accessible to very few limited implementation methods within
the String class. Can you somehow enforce that?

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h
File runtime/vm/unicode.h (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode64
runtime/vm/unicode.h:64: static const size_t kBlockSize = 1<<8;
Please name the constant 8, then you can use it below as well.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode67
runtime/vm/unicode.h:67: int16_t offset = stage1_[ch / kBlockSize] * kBlockSize;
Replace the division with a shift with a constant value? How about replacing the
multiplication with a shift as well?

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode68
runtime/vm/unicode.h:68: int32_t info = stage2_[offset + (ch % kBlockSize)];
Modulo -> and?

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode_data.cc
File runtime/vm/unicode_data.cc (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode_data.cc#newc...
runtime/vm/unicode_data.cc:23: 18, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6,
That's a lot of sixes. As discussed we should change the generating script to
provide an identity cutoff value.

[cshapiro, 2011-10-20 02:35:05]

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

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.cc#newcode5734
runtime/vm/object.cc:5734: ASSERT(!str.IsNull());
Done.  I have added a TODO here about speculation.  In the ToUpperCase and
ToLowerCase methods I have added a TODO about creating a Latin-1 fast path.  If
I missed something, let me know.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h#newcode2306
runtime/vm/object.h:2306: virtual void SetCharAt(intptr_t index, int32_t value)
const;
I have added a Transform method that is safe to use externally.  Within the
String class and its subtypes we can continue to use CharAddr.  Elsewhere, the
Transform method provides an interface for creating a string where its elements
are determined by a user-supplied mapping function.  This seems cleaner.  What
do you think?

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h
File runtime/vm/unicode.h (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode64
runtime/vm/unicode.h:64: static const size_t kBlockSize = 1<<8;
On 2011/10/18 22:01:13, Ivan Posva wrote:
> Please name the constant 8, then you can use it below as well.

Done.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode67
runtime/vm/unicode.h:67: int16_t offset = stage1_[ch / kBlockSize] * kBlockSize;
On 2011/10/18 22:01:13, Ivan Posva wrote:
> Replace the division with a shift with a constant value? How about replacing
the
> multiplication with a shift as well?

Done.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode.h#newcode68
runtime/vm/unicode.h:68: int32_t info = stage2_[offset + (ch % kBlockSize)];
On 2011/10/18 22:01:13, Ivan Posva wrote:
> Modulo -> and?

Done.

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode_data.cc
File runtime/vm/unicode_data.cc (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/unicode_data.cc#newc...
runtime/vm/unicode_data.cc:23: 18, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
6, 6, 6, 6,
Yup!  I have enhanced the table generator to prune the run of identical values
at the end of the index.

[Ivan Posva, 2011-10-21 14:03:37]

LGTM

-Ivan

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8333001/diff/1/runtime/vm/object.h#newcode2306
runtime/vm/object.h:2306: virtual void SetCharAt(intptr_t index, int32_t value)
const;
On 2011/10/20 02:35:05, cshapiro wrote:
> I have added a Transform method that is safe to use externally.  Within the
> String class and its subtypes we can continue to use CharAddr.  Elsewhere, the
> Transform method provides an interface for creating a string where its
elements
> are determined by a user-supplied mapping function.  This seems cleaner.  What
> do you think?

Looks a lot cleaner.

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5873: const OneByteString& onestr =
onestr -> result?

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5988: const TwoByteString& twostr =
ditto

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:6095: const FourByteString& fourstr =
ditto

[cshapiro, 2011-10-21 20:29:08]

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5873: const OneByteString& onestr =
Yes, that would be more consistent.  Renamed.

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:5988: const TwoByteString& twostr =
Done.

http://codereview.chromium.org/8333001/diff/6001/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:6095: const FourByteString& fourstr =
Done.