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

runtime/vm/unicode.h

Index: runtime/vm/unicode.h
diff --git a/runtime/vm/unicode.h b/runtime/vm/unicode.h
index 03a4b29d879898c2c83f06aac4a175d4729db209..cbbefd986e8c8c6fc65a24703f1ebf0e276f29b9 100644
--- a/runtime/vm/unicode.h
+++ b/runtime/vm/unicode.h
@@ -12,6 +12,27 @@ namespace dart {
 
 class String;
 
+class Utf : AllStatic {
+ public:
+  static bool IsLatin1(int32_t ch) {

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

for consistency can we call the parameter
"int32_t code_point" in all these functions.

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

Absolutely.  Done.

+    return (ch >= 0) && (ch <= 0xFF);
+  }
+
+  static bool IsBmp(int32_t ch) {
+    return (ch >= 0) && (ch <= 0xFFFF);
+  }
+
+  static bool IsSupplementary(int32_t ch) {
+    return (ch > 0xFFFF) && (ch <= 0x10FFFF);
+  }
+
+  // Returns true if the code point value is above Plane 17.
+  static bool IsOutOfRange(int32_t code_point) {
+    return (code_point > 0x10FFFF);

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

can we make this:
return (code_point < 0 || code_point > 0x10FFFF);

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

Done.

+  }
+};
+
+
 class Utf8 : AllStatic {
  public:
   enum Type {
@@ -20,11 +41,6 @@ class Utf8 : AllStatic {
     kSupplementary,  // Supplementary code point [U+010000, U+10FFFF].
   };
 
-  static const intptr_t kMaxOneByteChar   = 0x7F;
-  static const intptr_t kMaxTwoByteChar   = 0x7FF;
-  static const intptr_t kMaxThreeByteChar = 0xFFFF;
-  static const intptr_t kMaxFourByteChar  = 0x10FFFF;
-
   static intptr_t CodePointCount(const uint8_t* utf8_array,
                                  intptr_t array_len,
                                  Type* type);
@@ -56,26 +72,46 @@ class Utf8 : AllStatic {
                             intptr_t len);
   static bool DecodeCStringToUTF32(const char* str,
                                    int32_t* dst,
-                                   intptr_t len) {
-    ASSERT(str != NULL);
-    intptr_t array_len = strlen(str);
-    const uint8_t* utf8_array = reinterpret_cast<const uint8_t*>(str);
-    return DecodeToUTF32(utf8_array, array_len, dst, len);
+                                   intptr_t len);
+
+ private:
+  static const int32_t kMaxOneByteChar   = 0x7F;
+  static const int32_t kMaxTwoByteChar   = 0x7FF;
+  static const int32_t kMaxThreeByteChar = 0xFFFF;
+  static const int32_t kMaxFourByteChar  = 0x10FFFF;

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

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;

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

Another great idea.  Done.

+
+  static bool IsTrailByte(uint8_t code_unit) {
+    return (code_unit & 0xc0) == 0x80;
+  }
+
+  static bool IsNonShortestForm(uint32_t code_point, size_t num_code_units) {
+    return code_point < kOverlongMinimum[num_code_units];
   }
-};
 
+  static bool IsLatin1SequenceStart(uint8_t code_unit) {
+    // Check is codepoint is <= U+00FF
+    return (code_unit <= Utf8::kMaxOneByteChar);
+  }
 
-class Utf16 : AllStatic {
- public:
-  static const int32_t kMaxBmpCodepoint  = 0xFFFF;
+  static bool IsSupplementarySequenceStart(uint8_t code_unit) {
+    // Check is codepoint is >= U+10000.
+    return (code_unit >= 0xF0);
+  }
 
-  static const int32_t kLeadSurrogateOffset = (0xD800 - (0x10000 >> 10));
+  static const int8_t kTrailBytes[];
+  static const uint32_t kMagicBits[];
+  static const uint32_t kOverlongMinimum[];
+};
 
-  static const int32_t kSurrogateOffset = (0x10000 - (0xD800 << 10) - 0xDC00);
 
+class Utf16 : AllStatic {
+ private:
+  static const int32_t kMaxCodeUnit = 0xFFFF;

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

Why is this on the top here and not clubbed in with the other constants below?

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

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.

+
+ public:
   // Returns the length of the code point in UTF-16 code units.
   static intptr_t Length(int32_t ch) {
-    return (ch <= kMaxBmpCodepoint) ? 1 : 2;
+    return (ch <= Utf16::kMaxCodeUnit) ? 1 : 2;
   }
 
   // Returns true if ch is a lead or trail surrogate.
@@ -100,6 +136,11 @@ class Utf16 : AllStatic {
 
   // Encodes a single code point.
   static void Encode(int32_t codepoint, uint16_t* dst);
+
+ private:
+  static const int32_t kLeadSurrogateOffset = (0xD800 - (0x10000 >> 10));
+
+  static const int32_t kSurrogateOffset = (0x10000 - (0xD800 << 10) - 0xDC00);
 };
 
 
@@ -138,7 +179,7 @@ class CaseMapping : AllStatic {
   static const int kBlockSize = 1 << kBlockSizeLog2;
 
   static int32_t Convert(int32_t ch, int32_t mapping) {
-    if (ch <= 0xFF) {
+    if (Utf::IsLatin1(ch)) {
       int32_t info = stage2_[ch];
       if ((info & kTypeMask) == mapping) {
         ch += info >> kTypeShift;