Re: PR libstdc++/68222 Hide safe iterator operators

Thu, 23 Aug 2018 08:23:45 -0700 

On 23/08/18 15:29 +0100, Jonathan Wakely wrote:

On 23/08/18 08:51 +0100, Jonathan Wakely wrote:

On 21/08/18 22:18 +0200, François Dumont wrote:

On 21/08/2018 11:33, Jonathan Wakely wrote:

On 18/08/18 22:31 +0200, François Dumont wrote:
Here is the new proposal. It is indeed possible to keep _Safe_iterator and just add a _Category template parameter to it. 

While this is still a large patch (obviously, because it's changing a
lot!) I think this version is much easier to understand, and doesn't
add a whole new class unnecessarily. Thanks for updating it.


I introduce a friend declaration to access container _Base 
nested typedef from the safe iterator.



I review the safe const_iterator constructor from safe 
iterator. I now check if we are in the a const_iterator 
context so that compilers don't even try to consider this 
constructor when we are in an iterator context.


Nice.


I adapted _Safe_local_iterator the same way to keep 
consistency with _Safe_iterator and because I find the new 
design cleaner. It also fixes the same problem I fixed on 
_Safe_iterator when checking it iterator has been initialized 
using _MutableIterator() rather than _Iterator().



I stop overloading __get_distance for safe iterators or safe 
local iterators, it was useless. I prefer to introduce similar 
functions as members. Same for __get_distance_from_begin or 
__get_distance_to_end, and I move code in safe_iterator.tcc.


Tested under Linux x86_64 debug mode.

Ok to commit ?


OK for trunk, thanks again.




Note that it also avoids to adapt the pretty printers scripts.


I'll commit it tomorrow once I'll have rework the ChangeLog entry. 
I'll also add the test case in PR 68222 adapted for the testsuite.


I'm not sure when it started, but I'm seeing a few failures in the
testsuite, with linker errors like this:

FAIL: 25_algorithms/sort/34636.cc (test for excess errors)
Excess errors:
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:614: undefined reference to `std::__debug::vector<int, std::allocator<int> >* std::__niter_base<std::__debug::vector<int, std::allocator<int> >*, 
std::__cxx1998::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, 
std::allocator<std::__debug::vector<int, std::allocator<int> > > > >(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::__debug::vector<int, std::allocator<int> >*, std::__cxx1998::vector<std::__debug::vector<int, 
std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, 
std::allocator<int> > > >, std::random_access_iterator_tag> const&)'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:614: undefined reference to `std::__debug::vector<int, std::allocator<int> >* std::__niter_base<std::__debug::vector<int, std::allocator<int> >*, 
std::__cxx1998::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, 
std::allocator<std::__debug::vector<int, std::allocator<int> > > > >(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::__debug::vector<int, std::allocator<int> >*, std::__cxx1998::vector<std::__debug::vector<int, 
std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, 
std::allocator<int> > > >, std::random_access_iterator_tag> const&)'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:614: undefined reference to `std::__debug::vector<int, std::allocator<int> >* std::__niter_base<std::__debug::vector<int, std::allocator<int> >*, 
std::__cxx1998::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, 
std::allocator<std::__debug::vector<int, std::allocator<int> > > > >(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::__debug::vector<int, std::allocator<int> >*, std::__cxx1998::vector<std::__debug::vector<int, 
std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, 
std::allocator<int> > > >, std::random_access_iterator_tag> const&)'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:286: undefined reference to `std::__debug::vector<int, std::allocator<int> >* std::__niter_base<std::__debug::vector<int, std::allocator<int> >*, 
std::__cxx1998::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, 
std::allocator<std::__debug::vector<int, std::allocator<int> > > > >(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<std::__debug::vector<int, std::allocator<int> >*, std::__cxx1998::vector<std::__debug::vector<int, 
std::allocator<int> >, std::allocator<std::__debug::vector<int, std::allocator<int> > > > >, std::__debug::vector<std::__debug::vector<int, std::allocator<int> >, std::allocator<std::__debug::vector<int, 
std::allocator<int> > > >, std::random_access_iterator_tag> const&)'
collect2: error: ld returned 1 exit status

This happens when run like:

make check RUNTESTFLAGS="conformance.exp==25_algorithms/sort/*.cc 
--target_board=unix/-std=gnu++98/-D_GLIBCXX_USE_CXX11_ABI=0/-D_GLIBCXX_DEBUG"


Seems to be caused by r262349

  2018-07-03  François Dumont

          * include/bits/stl_algobase.h (__niter_wrap): New.
          (__copy_move_a2(_II, _II, _OI)): Use latter.
          (__copy_move_backward_a2(_BI1, _BI1, _BI2)): Likewise.
          (fill_n(_OI, _Size, const _Tp&)): Likewise.
          (equal(_II1, _II1, _II2)): Use __glibcxx_requires_can_increment.
          * include/debug/stl_iterator.h
          (std::__niter_base(const __gnu_cxx::_Safe_iterator<
          __gnu_cxx::__normal_iterator<>, _Sequence>&)): New declaration.
          * include/debug/vector (__niter_base(const __gnu_cxx::_Safe_iterator<
          __gnu_cxx::__normal_iterator<>, _Sequence>&)): New.


I think because include/debug/stl_iterator.h declares the __niter_base
overload unconditionally, but then include/debug/vector only defines
it for C++11 and later.


Fixed by this patch. Tested x86_64-linux, committed to trunk.




commit ef605abf0a3ae6ee05996f7e9bdef8bdc13e37df
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Thu Aug 23 16:11:22 2018 +0100

    Define debug mode function for C++98
    
    This function is declared unconditionally but was only defined for C++11
    and later, leading to linker errors when the testsuite was run with
    -std=gnu++98 -D_GLIBCXX_DEBUG added to the flags.
    
            * include/debug/vector (__niter_base): Define for C++98.

diff --git a/libstdc++-v3/include/debug/vector b/libstdc++-v3/include/debug/vector
index bd09b2414fc..e8d0928c7c0 100644
--- a/libstdc++-v3/include/debug/vector
+++ b/libstdc++-v3/include/debug/vector
@@ -774,9 +774,9 @@ namespace __debug
 
 } // namespace __debug
 
-#if __cplusplus >= 201103L
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+#if __cplusplus >= 201103L
   // DR 1182.
   /// std::hash specialization for vector<bool>.
   template<typename _Alloc>
@@ -787,6 +787,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       operator()(const __debug::vector<bool, _Alloc>& __b) const noexcept
       { return std::hash<_GLIBCXX_STD_C::vector<bool, _Alloc>>()(__b); }
     };
+#endif
 
  template<typename _Iterator, typename _Container, typename _Sequence>
     _Iterator
@@ -796,8 +797,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return std::__niter_base(__it.base()); }
 
 _GLIBCXX_END_NAMESPACE_VERSION
-#endif
-
 } // namespace std
 
 namespace __gnu_debug






















					Reply via email to