From e37f6087c6749d031b04accdc067f834f2c86ebe Mon Sep 17 00:00:00 2001
From: "kmillikin@chromium.org"
 <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 5 Mar 2009 08:10:42 +0000
Subject: [PATCH] In List::Add, correctly handle the case of adding a reference
 to a preexisting list element to a list, and to not return anything (we never
 used the return value).  Remove List::Insert, it is not currently used or
 needed.  Change List::AddBlock to take a copy of the element to be replicated
 rather than a reference.

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1424 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/list-inl.h | 27 +++++++++------------------
 src/list.h     | 14 +++++---------
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/src/list-inl.h b/src/list-inl.h
index ecc8915399..6dbd214d73 100644
--- a/src/list-inl.h
+++ b/src/list-inl.h
@@ -34,42 +34,33 @@ namespace v8 { namespace internal {
 
 
 template<typename T, class P>
-T& List<T, P>::Add(const T& element) {
-  if (length_ >= capacity_) {
+void List<T, P>::Add(const T& element) {
+  if (length_ < capacity_) {
+    data_[length_++] = element;
+  } else {
     // Grow the list capacity by 50%, but make sure to let it grow
     // even when the capacity is zero (possible initial case).
     int new_capacity = 1 + capacity_ + (capacity_ >> 1);
     T* new_data = NewData(new_capacity);
     memcpy(new_data, data_, capacity_ * sizeof(T));
+    // Since the element reference could be an element of the list,
+    // assign it to the new backing store before deleting the old.
+    new_data[length_++] = element;
     DeleteData(data_);
     data_ = new_data;
     capacity_ = new_capacity;
   }
-  return data_[length_++] = element;
 }
 
 
 template<typename T, class P>
-Vector<T> List<T, P>::AddBlock(const T& element, int count) {
+Vector<T> List<T, P>::AddBlock(T value, int count) {
   int start = length_;
-  for (int i = 0; i < count; i++)
-    Add(element);
+  for (int i = 0; i < count; i++) Add(value);
   return Vector<T>(&data_[start], count);
 }
 
 
-template<typename T, class P>
-T& List<T, P>::Insert(int i, const T& element) {
-  int free_index = length_ - 1;
-  Add(last());  // Add grows the list if necessary.
-  while (free_index > i) {
-    data_[free_index] = data_[free_index - 1];
-    free_index--;
-  }
-  data_[free_index] = element;
-}
-
-
 template<typename T, class P>
 T List<T, P>::Remove(int i) {
   T element = at(i);
diff --git a/src/list.h b/src/list.h
index 2cac21e95f..15f31fca20 100644
--- a/src/list.h
+++ b/src/list.h
@@ -53,13 +53,15 @@ class List {
   INLINE(void* operator new(size_t size)) { return P::New(size); }
   INLINE(void operator delete(void* p, size_t)) { return P::Delete(p); }
 
+  // Returns a reference to the element at index i.  This reference is
+  // not safe to use after operations that can change the list's
+  // backing store (eg, Add).
   inline T& operator[](int i) const  {
     ASSERT(0 <= i && i < length_);
     return data_[i];
   }
   inline T& at(int i) const  { return operator[](i); }
   inline T& last() const {
-    ASSERT(!is_empty());
     return at(length_ - 1);
   }
 
@@ -72,18 +74,12 @@ class List {
 
   // Adds a copy of the given 'element' to the end of the list,
   // expanding the list if necessary.
-  T& Add(const T& element);
+  void Add(const T& element);
 
   // Added 'count' elements with the value 'value' and returns a
   // vector that allows access to the elements.  The vector is valid
   // until the next change is made to this list.
-  Vector<T> AddBlock(const T& value, int count);
-
-  // Inserts a copy of the given element at index i in the list.  All
-  // elements formerly at or above i are moved up and the length of
-  // the list increases by one.  This function's complexity is linear
-  // in the size of the list.
-  T& Insert(int i, const T& element);
+  Vector<T> AddBlock(T value, int count);
 
   // Removes the i'th element without deleting it even if T is a
   // pointer type; moves all elements above i "down". Returns the