[Rcpp-devel] Bug: replace_last()

[Qiang Kou]

Hi, Masaki,

Please try the latest version on github. It should fix this bug.

KK

On Mon, May 16, 2016 at 9:42 PM, <teuder at gmail.com> wrote:

> Hi, Kevin and Qiang.
>
> After I sent my first post, I realized the problem as you pointed out.
>
> The problem can be resolved by replacing replace_first() and
> replace_last() to find() and rfind().
>
> -------------------
> inline String& replace_first(const char* s, const char* news) {
>    RCPP_STRING_DEBUG_2("String::replace_first(const char* = '%s' , const
> char* = '%s')", s, news);
>    if (is_na()) return *this;
>    setBuffer();
>    size_t index = buffer.find(s);
>    //size_t index = buffer.find_first_of(s);
>    if (index != std::string::npos) buffer.replace(index, strlen(s), news);
>    valid = false;
>    return *this;
> }
>
> inline String& replace_last(const char* s, const char* news) {
>    RCPP_STRING_DEBUG_2("String::replace_last(const char* = '%s' , const
> char* = '%s')", s, news);
>    if (is_na()) return *this;
>    setBuffer();
>    size_t index = buffer.rfind(s);
>    //size_t index = buffer.find_last_of(s);
>    if (index != std::string::npos) buffer.replace(index, strlen(s), news);
>    valid = false;
>    return *this;
> }
> -------------------
> > rcpp_string()
> replace_all()
> AabCDabCDA
>
> replace_first()
> AabCDABCDA
>
> replace_last()
> AABCDabCDA
> -------------------
>
> Regards
>
> Masaki
>
>
>
> Masaki Tsuda
> teuder at gmail.com
>
> 2016/05/17 5:57、Qiang Kou <qkou at umail.iu.edu> のメッセージ:
>
> Yes, Kevin,
>
> I think we need something like
>
> size_t index = std::distance(buffer.begin(), std::search(buffer.begin(),
> buffer.end(), s.begin(), s.end()));
>
> KK
>
> On Mon, May 16, 2016 at 2:04 PM, Kevin Ushey <kevinushey at gmail.com> wrote:
>
>> Thanks -- it looks to me like there is something more fundamentally
>> wrong with the 'replace_first' and 'replace_last' methods; ie, I don't
>> think we want to be using 'find_first_of' and 'find_last_of'
>> functions.
>>
>> Ie, when writing
>>
>>     string.replace_first("foo", "bar");
>>
>> I imagine we want to replace the first occurrence of 'foo' with 'bar',
>> and not the first discovered character of 'f' and 'o'.
>>
>> A simple repro for the bogus behavior:
>>
>> #include <Rcpp.h>
>> using namespace Rcpp;
>>
>> // [[Rcpp::export]]
>> String replace() {
>>   String foo("abba");
>>   return foo.replace_first("ba", "BA");
>> }
>>
>> /*** R
>> replace()
>> */
>>
>> gives
>>
>> > replace()
>> [1] "BAba"
>>
>> which is I doubt what anyone would expect.
>>
>> On Sun, May 8, 2016 at 6:14 AM, 津田真樹 <teuder at gmail.com> wrote:
>> > I find a bug in String::replace_last().
>> >
>> > I can reproduce the bug with the following code.
>> > (I also showed output of replace_first and replace_all for comparison.)
>> >
>> >
>> > =============
>> > // [[Rcpp::export]]
>> > void rcpp_string(){
>> >
>> >     String s("abcdabcd");
>> >     Rcout << "replace_first()\n";
>> >     Rcout << s.replace_first("ab", "AB").get_cstring() << "\n";
>> >     Rcout << "\n";
>> >
>> >     s="abcdabcd";
>> >     Rcout << "replace_last()\n";
>> >     Rcout << s.replace_last("ab", "AB").get_cstring()  << "\n";
>> >     Rcout << "\n";
>> >
>> >     s="abcdabcd";
>> >     Rcout << "replace_all()\n";
>> >     Rcout << s.replace_all("ab", "AB").get_cstring()  << "\n";
>> >     Rcout << "\n";
>> > }
>> > =============
>> >
>> > output
>> > =============
>> > replace_first()
>> > ABcdabcd
>> >
>> > replace_last()
>> > abcdaABd
>> >
>> > replace_all()
>> > ABcdABcd
>> > =============
>> >
>> >
>> > I also tried to fix the bug with following code. And the bug seems to
>> be fixed.
>> >
>> > =============
>> > inline String& replace_last(const char* s, const char* news) {
>> >     RCPP_STRING_DEBUG_2("String::replace_last(const char* = '%s' ,
>> const char* = '%s')", s, news);
>> >     if (is_na()) return *this;
>> >     setBuffer();
>> >     //size_t index = buffer.find_last_of(s);
>> >     size_t index = buffer.find_last_of(s) - (strlen(s)-1);
>> >     Rcout << "A" << index << "\n";
>> >     if (index != std::string::npos) buffer.replace(index, strlen(s),
>> news);
>> >     valid = false;
>> >     return *this;
>> > }
>> > =============
>> >
>> > output
>> > =============
>> > replace_first()
>> > ABcdabcd
>> >
>> > replace_last()
>> > abcdABcd
>> >
>> > replace_all()
>> > ABcdABcd
>> > =============
>> >
>> >
>> > Mac OS 10.10.5
>> > R version 3.2.3
>> > Rcpp 0.12.4
>> >
>> >
>> > _______________________________________________
>> > Rcpp-devel mailing list
>> > Rcpp-devel at lists.r-forge.r-project.org
>> > https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
>> _______________________________________________
>> Rcpp-devel mailing list
>> Rcpp-devel at lists.r-forge.r-project.org
>> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
>>
>
>
>
> --
> Qiang Kou
> qkou at umail.iu.edu
> School of Informatics and Computing, Indiana University
>
>


-- 
Qiang Kou
qkou at umail.iu.edu
School of Informatics and Computing, Indiana University
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20160516/c1b14e02/attachment-0001.html>