Move various top-level Unicode definitions into classes and methods.

Move various top-level Unicode definitions into classes and methods.

Committed: https://code.google.com/p/dart/source/detail?r=15568

[siva, 2012-11-30 02:42:53]

lgtm

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc#newcode34
runtime/lib/string.cc:34: if (value < 0) {
We could change this check to:
if (Utf::IsOutOfRange(value)) {

once Utf::OutOfRange includes the < 0 check.

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc#newcode41
runtime/lib/string.cc:41: if (!Utf::IsBmp(value)) {
if (Utf::IsSupplementary(value)) {
   ...
}
might make this more readable.

Actually a slightly more efficient way might be:

if (!Utf::IsLatin1(value)) {
  is_one_byte_string = false;
  if (Utf::IsSupplementary(value)) {
    utf16_len += 1;
  }
}

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

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode17
runtime/vm/unicode.h:17: static bool IsLatin1(int32_t ch) {
for consistency can we call the parameter
"int32_t code_point" in all these functions.

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode31
runtime/vm/unicode.h:31: return (code_point > 0x10FFFF);
can we make this:
return (code_point < 0 || code_point > 0x10FFFF);

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode81
runtime/vm/unicode.h:81: static const int32_t kMaxFourByteChar  = 0x10FFFF;
we could probably have a constant in class Utf which states
static const int32_t kMaxValue = 0x10FFFF;

and change this to
static const int32_t kMaxFourByteChar = Utf::kMaxValue;

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode109
runtime/vm/unicode.h:109: static const int32_t kMaxCodeUnit = 0xFFFF;
Why is this on the top here and not clubbed in with the other constants below?

[cshapiro, 2012-11-30 03:21:24]

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc#newcode34
runtime/lib/string.cc:34: if (value < 0) {
Right.  Done and done.

https://codereview.chromium.org/11414249/diff/1/runtime/lib/string.cc#newcode41
runtime/lib/string.cc:41: if (!Utf::IsBmp(value)) {
A great idea.  Done!

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

https://codereview.chromium.org/11414249/diff/1/runtime/vm/object.cc#newcode1...
runtime/vm/object.cc:10105: if (Utf::IsSupplementary(utf32_array[i])) {
I pulled this check into the preceding if statement.  Should we also assert that
the code point is not out of range?

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

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode17
runtime/vm/unicode.h:17: static bool IsLatin1(int32_t ch) {
Absolutely.  Done.

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode31
runtime/vm/unicode.h:31: return (code_point > 0x10FFFF);
Done.

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode81
runtime/vm/unicode.h:81: static const int32_t kMaxFourByteChar  = 0x10FFFF;
Another great idea.  Done.

https://codereview.chromium.org/11414249/diff/1/runtime/vm/unicode.h#newcode109
runtime/vm/unicode.h:109: static const int32_t kMaxCodeUnit = 0xFFFF;
I thought there was a scoping issue, but I was wrong.  I have relocated this
definition to the private section of the class definition.  Thanks.