Fix bug in collector.

Small cleanups in preparser.

TEST=cctest/test-utils/SequenceCollectorRegression

Review URL: http://codereview.chromium.org/7754014

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

src/preparser.cc

@@ -1557,7 +1557,7 @@ int DuplicateFinder::AddSymbol(i::Vector<const byte> key,
                                int value) {
   uint32_t hash = Hash(key, is_ascii);
   byte* encoding = BackupKey(key, is_ascii);
-  i::HashMap::Entry* entry = map_->Lookup(encoding, hash, true);
+  i::HashMap::Entry* entry = map_.Lookup(encoding, hash, true);
   int old_value = static_cast<int>(reinterpret_cast<intptr_t>(entry->value));
   entry->value =
     reinterpret_cast<void*>(static_cast<intptr_t>(value | old_value));
@@ -1584,7 +1584,8 @@ int DuplicateFinder::AddNumber(i::Vector<const char> key, int value) {
                              i::Vector<char>(number_buffer_, kBufferSize));
     length = i::StrLength(string);
   }
-  return AddAsciiSymbol(i::Vector<const char>(string, length), value);
+  return AddSymbol(i::Vector<const byte>(reinterpret_cast<const byte*>(string),
+                                         length), true, value);
 }
 
 

src/preparser.h

@@ -61,11 +61,7 @@ class DuplicateFinder {
   explicit DuplicateFinder(i::UnicodeCache* constants)
       : unicode_constants_(constants),
         backing_store_(16),
-        map_(new i::HashMap(&Match)) { }
-
-  ~DuplicateFinder() {
-    delete map_;
-  }
+        map_(&Match) { }
 
   int AddAsciiSymbol(i::Vector<const char> key, int value);
   int AddUC16Symbol(i::Vector<const uint16_t> key, int value);
@@ -101,7 +97,7 @@ class DuplicateFinder {
   i::UnicodeCache* unicode_constants_;
   // Backing store used to store strings used as hashmap keys.
   i::SequenceCollector<unsigned char> backing_store_;
-  i::HashMap* map_;
+  i::HashMap map_;
   // Buffer used for string->number->canonical string conversions.
   char number_buffer_[kBufferSize];
 };

src/utils.h

@@ -614,17 +614,7 @@ class Collector {
         new_capacity = min_capacity + growth;
       }
     }
-    Vector<T> new_chunk = Vector<T>::New(new_capacity);
-    int new_index = PrepareGrow(new_chunk);
-    if (index_ > 0) {
-      chunks_.Add(current_chunk_.SubVector(0, index_));
-    } else {
-      // Can happen if the call to PrepareGrow moves everything into
-      // the new chunk.
-      current_chunk_.Dispose();
-    }
-    current_chunk_ = new_chunk;
-    index_ = new_index;
+    NewChunk(new_capacity);
     ASSERT(index_ + min_capacity <= current_chunk_.length());
   }
 
@@ -632,8 +622,15 @@ class Collector {
   // some of the current data into the new chunk. The function may update
   // the current index_ value to represent data no longer in the current chunk.
   // Returns the initial index of the new chunk (after copied data).
-  virtual int PrepareGrow(Vector<T> new_chunk)  {
-    return 0;
+  virtual void NewChunk(int new_capacity)  {
+    Vector<T> new_chunk = Vector<T>::New(new_capacity);
+    if (index_ > 0) {
+      chunks_.Add(current_chunk_.SubVector(0, index_));
+    } else {
+      current_chunk_.Dispose();
+    }
+    current_chunk_ = new_chunk;
+    index_ = 0;
   }
 };
 
@@ -688,20 +685,26 @@ class SequenceCollector : public Collector<T, growth_factor, max_growth> {
   int sequence_start_;
 
   // Move the currently active sequence to the new chunk.
-  virtual int PrepareGrow(Vector<T> new_chunk) {
-    if (sequence_start_ != kNoSequence) {
+  virtual void NewChunk(int new_capacity) {
+    if (sequence_start_ == kNoSequence) {
+      // Fall back on default behavior if no sequence has been started.
+      this->Collector<T, growth_factor, max_growth>::NewChunk(new_capacity);
+      return;
+    }
     int sequence_length = this->index_ - sequence_start_;
-      // The new chunk is always larger than the current chunk, so there
-      // is room for the copy.
+    Vector<T> new_chunk = Vector<T>::New(sequence_length + new_capacity);
     ASSERT(sequence_length < new_chunk.length());
     for (int i = 0; i < sequence_length; i++) {
       new_chunk[i] = this->current_chunk_[sequence_start_ + i];
     }
-      this->index_ = sequence_start_;
-      sequence_start_ = 0;
-      return sequence_length;
+    if (sequence_start_ > 0) {
+      this->chunks_.Add(this->current_chunk_.SubVector(0, sequence_start_));
+    } else {
+      this->current_chunk_.Dispose();
     }
-    return 0;
+    this->current_chunk_ = new_chunk;
+    this->index_ = sequence_length;
+    sequence_start_ = 0;
   }
 };
 

test/cctest/test-utils.cc

@@ -195,3 +195,15 @@ TEST(SequenceCollector) {
   }
   result.Dispose();
 }
+
+
+TEST(SequenceCollectorRegression) {
+  SequenceCollector<char> collector(16);
+  collector.StartSequence();
+  collector.Add('0');
+  collector.AddBlock(
+      i::Vector<const char>("12345678901234567890123456789012", 32));
+  i::Vector<char> seq = collector.EndSequence();
+  CHECK_EQ(0, strncmp("0123456789012345678901234567890123",
+                      seq.start(), seq.length()));
+}