[libcxx] r198878 - Fix PR18404 - 'Bug in regex_token_iterator::operator++(int) implementation'. Enhance the tests for regex_token_iterator and regex_iterator.

Marshall Clow mclow.lists at gmail.com
Mon Jan 13 09:52:26 PST 2014


On Jan 12, 2014, at 7:08 PM, Bob Wilson <bob.wilson at apple.com> wrote:

> Marshall,
> 
> This change appears to have broken the libcxx buildbot.
> http://lab.llvm.org:8013/builders/libcxx_clang-x86_64-darwin11-RA/builds/587
> 
> It has been failing overall for a while for a different reason, so the problem wasn’t reported in the usual way. The test.libcxx.system failures are old, but the test.libcxx.new failure is new in build 587.  Since it appears to be testing the result of a regexp operator++, I’m pretty sure it is related to your change here.

Yeah, it is. However, it’s a real bug, not a problem with the test.

I should have caught it before I committed, but apparently I made a change w/o running the tests. (Bad Marshall! No biscuit!)
I changed:
	i3 = i;
	i++;
to:
	i3 = i++;

(and I could have _sworn_ that I re-ran the tests, but apparently not).

This exposed a bug in the copy constructor of regex_token_iterator, (a == where it should have been an =), which I had already fixed in the assignment operator.

Committed revision 199122 to fix this.
Sorry for the inconvenience.

— Marshall


