| Message ID | e54204f1-ece7-319d-095e-a5380f5a262c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | New istreambuf_iterator debug check | expand |
On Wed, 24 Jan 2018 17:39:59 +0100 François Dumont <frs.dumont@gmail.com> wrote: > Hi > > I'd like to propose this new debug check. Comparing with non-eos > istreambuf_iterator sounds like an obvious coding mistake. > > I propose it despite the stage 1 as it is just a new debug check, > it doesn't impact the lib in normal mode. > > Tested under Linux x86_64, ok to commit ? > > François > bool equal(const istreambuf_iterator& __b) const - { return _M_at_eof() == __b._M_at_eof(); } + { + bool __this_at_eof = _M_at_eof(); + bool __b_at_eof = __b._M_at_eof(); + + __glibcxx_requires_cond(__this_at_eof || __b_at_eof, _M_message( + "Abnormal comparison to non-end-of-stream istreambuf_iterator")); + return __this_at_eof == __b_at_eof; + } Looks strange for me. It is legal and possible that istreambuf_iterator will be in EOF state. -- - ptr
On 24/01/2018 18:53, Petr Ovtchenkov wrote: > On Wed, 24 Jan 2018 17:39:59 +0100 > François Dumont <frs.dumont@gmail.com> wrote: > >> Hi >> >> I'd like to propose this new debug check. Comparing with non-eos >> istreambuf_iterator sounds like an obvious coding mistake. >> >> I propose it despite the stage 1 as it is just a new debug check, >> it doesn't impact the lib in normal mode. >> >> Tested under Linux x86_64, ok to commit ? >> >> François >> > bool > equal(const istreambuf_iterator& __b) const > - { return _M_at_eof() == __b._M_at_eof(); } > + { > + bool __this_at_eof = _M_at_eof(); > + bool __b_at_eof = __b._M_at_eof(); > + > + __glibcxx_requires_cond(__this_at_eof || __b_at_eof, _M_message( > + "Abnormal comparison to non-end-of-stream istreambuf_iterator")); > + return __this_at_eof == __b_at_eof; > + } > > Looks strange for me. It is legal and possible that istreambuf_iterator > will be in EOF state. > Sure, but consider rather the associated 3_neg.cc showing the debug check purpose: cistreambuf_iter it1(istrs), it2(istrs); it1 == it2; // No sens
On Wed, 24 Jan 2018 21:34:48 +0100 François Dumont <frs.dumont@gmail.com> wrote: > On 24/01/2018 18:53, Petr Ovtchenkov wrote: > > On Wed, 24 Jan 2018 17:39:59 +0100 > > François Dumont <frs.dumont@gmail.com> wrote: > > > >> Hi > >> > >> I'd like to propose this new debug check. Comparing with non-eos > >> istreambuf_iterator sounds like an obvious coding mistake. > >> > >> I propose it despite the stage 1 as it is just a new debug check, > >> it doesn't impact the lib in normal mode. > >> > >> Tested under Linux x86_64, ok to commit ? > >> > >> François > >> > > bool > > equal(const istreambuf_iterator& __b) const > > - { return _M_at_eof() == __b._M_at_eof(); } > > + { > > + bool __this_at_eof = _M_at_eof(); > > + bool __b_at_eof = __b._M_at_eof(); > > + > > + __glibcxx_requires_cond(__this_at_eof || __b_at_eof, _M_message( > > + "Abnormal comparison to non-end-of-stream istreambuf_iterator")); > > + return __this_at_eof == __b_at_eof; > > + } > > > > Looks strange for me. It is legal and possible that istreambuf_iterator > > will be in EOF state. > > > Sure, but consider rather the associated 3_neg.cc showing the debug > check purpose: > > cistreambuf_iter it1(istrs), it2(istrs); > it1 == it2; // No sens > This is what author want to say. Neveretheless, __glibcxx_requires_cond(__this_at_eof || __b_at_eof, ... in equal looks bogus for me. -- - ptr
On 24/01/18 17:39 +0100, François Dumont wrote: >Hi > > I'd like to propose this new debug check. Comparing with non-eos >istreambuf_iterator sounds like an obvious coding mistake. Agreed, but that doesn't mean we can terminate the process. It's still valid C++, even though it's probably not what the author intended to do.
diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h index 292ef3a..2e46771 100644 --- a/libstdc++-v3/include/bits/streambuf_iterator.h +++ b/libstdc++-v3/include/bits/streambuf_iterator.h @@ -174,7 +174,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Return true both iterators are end or both are not end. bool equal(const istreambuf_iterator& __b) const - { return _M_at_eof() == __b._M_at_eof(); } + { + bool __this_at_eof = _M_at_eof(); + bool __b_at_eof = __b._M_at_eof(); + + __glibcxx_requires_cond(__this_at_eof || __b_at_eof, _M_message( + "Abnormal comparison to non-end-of-stream istreambuf_iterator")); + return __this_at_eof == __b_at_eof; + } private: int_type diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/1.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/1.cc new file mode 100644 index 0000000..bb2e6f8 --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/1.cc @@ -0,0 +1,50 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +#include <sstream> +#include <iterator> + +#include <testsuite_hooks.h> + +void test01() +{ + std::istringstream istrs("123456789"); + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter it(istrs); + cistreambuf_iter eof; + VERIFY( !it.equal(eof) ); + VERIFY( !eof.equal(it) ); + VERIFY( it != eof ); + VERIFY( eof != it ); +} + +void test02() +{ + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter eof; + VERIFY( eof.equal(eof) ); + VERIFY( eof == eof ); +} + +int main() +{ + test01(); + test02(); + return 0; +} diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc index 3fe1cf1..47a1a00 100644 --- a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/2.cc @@ -47,7 +47,10 @@ void test02(void) cistreambuf_iter istrb_it05(istrs01); cistreambuf_iter istrb_it06(istrs01.rdbuf()); + +#ifndef _GLIBCXX_DEBUG VERIFY( istrb_it05 == istrb_it06 ); +#endif // bool equal(istreambuf_iter& b) cistreambuf_iter istrb_it07(0); @@ -57,12 +60,14 @@ void test02(void) cistreambuf_iter istrb_it10; VERIFY( istrb_it10.equal(istrb_it09) ); +#ifndef _GLIBCXX_DEBUG cistreambuf_iter istrb_it11(istrs01); cistreambuf_iter istrb_it12(istrs01.rdbuf()); VERIFY( istrb_it11.equal(istrb_it12) ); cistreambuf_iter istrb_it13(istrs01); cistreambuf_iter istrb_it14(istrs01.rdbuf()); VERIFY( istrb_it14.equal(istrb_it13) ); +#endif cistreambuf_iter istrb_it15(istrs01); cistreambuf_iter istrb_it16; @@ -77,9 +82,11 @@ void test02(void) cistreambuf_iter istrb_it20; VERIFY( istrb_it19 == istrb_it20 ); +#ifndef _GLIBCXX_DEBUG cistreambuf_iter istrb_it21(istrs01); cistreambuf_iter istrb_it22(istrs01.rdbuf()); VERIFY( istrb_it22 == istrb_it21 ); +#endif cistreambuf_iter istrb_it23(istrs01); cistreambuf_iter istrb_it24; diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/3_neg.cc b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/3_neg.cc new file mode 100644 index 0000000..5e75704 --- /dev/null +++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/debug/3_neg.cc @@ -0,0 +1,37 @@ +// Copyright (C) 2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-do run { xfail *-*-* } } +// { dg-require-debug-mode "" } + +#include <sstream> +#include <iterator> + +void test01() +{ + std::istringstream istrs("123456789"); + typedef std::istreambuf_iterator<char> cistreambuf_iter; + + cistreambuf_iter it1(istrs), it2(istrs); + it1 == it2; // No sens +} + +int main() +{ + test01(); + return 0; +} diff --git a/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/char/1.cc b/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/char/1.cc index 0b2a8fb..fa5d314 100644 --- a/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/char/1.cc +++ b/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/char/1.cc @@ -36,7 +36,7 @@ void test01() in_iterator_type end1, it1; it1 = find(beg1, beg1, 'l'); - VERIFY( it1 == beg1 ); + VERIFY( it1 != end1 ); VERIFY( *it1 == 'D' ); it1 = find(end1, end1, 'D'); diff --git a/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/wchar_t/1.cc b/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/wchar_t/1.cc index bfb476b..84766fb 100644 --- a/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/wchar_t/1.cc +++ b/libstdc++-v3/testsuite/25_algorithms/find/istreambuf_iterators/wchar_t/1.cc @@ -36,7 +36,7 @@ void test01() in_iterator_type end1, it1; it1 = find(beg1, beg1, L'l'); - VERIFY( it1 == beg1 ); + VERIFY( it1 != end1 ); VERIFY( *it1 == L'D' ); it1 = find(end1, end1, L'D');