From c1dc429c02bd8f7095c9e65844134aa298d7a14f Mon Sep 17 00:00:00 2001
From: "lrn@chromium.org"
 <lrn@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 8 Sep 2011 13:44:11 +0000
Subject: [PATCH] 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          |  5 +++--
 src/preparser.h           |  8 ++-----
 src/utils.h               | 55 +++++++++++++++++++++++++----------------------
 test/cctest/test-utils.cc | 12 +++++++++++
 4 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/src/preparser.cc b/src/preparser.cc
index 310aeac..47d21ba 100644
--- a/src/preparser.cc
+++ b/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);
 }
 
 
diff --git a/src/preparser.h b/src/preparser.h
index 010c5e2..b97b7cf 100644
--- a/src/preparser.h
+++ b/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];
 };
diff --git a/src/utils.h b/src/utils.h
index 5a3cd4c..5a875d8 100644
--- a/src/utils.h
+++ b/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) {
-      int sequence_length = this->index_ - sequence_start_;
-      // The new chunk is always larger than the current chunk, so there
-      // is room for the copy.
-      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;
+  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;
     }
-    return 0;
+    int sequence_length = this->index_ - sequence_start_;
+    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];
+    }
+    if (sequence_start_ > 0) {
+      this->chunks_.Add(this->current_chunk_.SubVector(0, sequence_start_));
+    } else {
+      this->current_chunk_.Dispose();
+    }
+    this->current_chunk_ = new_chunk;
+    this->index_ = sequence_length;
+    sequence_start_ = 0;
   }
 };
 
diff --git a/test/cctest/test-utils.cc b/test/cctest/test-utils.cc
index e136858..e4f70df 100644
--- a/test/cctest/test-utils.cc
+++ b/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()));
+}
-- 
2.7.4