| Message ID | 1704fc02-f876-1fc5-070f-0decc2aa41fe@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 11/05/17 22:03 +0200, François Dumont wrote: >Hi > > _Safe_container _IsCxx11AllocatorAware template allocator is only >used if C++11 Abi is not used so I simplified it. > > * include/debug/safe_container.h [_GLIBCXX_USE_CXX11_ABI] > (_Safe_container<>): Remove _IsCxx11AllocatorAware template parameter. > * include/debug/string: Adapt. > > Tested under Linux x86_64 with debug mode and active C++11 Abi. Wait, doesn't this mean that debug containers have a different base class depending on which ABI is active in the translation unit? That is a One-Definition Rule violation when you link together objects compiled with different values of _GLIBCXX_USE_CXX11_ABI, which is supposed to work. I went to enormous effort to ensure it works. >François > >diff --git a/libstdc++-v3/include/debug/safe_container.h b/libstdc++-v3/include/debug/safe_container.h >index 3d44c15..e985c2a 100644 >--- a/libstdc++-v3/include/debug/safe_container.h >+++ b/libstdc++-v3/include/debug/safe_container.h >@@ -36,8 +36,12 @@ namespace __gnu_debug > /// Safe class dealing with some allocator dependent operations. > template<typename _SafeContainer, > typename _Alloc, >- template<typename> class _SafeBase, >- bool _IsCxx11AllocatorAware = true> >+ template<typename> class _SafeBase >+#if _GLIBCXX_USE_CXX11_ABI >+ > >+#else >+ , bool _IsCxx11AllocatorAware = true> >+#endif > class _Safe_container In any case, I don't think this is simpler, there are more lines of code, and the preprocessor condition makes it harder to read and harder to reason about. > : public _SafeBase<_SafeContainer> > { >@@ -82,8 +86,10 @@ namespace __gnu_debug > { > __glibcxx_check_self_move_assign(__x); > >+# if !_GLIBCXX_USE_CXX11_ABI > if (_IsCxx11AllocatorAware) > { >+# endif This is fairly pointless. Again it makes the code a bit harder to read, and since it's a compile-time constant the condition will be optimised away completely.
On 12/05/2017 12:38, Jonathan Wakely wrote: > On 11/05/17 22:03 +0200, François Dumont wrote: >> Hi >> >> _Safe_container _IsCxx11AllocatorAware template allocator is only >> used if C++11 Abi is not used so I simplified it. >> >> * include/debug/safe_container.h [_GLIBCXX_USE_CXX11_ABI] >> (_Safe_container<>): Remove _IsCxx11AllocatorAware template >> parameter. >> * include/debug/string: Adapt. >> >> Tested under Linux x86_64 with debug mode and active C++11 Abi. > > Wait, doesn't this mean that debug containers have a different base > class depending on which ABI is active in the translation unit? That > is a One-Definition Rule violation when you link together objects > compiled with different values of _GLIBCXX_USE_CXX11_ABI, which is > supposed to work. I went to enormous effort to ensure it works. > > >> François >> > >> diff --git a/libstdc++-v3/include/debug/safe_container.h >> b/libstdc++-v3/include/debug/safe_container.h >> index 3d44c15..e985c2a 100644 >> --- a/libstdc++-v3/include/debug/safe_container.h >> +++ b/libstdc++-v3/include/debug/safe_container.h >> @@ -36,8 +36,12 @@ namespace __gnu_debug >> /// Safe class dealing with some allocator dependent operations. >> template<typename _SafeContainer, >> typename _Alloc, >> - template<typename> class _SafeBase, >> - bool _IsCxx11AllocatorAware = true> >> + template<typename> class _SafeBase >> +#if _GLIBCXX_USE_CXX11_ABI >> + > >> +#else >> + , bool _IsCxx11AllocatorAware = true> >> +#endif >> class _Safe_container > > In any case, I don't think this is simpler, there are more lines of > code, and the preprocessor condition makes it harder to read and > harder to reason about. > >> : public _SafeBase<_SafeContainer> >> { >> @@ -82,8 +86,10 @@ namespace __gnu_debug >> { >> __glibcxx_check_self_move_assign(__x); >> >> +# if !_GLIBCXX_USE_CXX11_ABI >> if (_IsCxx11AllocatorAware) >> { >> +# endif > > This is fairly pointless. Again it makes the code a bit harder to > read, and since it's a compile-time constant the condition will be > optimised away completely. > > Ok, I reverted it. François
diff --git a/libstdc++-v3/include/debug/safe_container.h b/libstdc++-v3/include/debug/safe_container.h index 3d44c15..e985c2a 100644 --- a/libstdc++-v3/include/debug/safe_container.h +++ b/libstdc++-v3/include/debug/safe_container.h @@ -36,8 +36,12 @@ namespace __gnu_debug /// Safe class dealing with some allocator dependent operations. template<typename _SafeContainer, typename _Alloc, - template<typename> class _SafeBase, - bool _IsCxx11AllocatorAware = true> + template<typename> class _SafeBase +#if _GLIBCXX_USE_CXX11_ABI + > +#else + , bool _IsCxx11AllocatorAware = true> +#endif class _Safe_container : public _SafeBase<_SafeContainer> { @@ -82,8 +86,10 @@ namespace __gnu_debug { __glibcxx_check_self_move_assign(__x); +# if !_GLIBCXX_USE_CXX11_ABI if (_IsCxx11AllocatorAware) { +# endif typedef __gnu_cxx::__alloc_traits<_Alloc> _Alloc_traits; bool __xfer_memory = _Alloc_traits::_S_propagate_on_move_assign() @@ -92,9 +98,11 @@ namespace __gnu_debug _Base::_M_swap(__x); else this->_M_invalidate_all(); +# if !_GLIBCXX_USE_CXX11_ABI } else _Base::_M_swap(__x); +# endif __x._M_invalidate_all(); return *this; @@ -103,7 +111,9 @@ namespace __gnu_debug void _M_swap(_Safe_container& __x) noexcept { +# if !_GLIBCXX_USE_CXX11_ABI if (_IsCxx11AllocatorAware) +# endif { typedef __gnu_cxx::__alloc_traits<_Alloc> _Alloc_traits; diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string index 9d4057b..8fd292a 100644 --- a/libstdc++-v3/include/debug/string +++ b/libstdc++-v3/include/debug/string @@ -44,13 +44,20 @@ template<typename _CharT, typename _Traits = std::char_traits<_CharT>, class basic_string : public __gnu_debug::_Safe_container< basic_string<_CharT, _Traits, _Allocator>, - _Allocator, _Safe_sequence, bool(_GLIBCXX_USE_CXX11_ABI)>, +#if _GLIBCXX_USE_CXX11_ABI + _Allocator, _Safe_sequence>, +#else + _Allocator, _Safe_sequence, false>, +#endif public std::basic_string<_CharT, _Traits, _Allocator> { typedef std::basic_string<_CharT, _Traits, _Allocator> _Base; typedef __gnu_debug::_Safe_container< - basic_string, _Allocator, _Safe_sequence, bool(_GLIBCXX_USE_CXX11_ABI)> - _Safe; +#if _GLIBCXX_USE_CXX11_ABI + basic_string, _Allocator, _Safe_sequence> _Safe; +#else + basic_string, _Allocator, _Safe_sequence, false> _Safe; +#endif public: // types:
Hi _Safe_container _IsCxx11AllocatorAware template allocator is only used if C++11 Abi is not used so I simplified it. * include/debug/safe_container.h [_GLIBCXX_USE_CXX11_ABI] (_Safe_container<>): Remove _IsCxx11AllocatorAware template parameter. * include/debug/string: Adapt. Tested under Linux x86_64 with debug mode and active C++11 Abi. François