On Jan 12, 2014, at 7:08 PM, Bob Wilson <[email protected]> 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 <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
