Fix input and output to handle UTF16 surrogate pairs.

src/objects.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 6491 matching lines @@
 
 class StringHasher {
  public:
   explicit inline StringHasher(int length, uint32_t seed);
 
   // Returns true if the hash of this string can be computed without
   // looking at the contents.
   inline bool has_trivial_hash();
 
   // Add a character to the hash and update the array index calculation.
-  inline void AddCharacter(uc32 c);
+  inline void AddCharacter(uint32_t c);
 
   // Adds a character to the hash but does not update the array index
   // calculation.  This can only be called when it has been verified
   // that the input is not an array index.
-  inline void AddCharacterNoIndex(uc32 c);
+  inline void AddCharacterNoIndex(uint32_t c);
+
+  // Add a character above 0xffff as a surrogate pair.  These can get into
+  // the hasher through the routines that take a UTF-8 string and make a symbol.
+  void AddSurrogatePair(uc32 c);
+  void AddSurrogatePairNoIndex(uc32 c);
 
   // Returns the value to store in the hash field of a string with
   // the given length and contents.
   uint32_t GetHashField();
 
   // Returns true if the characters seen so far make up a legal array
   // index.
   bool is_array_index() { return is_array_index_; }
 
   bool is_valid() { return is_valid_; }
@@ skipping 229 matching lines @@
   SmartArrayPointer<char> ToCString(AllowNullsFlag allow_nulls,
                                     RobustnessFlag robustness_flag,
                                     int offset,
                                     int length,
                                     int* length_output = 0);
   SmartArrayPointer<char> ToCString(
       AllowNullsFlag allow_nulls = DISALLOW_NULLS,
       RobustnessFlag robustness_flag = FAST_STRING_TRAVERSAL,
       int* length_output = 0);
 
-  inline int Utf8Length() { return Utf8Length(this, 0, length()); }
-  static int Utf8Length(String* input, int from, int to);
+  inline int Utf8Length() {

[rossberg, 2012/03/07 13:32:47]

Even with the inline definition here, I think a comment is warranted that a
negative result signals failure, and how to react to that (maybe combine with
the comment below).

[Erik Corry, 2012/03/11 19:29:22]

This issue disappears with the move to a handle based function.

+    bool dummy;
+    bool failure = false;
+    int len = Utf8Length(this, 0, length(), false, 100, &failure, &dummy);
+    if (failure) return -1;
+    return len;
+  }
+  // Call again on a flatter string if the failure variable gets set.
+  static int Utf8Length(String* input,

[rossberg, 2012/03/07 13:32:47]

Please specify the result of these functions in the case where partial
surrogates appear at either end of the string.

[Erik Corry, 2012/03/11 19:29:22]

Done.  (They are coded as 'illegal' UTF-8 sequences.  The same applies to
unpaired surrogates inside the string.)

+                        int from,
+                        int to,
+                        bool preceeded_by_surrogate,
+                        int max_recursion,

[rossberg, 2012/03/07 13:32:47]

Is this a useful interface? How is a caller supposed to pick a suitable
recursion limit? It seems to me that you could as well make it an internal
constant.

In fact, why not abstract away the flattening and retry internally, instead of
making it the responsibility of every call site? Is there a reason a caller
would want to know about this happening, or do something special? (I didn't see
anything like that in the CL.)

[Erik Corry, 2012/03/11 19:29:22]

Yes, this is the correct solution.  This means it is a function that takes a
handle, rather than an operation on the raw string pointer.  As such it moves to
handles.h instead of being in objects.h

+                        bool* failure,
+                        bool* ends_in_surrogate);
 
   // Return a 16 bit Unicode representation of the string.
   // The string should be nearly flat, otherwise the performance of
   // of this method may be very bad.  Setting robustness_flag to
   // ROBUST_STRING_TRAVERSAL invokes behaviour that is robust  This means it
   // handles unexpected data without causing assert failures and it does not
   // do any heap allocations.  This is useful when printing stack traces.
   SmartArrayPointer<uc16> ToWideCString(
       RobustnessFlag robustness_flag = FAST_STRING_TRAVERSAL);
 
@@ skipping 46 matching lines @@
   static const int kHashFieldOffset = kLengthOffset + kPointerSize;
   static const int kSize = kHashFieldOffset + kPointerSize;
 
   // Maximum number of characters to consider when trying to convert a string
   // value into an array index.
   static const int kMaxArrayIndexSize = 10;
 
   // Max ASCII char code.
   static const int kMaxAsciiCharCode = unibrow::Utf8::kMaxOneByteChar;
   static const unsigned kMaxAsciiCharCodeU = unibrow::Utf8::kMaxOneByteChar;
-  static const int kMaxUC16CharCode = 0xffff;
+  static const int kMaxUtf16CodeUnit = 0xffff;
 
   // Mask constant for checking if a string has a computed hash code
   // and if it is an array index.  The least significant bit indicates
   // whether a hash code has been computed.  If the hash code has been
   // computed the 2nd bit tells whether the string can be used as an
   // array index.
   static const int kHashNotComputedMask = 1;
   static const int kIsNotArrayIndexMask = 1 << 1;
   static const int kNofHashBitFields = 2;
 
@@ skipping 1615 matching lines @@
     } else {
       value &= ~(1 << bit_position);
     }
     return value;
   }
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_OBJECTS_H_