Remove String::Visit

R=yangguo@chromium.org

BUG=

Review URL: https://codereview.chromium.org/254763008

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

src/objects-inl.h

@@ -3130,96 +3130,60 @@ String* String::GetUnderlying() {
 }
 
 
-template<class Visitor, class ConsOp>
-void String::Visit(
-    String* string,
-    unsigned offset,
-    Visitor& visitor,
-    ConsOp& cons_op,
-    int32_t type,
-    unsigned length) {
-  ASSERT(length == static_cast<unsigned>(string->length()));
+template<class Visitor>
+ConsString* String::VisitFlat(Visitor* visitor,
+                              String* string,
+                              const int offset) {
+  int slice_offset = offset;
+  const int length = string->length();
   ASSERT(offset <= length);
-  unsigned slice_offset = offset;
   while (true) {
-    ASSERT(type == string->map()->instance_type());
-
+    int32_t type = string->map()->instance_type();
     switch (type & (kStringRepresentationMask | kStringEncodingMask)) {
       case kSeqStringTag | kOneByteStringTag:
-        visitor.VisitOneByteString(
+        visitor->VisitOneByteString(
             SeqOneByteString::cast(string)->GetChars() + slice_offset,
             length - offset);
-        return;
+        return NULL;
 
       case kSeqStringTag | kTwoByteStringTag:
-        visitor.VisitTwoByteString(
+        visitor->VisitTwoByteString(
             SeqTwoByteString::cast(string)->GetChars() + slice_offset,
             length - offset);
-        return;
+        return NULL;
 
       case kExternalStringTag | kOneByteStringTag:
-        visitor.VisitOneByteString(
+        visitor->VisitOneByteString(
             ExternalAsciiString::cast(string)->GetChars() + slice_offset,
             length - offset);
-        return;
+        return NULL;
 
       case kExternalStringTag | kTwoByteStringTag:
-        visitor.VisitTwoByteString(
+        visitor->VisitTwoByteString(
             ExternalTwoByteString::cast(string)->GetChars() + slice_offset,
             length - offset);
-        return;
+        return NULL;
 
       case kSlicedStringTag | kOneByteStringTag:
       case kSlicedStringTag | kTwoByteStringTag: {
         SlicedString* slicedString = SlicedString::cast(string);
         slice_offset += slicedString->offset();
         string = slicedString->parent();
-        type = string->map()->instance_type();
         continue;
       }
 
       case kConsStringTag | kOneByteStringTag:
       case kConsStringTag | kTwoByteStringTag:
-        string = cons_op.Operate(string, &offset, &type, &length);
-        if (string == NULL) return;
-        slice_offset = offset;
-        ASSERT(length == static_cast<unsigned>(string->length()));
-        continue;
+        return ConsString::cast(string);
 
       default:
         UNREACHABLE();
-        return;
+        return NULL;
     }
   }
 }
 
 
-// TODO(dcarney): Remove this class after conversion to VisitFlat.
-class ConsStringCaptureOp {
- public:
-  inline ConsStringCaptureOp() : cons_string_(NULL) {}
-  inline String* Operate(String* string, unsigned*, int32_t*, unsigned*) {
-    cons_string_ = ConsString::cast(string);
-    return NULL;
-  }
-  ConsString* cons_string_;
-};
-
-
-template<class Visitor>
-ConsString* String::VisitFlat(Visitor* visitor,
-                              String* string,
-                              int offset,
-                              int length,
-                              int32_t type) {
-  ASSERT(length >= 0 && length == string->length());
-  ASSERT(offset >= 0 && offset <= length);
-  ConsStringCaptureOp op;
-  Visit(string, offset, *visitor, op, type, static_cast<unsigned>(length));
-  return op.cons_string_;
-}
-
-
 uint16_t SeqOneByteString::SeqOneByteStringGet(int index) {
   ASSERT(index >= 0 && index < length());
   return READ_BYTE_FIELD(this, kHeaderSize + index * kCharSize);
@@ -3399,12 +3363,7 @@ const uint16_t* ExternalTwoByteString::ExternalTwoByteStringGetData(
 }
 
 
-String* ConsStringNullOp::Operate(String*, unsigned*, int32_t*, unsigned*) {
-  return NULL;
-}
-
-
-unsigned ConsStringIteratorOp::OffsetForDepth(unsigned depth) {
+int ConsStringIteratorOp::OffsetForDepth(int depth) {
   return depth & kDepthMask;
 }
 
@@ -3432,45 +3391,9 @@ void ConsStringIteratorOp::Pop() {
 }
 
 
-bool ConsStringIteratorOp::HasMore() {
-  return depth_ != 0;
-}
-
-
-void ConsStringIteratorOp::Reset() {
-  depth_ = 0;
-}
-
-
-String* ConsStringIteratorOp::ContinueOperation(int32_t* type_out,
-                                                unsigned* length_out) {
-  bool blew_stack = false;
-  String* string = NextLeaf(&blew_stack, type_out, length_out);
-  // String found.
-  if (string != NULL) {
-    // Verify output.
-    ASSERT(*length_out == static_cast<unsigned>(string->length()));
-    ASSERT(*type_out == string->map()->instance_type());
-    return string;
-  }
-  // Traversal complete.
-  if (!blew_stack) return NULL;
-  // Restart search from root.
-  unsigned offset_out;
-  string = Search(&offset_out, type_out, length_out);
-  // Verify output.
-  ASSERT(string == NULL || offset_out == 0);
-  ASSERT(string == NULL ||
-         *length_out == static_cast<unsigned>(string->length()));
-  ASSERT(string == NULL || *type_out == string->map()->instance_type());
-  return string;
-}
-
-
 uint16_t StringCharacterStream::GetNext() {
   ASSERT(buffer8_ != NULL && end_ != NULL);
   // Advance cursor if needed.
-  // TODO(dcarney): Ensure uses of the api call HasMore first and avoid this.
   if (buffer8_ == end_) HasMore();
   ASSERT(buffer8_ < end_);
   return is_one_byte_ ? *buffer8_++ : *buffer16_++;
@@ -3479,41 +3402,39 @@ uint16_t StringCharacterStream::GetNext() {
 
 StringCharacterStream::StringCharacterStream(String* string,
                                              ConsStringIteratorOp* op,
-                                             unsigned offset)
+                                             int offset)
   : is_one_byte_(false),
     op_(op) {
   Reset(string, offset);
 }
 
 
-void StringCharacterStream::Reset(String* string, unsigned offset) {
-  op_->Reset();
+void StringCharacterStream::Reset(String* string, int offset) {
   buffer8_ = NULL;
   end_ = NULL;
-  int32_t type = string->map()->instance_type();
-  unsigned length = string->length();
-  String::Visit(string, offset, *this, *op_, type, length);
+  ConsString* cons_string = String::VisitFlat(this, string, offset);
+  op_->Reset(cons_string, offset);
+  if (cons_string != NULL) {
+    string = op_->Next(&offset);
+    if (string != NULL) String::VisitFlat(this, string, offset);
+  }
 }
 
 
 bool StringCharacterStream::HasMore() {
   if (buffer8_ != end_) return true;
-  if (!op_->HasMore()) return false;
-  unsigned length;
-  int32_t type;
-  String* string = op_->ContinueOperation(&type, &length);
+  int offset;
+  String* string = op_->Next(&offset);
+  ASSERT_EQ(offset, 0);
   if (string == NULL) return false;
-  ASSERT(!string->IsConsString());
-  ASSERT(string->length() != 0);
-  ConsStringNullOp null_op;
-  String::Visit(string, 0, *this, null_op, type, length);
+  String::VisitFlat(this, string);
   ASSERT(buffer8_ != end_);
   return true;
 }
 
 
 void StringCharacterStream::VisitOneByteString(
-    const uint8_t* chars, unsigned length) {
+    const uint8_t* chars, int length) {
   is_one_byte_ = true;
   buffer8_ = chars;
   end_ = chars + length;
@@ -3521,7 +3442,7 @@ void StringCharacterStream::VisitOneByteString(
 
 
 void StringCharacterStream::VisitTwoByteString(
-    const uint16_t* chars, unsigned length) {
+    const uint16_t* chars, int length) {
   is_one_byte_ = false;
   buffer16_ = chars;
   end_ = reinterpret_cast<const uint8_t*>(chars + length);

src/objects.h

@@ -9209,29 +9209,10 @@ class String: public Name {
     return NonOneByteStart(chars, length) >= length;
   }
 
-  // TODO(dcarney): Replace all instances of this with VisitFlat.
-  template<class Visitor, class ConsOp>
-  static inline void Visit(String* string,
-                           unsigned offset,
-                           Visitor& visitor,
-                           ConsOp& cons_op,
-                           int32_t type,
-                           unsigned length);
-
-  template<class Visitor>
-  static inline ConsString* VisitFlat(Visitor* visitor,
-                                      String* string,
-                                      int offset,
-                                      int length,
-                                      int32_t type);
-
   template<class Visitor>
   static inline ConsString* VisitFlat(Visitor* visitor,
                                       String* string,
-                                      int offset = 0) {
-    int32_t type = string->map()->instance_type();
-    return VisitFlat(visitor, string, offset, string->length(), type);
-  }
+                                      int offset = 0);
 
   static Handle<FixedArray> CalculateLineEnds(Handle<String> string,
                                               bool include_ending_line);
@@ -9617,57 +9598,63 @@ class ConsStringNullOp {
 // This maintains an off-stack representation of the stack frames required
 // to traverse a ConsString, allowing an entirely iterative and restartable
 // traversal of the entire string
-// Note: this class is not GC-safe.
 class ConsStringIteratorOp {
  public:
   inline ConsStringIteratorOp() {}
-  String* Operate(String* string,
-                  unsigned* offset_out,
-                  int32_t* type_out,
-                  unsigned* length_out);
-  inline String* ContinueOperation(int32_t* type_out, unsigned* length_out);
-  inline void Reset();
-  inline bool HasMore();
+  inline ConsStringIteratorOp(ConsString* cons_string, int offset = 0) {
+    Reset(cons_string, offset);
+  }
+  inline void Reset(ConsString* cons_string, int offset = 0) {
+    depth_ = 0;
+    // Next will always return NULL.
+    if (cons_string == NULL) return;
+    Initialize(cons_string, offset);
+  }
+  // Returns NULL when complete.
+  inline String* Next(int* offset_out) {
+    *offset_out = 0;
+    if (depth_ == 0) return NULL;
+    return Continue(offset_out);
+  }
 
  private:
-  // TODO(dcarney): Templatize this out for different stack sizes.
-  static const unsigned kStackSize = 32;
+  static const int kStackSize = 32;
   // Use a mask instead of doing modulo operations for stack wrapping.
-  static const unsigned kDepthMask = kStackSize-1;
+  static const int kDepthMask = kStackSize-1;
   STATIC_ASSERT(IS_POWER_OF_TWO(kStackSize));
-  static inline unsigned OffsetForDepth(unsigned depth);
+  static inline int OffsetForDepth(int depth);
 
   inline void PushLeft(ConsString* string);
   inline void PushRight(ConsString* string);
   inline void AdjustMaximumDepth();
   inline void Pop();
-  String* NextLeaf(bool* blew_stack, int32_t* type_out, unsigned* length_out);
-  String* Search(unsigned* offset_out,
-                 int32_t* type_out,
-                 unsigned* length_out);
+  inline bool StackBlown() { return maximum_depth_ - depth_ == kStackSize; }
+  void Initialize(ConsString* cons_string, int offset);
+  String* Continue(int* offset_out);
+  String* NextLeaf(bool* blew_stack);
+  String* Search(int* offset_out);
 
-  unsigned depth_;
-  unsigned maximum_depth_;
   // Stack must always contain only frames for which right traversal
   // has not yet been performed.
   ConsString* frames_[kStackSize];
-  unsigned consumed_;
   ConsString* root_;
+  int depth_;
+  int maximum_depth_;
+  int consumed_;
   DISALLOW_COPY_AND_ASSIGN(ConsStringIteratorOp);
 };
 
 
-// Note: this class is not GC-safe.
 class StringCharacterStream {
  public:
   inline StringCharacterStream(String* string,
                                ConsStringIteratorOp* op,
-                               unsigned offset = 0);
+                               int offset = 0);
   inline uint16_t GetNext();
   inline bool HasMore();
-  inline void Reset(String* string, unsigned offset = 0);
-  inline void VisitOneByteString(const uint8_t* chars, unsigned length);
-  inline void VisitTwoByteString(const uint16_t* chars, unsigned length);
+  inline void Reset(String* string, int offset = 0);
+  inline void VisitOneByteString(const uint8_t* chars, int length);
+  inline void VisitTwoByteString(const uint16_t* chars, int length);
 
  private:
   bool is_one_byte_;

test/cctest/test-strings.cc

@@ -230,11 +230,11 @@ class ConsStringStats {
   }
   void Reset();
   void VerifyEqual(const ConsStringStats& that) const;
-  unsigned leaves_;
-  unsigned empty_leaves_;
-  unsigned chars_;
-  unsigned left_traversals_;
-  unsigned right_traversals_;
+  int leaves_;
+  int empty_leaves_;
+  int chars_;
+  int left_traversals_;
+  int right_traversals_;
  private:
   DISALLOW_COPY_AND_ASSIGN(ConsStringStats);
 };
@@ -250,11 +250,11 @@ void ConsStringStats::Reset() {
 
 
 void ConsStringStats::VerifyEqual(const ConsStringStats& that) const {
-  CHECK(this->leaves_ == that.leaves_);
-  CHECK(this->empty_leaves_ == that.empty_leaves_);
-  CHECK(this->chars_ == that.chars_);
-  CHECK(this->left_traversals_ == that.left_traversals_);
-  CHECK(this->right_traversals_ == that.right_traversals_);
+  CHECK_EQ(this->leaves_, that.leaves_);
+  CHECK_EQ(this->empty_leaves_, that.empty_leaves_);
+  CHECK_EQ(this->chars_, that.chars_);
+  CHECK_EQ(this->left_traversals_, that.left_traversals_);
+  CHECK_EQ(this->right_traversals_, that.right_traversals_);
 }
 
 
@@ -270,14 +270,14 @@ class ConsStringGenerationData {
   double leftness_;
   double rightness_;
   double empty_leaf_threshold_;
-  unsigned max_leaves_;
+  int max_leaves_;
   // Cached data.
   Handle<String> building_blocks_[kNumberOfBuildingBlocks];
   String* empty_string_;
   MyRandomNumberGenerator rng_;
   // Stats.
   ConsStringStats stats_;
-  unsigned early_terminations_;
+  int early_terminations_;
  private:
   DISALLOW_COPY_AND_ASSIGN(ConsStringGenerationData);
 };
@@ -356,23 +356,14 @@ void AccumulateStats(Handle<String> cons_string, ConsStringStats* stats) {
 
 void AccumulateStatsWithOperator(
     ConsString* cons_string, ConsStringStats* stats) {
-  unsigned offset = 0;
-  int32_t type = cons_string->map()->instance_type();
-  unsigned length = static_cast<unsigned>(cons_string->length());
-  ConsStringIteratorOp op;
-  String* string = op.Operate(cons_string, &offset, &type, &length);
-  CHECK(string != NULL);
-  while (true) {
-    ASSERT(!string->IsConsString());
+  ConsStringIteratorOp op(cons_string);
+  String* string;
+  int offset;
+  while (NULL != (string = op.Next(&offset))) {
     // Accumulate stats.
+    CHECK_EQ(0, offset);
     stats->leaves_++;
     stats->chars_ += string->length();
-    // Check for completion.
-    bool keep_going_fast_check = op.HasMore();
-    string = op.ContinueOperation(&type, &length);
-    if (string == NULL) return;
-    // Verify no false positives for fast check.
-    CHECK(keep_going_fast_check);
   }
 }
 
@@ -380,7 +371,7 @@ void AccumulateStatsWithOperator(
 void VerifyConsString(Handle<String> root, ConsStringGenerationData* data) {
   // Verify basic data.
   CHECK(root->IsConsString());
-  CHECK(static_cast<unsigned>(root->length()) == data->stats_.chars_);
+  CHECK_EQ(root->length(), data->stats_.chars_);
   // Recursive verify.
   ConsStringStats stats;
   AccumulateStats(ConsString::cast(*root), &stats);
@@ -625,9 +616,9 @@ static void VerifyCharacterStream(
     // Want to test the offset == length case.
     if (offset > length) offset = length;
     StringCharacterStream flat_stream(
-        flat_string, &cons_string_iterator_op_1, static_cast<unsigned>(offset));
+        flat_string, &cons_string_iterator_op_1, offset);
     StringCharacterStream cons_stream(
-        cons_string, &cons_string_iterator_op_2, static_cast<unsigned>(offset));
+        cons_string, &cons_string_iterator_op_2, offset);
     for (int i = offset; i < length; i++) {
       uint16_t c = flat_string->Get(i);
       CHECK(flat_stream.HasMore());