[PATCH] Specialize string::replace (Take2)

[Paolo Carlini <pcarlini at unitus dot it>]

Hi,

this is the second take, where I have incorporated to my best Philip's 
and Nathan's (privately expressed) comments:

- For Nathan: as you can see, this version avoids /completely/ any form 
of cast (of course, no free lunch: some still happen but only behind the 
scenes, in the form of implicit basic_string::_M_rep calls ;) Do you 
like it?

- For Philip: I think I have implemented your hints about const-ness and 
base(). Ok with you?

Some tests added, tested i686-pc-linux-gnu.

Ciao, Paolo.

//////////////////

2002-03-29  Paolo Carlini  <pcarlini@unitus.it>
            Nathan Myers  <ncm@cantrip.org>
            Philip Martin  <philip@codematters.co.uk>     
       
        * include/bits/basic_string.h
        (replace(i1, i2, _CharT* k1, _CharT* k2),
        replace(i2, i2, const _CharT* k1, const _CharT* k2),
        replace(i1, i2, iterator k1, iterator k2,
        replace(i1, i2, const_iterator k1, const_iterator k2):
        New specializations to optimize for the common cases of
        pointers and iterators.
        (replace(pos, n1, s, n2)): Tweak.
        * include/bits/basic_string.tcc: Tweak comments.
        * testsuite/21_strings/replace.cc (test04): New tests.

diff -urN libstdc++-v3-orig/include/bits/basic_string.h 
libstdc++-v3/include/bits/basic_string.h
--- libstdc++-v3-orig/include/bits/basic_string.h    Wed Mar  6 22:22:52 
2002
+++ libstdc++-v3/include/bits/basic_string.h    Fri Mar 29 16:08:18 2002
@@ -647,8 +647,8 @@
         || less<const _CharT*>()(_M_data() + __size, __s))
       return _M_replace_safe(_M_ibegin() + __pos,
                  _M_ibegin() + __pos + __foldn1, __s, __s + __n2);   
-    else return this->replace(_M_check(__pos), _M_fold(__pos, __n1),
-                  __s, __s + __n2);
+    else return replace(_M_check(__pos), _M_fold(__pos, __n1),
+                basic_string(__s, __s + __n2));
       }
 
       basic_string&
@@ -682,6 +682,28 @@
         { return _M_replace(__i1, __i2, __k1, __k2,
          typename iterator_traits<_InputIterator>::iterator_category()); }
 
+      basic_string&
+      replace(iterator __i1, iterator __i2, _CharT* __k1, _CharT* __k2)
+        { return this->replace(__i1 - _M_ibegin(), __i2 - __i1,
+                   __k1, __k2 - __k1); }
+
+      basic_string&
+      replace(iterator __i1, iterator __i2, const _CharT* __k1, const 
_CharT* __k2)
+        { return this->replace(__i1 - _M_ibegin(), __i2 - __i1,
+                   __k1, __k2 - __k1); }
+
+      basic_string&
+      replace(iterator __i1, iterator __i2, iterator __k1, iterator __k2)
+        { return this->replace(__i1 - _M_ibegin(), __i2 - __i1,
+                   __k1.base(), __k2 - __k1);
+    }
+
+      basic_string&
+      replace(iterator __i1, iterator __i2, const_iterator __k1, 
const_iterator __k2)
+        { return this->replace(__i1 - _M_ibegin(), __i2 - __i1,
+                   __k1.base(), __k2 - __k1);
+    }
+
     private:
       template<class _InputIterator>
         basic_string&
@@ -690,8 +712,8 @@
 
       template<class _ForwardIterator>
         basic_string&
-        _M_replace_safe(iterator __i1, iterator __i2, _ForwardIterator 
__k1,
-           _ForwardIterator __k2);
+        _M_replace_safe(iterator __i1, iterator __i2, _ForwardIterator 
__k1,
+            _ForwardIterator __k2);
 
       // _S_construct_aux is used to implement the 21.3.1 para 15 which
       // requires special behaviour if _InIter is an integral type
diff -urN libstdc++-v3-orig/include/bits/basic_string.tcc 
libstdc++-v3/include/bits/basic_string.tcc
--- libstdc++-v3-orig/include/bits/basic_string.tcc    Tue Mar 12 
23:10:33 2002
+++ libstdc++-v3/include/bits/basic_string.tcc    Fri Mar 29 15:26:41 2002
@@ -498,13 +498,6 @@
       // else nothing (in particular, avoid calling _M_mutate() 
unnecessarily.)
     }
  
-  // This is the general replace helper, which gets instantiated both
-  // for input-iterators and forward-iterators. It buffers internally and
-  // then calls _M_replace_safe. For input-iterators this is almost the
-  // best we can do, but for forward-iterators many optimizations could be
-  // conceived: f.i., when source and destination ranges do not overlap
-  // buffering is not really needed. In order to easily implement them, it
-  // could become useful to add an _M_replace(forward_iterator_tag)
   template<typename _CharT, typename _Traits, typename _Alloc>
     template<typename _InputIter>
       basic_string<_CharT, _Traits, _Alloc>&
@@ -518,10 +511,8 @@
       }
 
   // This is a special replace helper, which does not buffer internally
-  // and can be used in the "safe" situations involving forward-iterators,
+  // and can be used in "safe" situations involving forward-iterators,
   // i.e., when source and destination ranges are known to not overlap.
-  // Presently, is called by _M_replace, by the various append and by
-  // the assigns.
   template<typename _CharT, typename _Traits, typename _Alloc>
     template<typename _ForwardIter>
       basic_string<_CharT, _Traits, _Alloc>&
diff -urN libstdc++-v3-orig/testsuite/21_strings/replace.cc 
libstdc++-v3/testsuite/21_strings/replace.cc
--- libstdc++-v3-orig/testsuite/21_strings/replace.cc    Mon Jan 14 
20:04:15 2002
+++ libstdc++-v3/testsuite/21_strings/replace.cc    Fri Mar 29 15:24:57 2002
@@ -139,10 +139,47 @@
   VERIFY(str01 == "ultra");
 }
 
+// Some more tests for
+// template<typename InputIter>
+//   string& replace(iterator it1, iterator it2, InputIter j1, 
InputIter j2)
+void
+test04()
+{
+  std::string str01 = "geogaddi";
+  std::string str02;
+
+  typedef std::string::iterator iterator;
+  typedef std::string::const_iterator const_iterator;
+ 
+  iterator it1 = str01.begin();
+  iterator it2 = str01.end();
+  str02.replace(str02.begin(), str02.end(), it1, it2);
+  VERIFY(str02 == "geogaddi");
+
+  str02 = "";
+  const_iterator c_it1 = str01.begin();
+  const_iterator c_it2 = str01.end();
+  str02.replace(str02.begin(), str02.end(), c_it1, c_it2);
+  VERIFY(str02 == "geogaddi");
+
+  str02 = "";
+  const char* c_ptr1 = str01.c_str();
+  const char* c_ptr2 = str01.c_str() + 8;
+  str02.replace(str02.begin(), str02.end(), c_ptr1, c_ptr2);
+  VERIFY(str02 == "geogaddi");
+
+  str02 = "";
+  char* ptr1 = &*str01.begin();
+  char* ptr2 = &*str01.end();
+  str02.replace(str02.begin(), str02.end(), ptr1, ptr2);
+  VERIFY(str02 == "geogaddi");
+}
+
 int main()
 {
   test01();
   test02();
   test03();
+  test04();
   return 0;
 }

Attachment: patch_replace
Description: application/java-vm