Cleanup StringCharacterStream and add initial test cases.

BUG=

Review URL: https://chromiumcodereview.appspot.com/11438046
Patch from Dan Carney <dcarney@google.com>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13189 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/objects-inl.h

@@ -2521,14 +2521,12 @@ void String::Visit(
     String* string,
     unsigned offset,
     Visitor& visitor,
-    ConsOp& consOp,
+    ConsOp& cons_op,
     int32_t type,
     unsigned length) {
-
   ASSERT(length == static_cast<unsigned>(string->length()));
   ASSERT(offset <= length);
-
-  unsigned sliceOffset = offset;
+  unsigned slice_offset = offset;
   while (true) {
     ASSERT(type == string->map()->instance_type());
 
@@ -2536,35 +2534,36 @@ void String::Visit(
       case kSeqStringTag | kOneByteStringTag:
         visitor.VisitOneByteString(
             reinterpret_cast<const uint8_t*>(
-                SeqOneByteString::cast(string)->GetChars()) + sliceOffset,
+                SeqOneByteString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kSeqStringTag | kTwoByteStringTag:
         visitor.VisitTwoByteString(
             reinterpret_cast<const uint16_t*>(
-                SeqTwoByteString::cast(string)->GetChars()) + sliceOffset,
+                SeqTwoByteString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kExternalStringTag | kOneByteStringTag:
         visitor.VisitOneByteString(
             reinterpret_cast<const uint8_t*>(
-                ExternalAsciiString::cast(string)->GetChars()) + sliceOffset,
+                ExternalAsciiString::cast(string)->GetChars()) + slice_offset,
                 length - offset);
         return;
 
       case kExternalStringTag | kTwoByteStringTag:
         visitor.VisitTwoByteString(
             reinterpret_cast<const uint16_t*>(
-                ExternalTwoByteString::cast(string)->GetChars()) + sliceOffset,
+                ExternalTwoByteString::cast(string)->GetChars())
+                    + slice_offset,
                 length - offset);
         return;
 
       case kSlicedStringTag | kOneByteStringTag:
       case kSlicedStringTag | kTwoByteStringTag: {
         SlicedString* slicedString = SlicedString::cast(string);
-        sliceOffset += slicedString->offset();
+        slice_offset += slicedString->offset();
         string = slicedString->parent();
         type = string->map()->instance_type();
         continue;
@@ -2572,10 +2571,10 @@ void String::Visit(
 
       case kConsStringTag | kOneByteStringTag:
       case kConsStringTag | kTwoByteStringTag:
-        string = consOp.Operate(ConsString::cast(string), &offset, &type,
+        string = cons_op.Operate(ConsString::cast(string), &offset, &type,
             &length);
         if (string == NULL) return;
-        sliceOffset = offset;
+        slice_offset = offset;
         ASSERT(length == static_cast<unsigned>(string->length()));
         continue;
 
@@ -2770,34 +2769,14 @@ unsigned ConsStringIteratorOp::OffsetForDepth(unsigned depth) {
 }
 
 
-uint32_t ConsStringIteratorOp::MaskForDepth(unsigned depth) {
-  return 1 << OffsetForDepth(depth);
-}
-
-
-void ConsStringIteratorOp::SetRightDescent() {
-  trace_ |= MaskForDepth(depth_ - 1);
-}
-
-
-void ConsStringIteratorOp::ClearRightDescent() {
-  trace_ &= ~MaskForDepth(depth_ - 1);
-}
-
-
 void ConsStringIteratorOp::PushLeft(ConsString* string) {
   frames_[depth_++ & kDepthMask] = string;
 }
 
 
-void ConsStringIteratorOp::PushRight(ConsString* string, int32_t type) {
-  // Inplace update
+void ConsStringIteratorOp::PushRight(ConsString* string) {
+  // Inplace update.
   frames_[(depth_-1) & kDepthMask] = string;
-  if (depth_ != 1) return;
-  // Optimization: can replace root in this case.
-  root_ = string;
-  root_type_ = type;
-  root_length_ = string->length();
 }
 
 
@@ -2814,8 +2793,8 @@ void ConsStringIteratorOp::Pop() {
 
 
 void ConsStringIteratorOp::Reset() {
-  consumed_ = 0;
-  ResetStack();
+  depth_ = 0;
+  maximum_depth_ = 0;
 }
 
 
@@ -2824,19 +2803,13 @@ bool ConsStringIteratorOp::HasMore() {
 }
 
 
-void ConsStringIteratorOp::ResetStack() {
-  depth_ = 0;
-  maximum_depth_ = 0;
-}
-
-
 bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
-  bool blewStack;
+  bool blew_stack;
   int32_t type;
-  String* string = NextLeaf(&blewStack, &type);
+  unsigned length;
+  String* string = NextLeaf(&blew_stack, &type, &length);
   // String found.
   if (string != NULL) {
-    unsigned length = string->length();
     consumed_ += length;
     response->string_ = string;
     response->offset_ = 0;
@@ -2845,9 +2818,11 @@ bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
     return true;
   }
   // Traversal complete.
-  if (!blewStack) return false;
+  if (!blew_stack) return false;
   // Restart search.
-  ResetStack();
+  Reset();
+  // TODO(dcarney) This is unnecessary.
+  // After a reset, we don't need a String::Visit
   response->string_ = root_;
   response->offset_ = consumed_;
   response->length_ = root_length_;
@@ -2857,14 +2832,14 @@ bool ConsStringIteratorOp::ContinueOperation(ContinueResponse* response) {
 
 
 uint16_t StringCharacterStream::GetNext() {
-  ASSERT(buffer8_ != NULL);
+  ASSERT((buffer8_ == NULL && end_ == NULL) || buffer8_ < end_);
   return is_one_byte_ ? *buffer8_++ : *buffer16_++;
 }
 
 
 StringCharacterStream::StringCharacterStream(
     String* string, unsigned offset, ConsStringIteratorOp* op)
-  : is_one_byte_(true),
+  : is_one_byte_(false),
     buffer8_(NULL),
     end_(NULL),
     op_(op) {
@@ -2878,11 +2853,7 @@ bool StringCharacterStream::HasMore() {
   if (buffer8_ != end_) return true;
   if (!op_->HasMore()) return false;
   ConsStringIteratorOp::ContinueResponse response;
-  // This has been checked above
-  if (!op_->ContinueOperation(&response)) {
-    UNREACHABLE();
-    return false;
-  }
+  if (!op_->ContinueOperation(&response)) return false;
   String::Visit(response.string_,
       response.offset_, *this, *op_, response.type_, response.length_);
   return true;

src/objects.cc

@@ -7026,71 +7026,67 @@ void StringInputBuffer::Seek(unsigned pos) {
 }
 
 
-String* ConsStringIteratorOp::Operate(ConsString* consString,
-    unsigned* outerOffset, int32_t* typeOut, unsigned* lengthOut) {
-  ASSERT(*lengthOut == (unsigned)consString->length());
+String* ConsStringIteratorOp::Operate(ConsString* cons_string,
+    unsigned* offset_out, int32_t* type_out, unsigned* length_out) {
+  ASSERT(*length_out == (unsigned)cons_string->length());
+  ASSERT(depth_ == 0);
   // Push the root string.
-  PushLeft(consString);
-  root_ = consString;
-  root_type_ = *typeOut;
-  root_length_ = *lengthOut;
-  unsigned targetOffset = *outerOffset;
+  PushLeft(cons_string);
+  root_ = cons_string;
+  root_type_ = *type_out;
+  root_length_ = *length_out;
+  consumed_ = *offset_out;
+  unsigned targetOffset = *offset_out;
   unsigned offset = 0;
   while (true) {
     // Loop until the string is found which contains the target offset.
-    String* string = consString->first();
+    String* string = cons_string->first();
     unsigned length = string->length();
     int32_t type;
     if (targetOffset < offset + length) {
       // Target offset is in the left branch.
-      // Mark the descent.
-      ClearRightDescent();
       // Keep going if we're still in a ConString.
       type = string->map()->instance_type();
       if ((type & kStringRepresentationMask) == kConsStringTag) {
-        consString = ConsString::cast(string);
-        PushLeft(consString);
+        cons_string = ConsString::cast(string);
+        PushLeft(cons_string);
         continue;
       }
+      // Tell the stack we're done decending.
+      AdjustMaximumDepth();
     } else {
       // Descend right.
       // Update progress through the string.
       offset += length;
       // Keep going if we're still in a ConString.
-      string = consString->second();
+      string = cons_string->second();
       type = string->map()->instance_type();
       if ((type & kStringRepresentationMask) == kConsStringTag) {
-        consString = ConsString::cast(string);
-        PushRight(consString, type);
+        cons_string = ConsString::cast(string);
+        PushRight(cons_string);
+        // TODO(dcarney) Add back root optimization.
         continue;
       }
-      // Mark the descent.
-      SetRightDescent();
       // Need this to be updated for the current string.
       length = string->length();
       // Account for the possibility of an empty right leaf.
-      while (length == 0) {
-        bool blewStack;
-        // Need to adjust maximum depth for NextLeaf to work.
-        AdjustMaximumDepth();
-        string = NextLeaf(&blewStack, &type);
-        if (string == NULL) {
-          // Luckily, this case is impossible.
-          ASSERT(!blewStack);
+      // This happens only if we have asked for an offset outside the string.
+      if (length == 0) {
+        Reset();
         return NULL;
       }
-        length = string->length();
-      }
-    }
       // Tell the stack we're done decending.
       AdjustMaximumDepth();
+      // Pop stack so next iteration is in correct place.
+      Pop();
+    }
     ASSERT(length != 0);
     // Adjust return values and exit.
     unsigned innerOffset = targetOffset - offset;
     consumed_ += length - innerOffset;
-    *outerOffset = innerOffset;
-    *typeOut = type;
-    *lengthOut = length;
+    *offset_out = innerOffset;
+    *type_out = type;
+    *length_out = length;
     return string;
   }
   UNREACHABLE();
@@ -7098,52 +7094,49 @@ String* ConsStringIteratorOp::Operate(ConsString* consString,
 }
 
 
-String* ConsStringIteratorOp::NextLeaf(bool* blewStack, int32_t* typeOut) {
+String* ConsStringIteratorOp::NextLeaf(
+    bool* blew_stack, int32_t* type_out, unsigned* length_out) {
   while (true) {
     // Tree traversal complete.
     if (depth_ == 0) {
-      *blewStack = false;
+      *blew_stack = false;
       return NULL;
     }
     // We've lost track of higher nodes.
     if (maximum_depth_ - depth_ == kStackSize) {
-      *blewStack = true;
+      *blew_stack = true;
       return NULL;
     }
-    // Check if we're done with this level.
-    bool haveAlreadyReadRight = trace_ & MaskForDepth(depth_ - 1);
-    if (haveAlreadyReadRight) {
-      Pop();
-      continue;
-    }
     // Go right.
-    ConsString* consString = frames_[OffsetForDepth(depth_ - 1)];
-    String* string = consString->second();
+    ConsString* cons_string = frames_[OffsetForDepth(depth_ - 1)];
+    String* string = cons_string->second();
     int32_t type = string->map()->instance_type();
     if ((type & kStringRepresentationMask) != kConsStringTag) {
-      // Don't need to mark the descent here.
       // Pop stack so next iteration is in correct place.
       Pop();
-      *typeOut = type;
+      unsigned length = (unsigned) string->length();
+      // Could be a flattened ConsString.
+      if (length == 0) continue;
+      *length_out = length;
+      *type_out = type;
       return string;
     }
-    // No need to mark the descent.
-    consString = ConsString::cast(string);
-    PushRight(consString, type);
+    cons_string = ConsString::cast(string);
+    // TODO(dcarney) Add back root optimization.
+    PushRight(cons_string);
     // Need to traverse all the way left.
     while (true) {
       // Continue left.
-      // Update marker.
-      ClearRightDescent();
-      string = consString->first();
+      string = cons_string->first();
       type = string->map()->instance_type();
       if ((type & kStringRepresentationMask) != kConsStringTag) {
         AdjustMaximumDepth();
-        *typeOut = type;
+        *type_out = type;
+        *length_out = string->length();
         return string;
       }
-      consString = ConsString::cast(string);
-      PushLeft(consString);
+      cons_string = ConsString::cast(string);
+      PushLeft(cons_string);
     }
   }
   UNREACHABLE();

src/objects.h

@@ -7514,7 +7514,7 @@ class String: public HeapObject {
   static inline void Visit(String* string,
                            unsigned offset,
                            Visitor& visitor,
-                           ConsOp& consOp,
+                           ConsOp& cons_op,
                            int32_t type,
                            unsigned length);
 
@@ -7985,8 +7985,8 @@ class ConsStringIteratorOp {
     int32_t type_;
   };
   inline ConsStringIteratorOp() {}
-  String* Operate(ConsString* consString, unsigned* outerOffset,
-      int32_t* typeOut, unsigned* lengthOut);
+  String* Operate(ConsString* cons_string, unsigned* offset_out,
+      int32_t* type_out, unsigned* length_out);
   inline bool ContinueOperation(ContinueResponse* response);
   inline void Reset();
   inline bool HasMore();
@@ -7998,20 +7998,17 @@ class ConsStringIteratorOp {
   static const unsigned kDepthMask = kStackSize-1;
   STATIC_ASSERT(IS_POWER_OF_TWO(kStackSize));
   static inline unsigned OffsetForDepth(unsigned depth);
-  static inline uint32_t MaskForDepth(unsigned depth);
 
-  inline void ClearRightDescent();
-  inline void SetRightDescent();
   inline void PushLeft(ConsString* string);
-  inline void PushRight(ConsString* string, int32_t type);
+  inline void PushRight(ConsString* string);
   inline void AdjustMaximumDepth();
   inline void Pop();
-  inline void ResetStack();
-  String* NextLeaf(bool* blewStack, int32_t* typeOut);
+  String* NextLeaf(bool* blew_stack, int32_t* type_out, unsigned* length_out);
 
   unsigned depth_;
   unsigned maximum_depth_;
-  uint32_t trace_;
+  // Stack must always contain only frames for which right traversal
+  // has not yet been performed.
   ConsString* frames_[kStackSize];
   unsigned consumed_;
   ConsString* root_;