> 
> On Jan 9, 2014, at 10:25 AM, Marshall Clow <mclow.lists at gmail.com> wrote:
> 
>> Author: marshall
>> Date: Thu Jan  9 12:25:57 2014
>> New Revision: 198878
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=198878&view=rev
>> Log:
>> Fix PR18404 - 'Bug in regex_token_iterator::operator++(int) implementation'. Enhance the tests for regex_token_iterator and regex_iterator.
>> 
>> Modified:
>>   libcxx/trunk/include/regex
>>   libcxx/trunk/test/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp
>>   libcxx/trunk/test/re/re.iter/re.tokiter/re.tokiter.incr/post.pass.cpp
>> 
>> Modified: libcxx/trunk/include/regex
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/regex?rev=198878&r1=198877&r2=198878&view=diff
>> ==============================================================================
>> --- libcxx/trunk/include/regex (original)
>> +++ libcxx/trunk/include/regex Thu Jan  9 12:25:57 2014
>> @@ -6188,6 +6188,12 @@ public:
>> 
>> private:
>>    void __init(_BidirectionalIterator __a, _BidirectionalIterator __b);
>> +    void __establish_result () {
>> +        if (__subs_[_N_] == -1)
>> +            __result_ = &__position_->prefix();
>> +        else
>> +            __result_ = &(*__position_)[__subs_[_N_]];
>> +        }       
>> };
>> 
>> template <class _BidirectionalIterator, class _CharT, class _Traits>
>> @@ -6205,12 +6211,7 @@ regex_token_iterator<_BidirectionalItera
>>    __init(_BidirectionalIterator __a, _BidirectionalIterator __b)
>> {
>>    if (__position_ != _Position())
>> -    {
>> -        if (__subs_[_N_] == -1)
>> -            __result_ = &__position_->prefix();
>> -        else
>> -            __result_ = &(*__position_)[__subs_[_N_]];
>> -    }
>> +        __establish_result ();
>>    else if (__subs_[_N_] == -1)
>>    {
>>        __suffix_.matched = true;
>> @@ -6288,6 +6289,8 @@ regex_token_iterator<_BidirectionalItera
>> {
>>    if (__x.__result_ == &__x.__suffix_)
>>        __result_ == &__suffix_;
>> +    else if ( __result_ != nullptr )
>> +        __establish_result ();
>> }
>> 
>> template <class _BidirectionalIterator, class _CharT, class _Traits>
>> @@ -6299,12 +6302,15 @@ regex_token_iterator<_BidirectionalItera
>>    {
>>        __position_ = __x.__position_;
>>        if (__x.__result_ == &__x.__suffix_)
>> -            __result_ == &__suffix_;
>> +            __result_ = &__suffix_;
>>        else
>>            __result_ = __x.__result_;
>>        __suffix_ = __x.__suffix_;
>>        _N_ = __x._N_;
>>        __subs_ = __x.__subs_;
>> +
>> +        if ( __result_ != nullptr && __result_ != &__suffix_ )
>> +            __establish_result();
>>    }
>>    return *this;
>> }
>> @@ -6337,22 +6343,14 @@ regex_token_iterator<_BidirectionalItera
>>    else if (_N_ + 1 < __subs_.size())
>>    {
>>        ++_N_;
>> -        if (__subs_[_N_] == -1)
>> -            __result_ = &__position_->prefix();
>> -        else
>> -            __result_ = &(*__position_)[__subs_[_N_]];
>> +        __establish_result();
>>    }
>>    else
>>    {
>>        _N_ = 0;
>>        ++__position_;
>>        if (__position_ != _Position())
>> -        {
>> -            if (__subs_[_N_] == -1)
>> -                __result_ = &__position_->prefix();
>> -            else
>> -                __result_ = &(*__position_)[__subs_[_N_]];
>> -        }
>> +            __establish_result();
>>        else
>>        {
>>            if (_VSTD::find(__subs_.begin(), __subs_.end(), -1) != __subs_.end()
>> 
>> Modified: libcxx/trunk/test/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp?rev=198878&r1=198877&r2=198878&view=diff
>> ==============================================================================
>> --- libcxx/trunk/test/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp (original)
>> +++ libcxx/trunk/test/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp Thu Jan  9 12:25:57 2014
>> @@ -22,21 +22,76 @@ int main()
>>        std::regex phone_numbers("\\d{3}-\\d{4}");
>>        const char phone_book[] = "555-1234, 555-2345, 555-3456";
>>        std::cregex_iterator i(std::begin(phone_book), std::end(phone_book), phone_numbers);
>> +        std::cregex_iterator i2 = i;
>>        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>>        assert((*i).size() == 1);
>>        assert((*i).position() == 0);
>>        assert((*i).str() == "555-1234");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>>        i++;
>>        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>>        assert((*i).size() == 1);
>>        assert((*i).position() == 10);
>>        assert((*i).str() == "555-2345");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>>        i++;
>>        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>>        assert((*i).size() == 1);
>>        assert((*i).position() == 20);
>>        assert((*i).str() == "555-3456");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>>        i++;
>>        assert(i == std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>> +    }
>> +    {
>> +        std::regex phone_numbers("\\d{3}-\\d{4}");
>> +        const char phone_book[] = "555-1234, 555-2345, 555-3456";
>> +        std::cregex_iterator i(std::begin(phone_book), std::end(phone_book), phone_numbers);
>> +        std::cregex_iterator i2 = i;
>> +        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>> +        assert((*i).size() == 1);
>> +        assert((*i).position() == 0);
>> +        assert((*i).str() == "555-1234");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>> +        ++i;
>> +        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>> +        assert((*i).size() == 1);
>> +        assert((*i).position() == 10);
>> +        assert((*i).str() == "555-2345");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>> +        ++i;
>> +        assert(i != std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>> +        assert((*i).size() == 1);
>> +        assert((*i).position() == 20);
>> +        assert((*i).str() == "555-3456");
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>> +        ++i;
>> +        assert(i == std::cregex_iterator());
>> +        assert(i2!= std::cregex_iterator());
>> +        assert((*i2).size() == 1);
>> +        assert((*i2).position() == 0);
>> +        assert((*i2).str() == "555-1234");
>>    }
>> }
>> 
>> Modified: libcxx/trunk/test/re/re.iter/re.tokiter/re.tokiter.incr/post.pass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/re/re.iter/re.tokiter/re.tokiter.incr/post.pass.cpp?rev=198878&r1=198877&r2=198878&view=diff
>> ==============================================================================
>> --- libcxx/trunk/test/re/re.iter/re.tokiter/re.tokiter.incr/post.pass.cpp (original)
>> +++ libcxx/trunk/test/re/re.iter/re.tokiter/re.tokiter.incr/post.pass.cpp Thu Jan  9 12:25:57 2014
>> @@ -23,19 +23,82 @@ int main()
>>        const char phone_book[] = "start 555-1234, 555-2345, 555-3456 end";
>>        std::cregex_token_iterator i(std::begin(phone_book), std::end(phone_book)-1,
>>                                     phone_numbers, -1);
>> -        assert(i != std::cregex_token_iterator());
>> +        std::cregex_token_iterator i2 = i;
>> +        std::cregex_token_iterator i3;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>>        assert(i->str() == "start ");
>> -        i++;
>> -        assert(i != std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        i3 = i++;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>>        assert(i->str() == ", ");
>> -        i++;
>> -        assert(i != std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == "start ");
>> +        i3 = i++;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>>        assert(i->str() == ", ");
>> -        i++;
>> -        assert(i != std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == ", ");
>> +        i3 = i++;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>>        assert(i->str() == " end");
>> -        i++;
>> -        assert(i == std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == ", ");
>> +        i3 = i++;
>> +        assert(i  == std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == " end");
>> +    }
>> +    {
>> +        std::regex phone_numbers("\\d{3}-\\d{4}");
>> +        const char phone_book[] = "start 555-1234, 555-2345, 555-3456 end";
>> +        std::cregex_token_iterator i(std::begin(phone_book), std::end(phone_book)-1,
>> +                                     phone_numbers, -1);
>> +        std::cregex_token_iterator i2 = i;
>> +        std::cregex_token_iterator i3;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i->str() == "start ");
>> +        assert(i2->str() == "start ");
>> +        i3 = i;
>> +        ++i;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>> +        assert(i->str() == ", ");
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == "start ");
>> +        i3 = i;
>> +        ++i;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>> +        assert(i->str() == ", ");
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == ", ");
>> +        i3 = i;
>> +        ++i;
>> +        assert(i  != std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>> +        assert(i->str() == " end");
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == ", ");
>> +        i3 = i;
>> +        ++i;
>> +        assert(i  == std::cregex_token_iterator());
>> +        assert(i2 != std::cregex_token_iterator());
>> +        assert(i3 != std::cregex_token_iterator());
>> +        assert(i2->str() == "start ");
>> +        assert(i3->str() == " end");
>>    }
>>    {
>>        std::regex phone_numbers("\\d{3}-\\d{4}");
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 





More information about the cfe-commits mailing list