* [PATCH] Complete __gnu_debug::basic_string
@ 2021-03-16 20:55 François Dumont
2021-03-19 19:41 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: François Dumont @ 2021-03-16 20:55 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]
Following:
https://gcc.gnu.org/pipermail/libstdc++/2021-March/052158.html
Here is the patch to complete __gnu_debug::basic_string support.
Contrarily to what I thought code in std::basic_string to generate a
basic_string_view works just fine for __gnu_debug::basic_string.
libstdc++: [_GLIBCXX_DEBUG] Add __gnu_debug
u8string/u16string/u32string
Complete __gnu_debug::basic_string support so that it can be used as a
transparent replacement of std::basic_string.
libstdc++-v3/ChangeLog:
* include/debug/string
(basic_string(const _CharT*, const _Allocator&)): Remove
assign call.
(basic_string<>::insert(const_iterator, _InputIte,
_InputIte)): Try to
remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
[_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
(__gnu_debug::u16string, __gnu_debug::u32string): New.
[!_GLIBCXX_COMPATIBILITY_CXX0X](std::hash<__gnu_debug::string>): New.
[!_GLIBCXX_COMPATIBILITY_CXX0X][_GLIBCXX_USE_WCHAR_T](std::hash<__gnu_debug::wstring>):
New.
[_GLIBCXX_USE_CHAR8_T](std::hash<__gnu_debug::u8string>): New.
(std::hash<__gnu_debug::u16string>): New.
(std::hash<__gnu_debug::u32string>): New.
* testsuite/21_strings/basic_string/hash/hash_char8_t.cc:
Adapt for
__gnu_debug basic_string.
Tested under Linux x86_64.
Ok to commit ?
François
[-- Attachment #2: debug_string.patch --]
[-- Type: text/x-patch, Size: 6946 bytes --]
diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
index d6eb5280ade..dec23f6277b 100644
--- a/libstdc++-v3/include/debug/string
+++ b/libstdc++-v3/include/debug/string
@@ -41,6 +41,14 @@
__gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
._M_message(#_Cond)._M_error()
+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+# define _GLIBCXX_CPP11_AND_CXX11_ABI 1
+# define _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(Statement) Statement
+#else
+# define _GLIBCXX_CPP11_AND_CXX11_ABI 0
+# define _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(Statement)
+#endif
+
namespace __gnu_debug
{
/** Checks that __s is non-NULL or __n == 0, and then returns __s. */
@@ -180,7 +188,7 @@ namespace __gnu_debug
basic_string(const _CharT* __s, const _Allocator& __a = _Allocator())
: _Base(__glibcxx_check_string_constructor(__s), __a)
- { this->assign(__s); }
+ { }
basic_string(size_type __n, _CharT __c,
const _Allocator& __a = _Allocator())
@@ -637,15 +645,22 @@ namespace __gnu_debug
__glibcxx_check_insert_range(__p, __first, __last, __dist);
typename _Base::iterator __res;
-#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+#if ! _GLIBCXX_CPP11_AND_CXX11_ABI
+ const size_type __offset = __p.base() - _Base::begin();
+#endif
if (__dist.second >= __dp_sign)
- __res = _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
- __gnu_debug::__unsafe(__last));
+ {
+ _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(__res =)
+ _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
+ __gnu_debug::__unsafe(__last));
+ }
else
- __res = _Base::insert(__p.base(), __first, __last);
-#else
- const size_type __offset = __p.base() - _Base::begin();
- _Base::insert(__p.base(), __first, __last);
+ {
+ _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(__res =)
+ _Base::insert(__p.base(), __first, __last);
+ }
+
+#if ! _GLIBCXX_CPP11_AND_CXX11_ABI
__res = _Base::begin() + __offset;
#endif
this->_M_invalidate_all();
@@ -1287,6 +1302,19 @@ namespace __gnu_debug
typedef basic_string<wchar_t> wstring;
#endif
+#ifdef _GLIBCXX_USE_CHAR8_T
+ /// A string of @c char8_t
+ typedef basic_string<char8_t> u8string;
+#endif
+
+#if __cplusplus >= 201103L
+ /// A string of @c char16_t
+ typedef basic_string<char16_t> u16string;
+
+ /// A string of @c char32_t
+ typedef basic_string<char32_t> u32string;
+#endif
+
template<typename _CharT, typename _Traits, typename _Allocator>
struct _Insert_range_from_self_is_safe<
__gnu_debug::basic_string<_CharT, _Traits, _Allocator> >
@@ -1294,4 +1322,96 @@ namespace __gnu_debug
} // namespace __gnu_debug
+#if __cplusplus >= 201103L
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+ // DR 1182.
+
+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
+ /// std::hash specialization for string.
+ template<>
+ struct hash<__gnu_debug::string>
+ : public __hash_base<size_t, __gnu_debug::string>
+ {
+ size_t
+ operator()(const __gnu_debug::string& __s) const noexcept
+ { return std::hash<std::string>()(__s); }
+ };
+
+ template<>
+ struct __is_fast_hash<hash<__gnu_debug::string>> : std::false_type
+ { };
+
+#ifdef _GLIBCXX_USE_WCHAR_T
+ /// std::hash specialization for wstring.
+ template<>
+ struct hash<__gnu_debug::wstring>
+ : public __hash_base<size_t, __gnu_debug::wstring>
+ {
+ size_t
+ operator()(const __gnu_debug::wstring& __s) const noexcept
+ { return std::hash<__gnu_debug::wstring>()(__s); }
+ };
+
+ template<>
+ struct __is_fast_hash<hash<__gnu_debug::wstring>> : std::false_type
+ { };
+#endif
+#endif /* _GLIBCXX_COMPATIBILITY_CXX0X */
+
+#ifdef _GLIBCXX_USE_CHAR8_T
+ /// std::hash specialization for u8string.
+ template<>
+ struct hash<__gnu_debug::u8string>
+ : public __hash_base<size_t, __gnu_debug::u8string>
+ {
+ size_t
+ operator()(const __gnu_debug::u8string& __s) const noexcept
+ { return std::hash<std::u8string>()(__s); }
+ };
+
+ template<>
+ struct __is_fast_hash<hash<__gnu_debug::u8string>> : std::false_type
+ { };
+#endif
+
+ /// std::hash specialization for u16string.
+ template<>
+ struct hash<__gnu_debug::u16string>
+ : public __hash_base<size_t, __gnu_debug::u16string>
+ {
+ size_t
+ operator()(const __gnu_debug::u16string& __s) const noexcept
+ { return std::hash<std::u16string>()(__s); }
+ };
+
+ template<>
+ struct __is_fast_hash<hash<__gnu_debug::u16string>> : std::false_type
+ { };
+
+ /// std::hash specialization for u32string.
+ template<>
+ struct hash<__gnu_debug::u32string>
+ : public __hash_base<size_t, __gnu_debug::u32string>
+ {
+ size_t
+ operator()(const __gnu_debug::u32string& __s) const noexcept
+ { return std::hash<std::u32string>()(__s); }
+ };
+
+ template<>
+ struct __is_fast_hash<hash<__gnu_debug::u32string>> : std::false_type
+ { };
+
+_GLIBCXX_END_NAMESPACE_VERSION
+ } // namespace std
+
+#endif /* C++11 */
+
+#undef _GLIBCXX_CPP11_AND_CXX11_ABI
+#undef _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY
+
#endif
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
index c0e8975ce4a..7f8792bbd76 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
@@ -18,10 +18,19 @@
// { dg-options "-std=gnu++2a" }
// { dg-do run { target c++2a } }
-#include <string>
#include <memory_resource>
#include <testsuite_hooks.h>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+
+using namespace __gnu_debug;
+#else
+# include <string>
+
+using namespace std;
+#endif
+
// C++2a N4810 21.3.5 [basic.string.hash]
// If S is one of these string types, SV is the corresponding string view type,
// and s is an object of type S, then hash<S>()(s) == hash<SV>()(SV(s)).
@@ -38,8 +47,8 @@ template<typename S>
void
test01()
{
- VERIFY( test(std::string("a narrow string")) );
- VERIFY( test(std::u8string(u8"a utf-8 string")) );
+ VERIFY( test(string("a narrow string")) );
+ VERIFY( test(u8string(u8"a utf-8 string")) );
#if _GLIBCXX_USE_CXX11_ABI
VERIFY( test(std::pmr::string("a narrow string, but with PMR!")) );
VERIFY( test(std::pmr::u8string(u8"a utf-8 string, but with PMR!")) );
@@ -50,9 +59,9 @@ void
test02()
{
using std::hash;
- std::string native("a string, a string, my stringdom for a string");
- std::u8string utf8(u8"a string, a string, my stringdom for a string");
- VERIFY( hash<std::string>()(native) == hash<std::u8string>()(utf8) );
+ string native("a string, a string, my stringdom for a string");
+ u8string utf8(u8"a string, a string, my stringdom for a string");
+ VERIFY( hash<string>()(native) == hash<u8string>()(utf8) );
}
int
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-16 20:55 [PATCH] Complete __gnu_debug::basic_string François Dumont
@ 2021-03-19 19:41 ` Jonathan Wakely
2021-03-20 21:32 ` François Dumont
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-19 19:41 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 16/03/21 21:55 +0100, François Dumont via Libstdc++ wrote:
>Following:
>
>https://gcc.gnu.org/pipermail/libstdc++/2021-March/052158.html
>
>Here is the patch to complete __gnu_debug::basic_string support.
>Contrarily to what I thought code in std::basic_string to generate a
>basic_string_view works just fine for __gnu_debug::basic_string.
>
> libstdc++: [_GLIBCXX_DEBUG] Add __gnu_debug
>u8string/u16string/u32string
>
> Complete __gnu_debug::basic_string support so that it can be used as a
> transparent replacement of std::basic_string.
>
> libstdc++-v3/ChangeLog:
>
> * include/debug/string
> (basic_string(const _CharT*, const _Allocator&)): Remove
>assign call.
> (basic_string<>::insert(const_iterator, _InputIte,
>_InputIte)): Try to
> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
> (__gnu_debug::u16string, __gnu_debug::u32string): New.
>[!_GLIBCXX_COMPATIBILITY_CXX0X](std::hash<__gnu_debug::string>): New.
>[!_GLIBCXX_COMPATIBILITY_CXX0X][_GLIBCXX_USE_WCHAR_T](std::hash<__gnu_debug::wstring>):
>New.
>[_GLIBCXX_USE_CHAR8_T](std::hash<__gnu_debug::u8string>): New.
> (std::hash<__gnu_debug::u16string>): New.
> (std::hash<__gnu_debug::u32string>): New.
> * testsuite/21_strings/basic_string/hash/hash_char8_t.cc:
>Adapt for
> __gnu_debug basic_string.
>
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>
>diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
>index d6eb5280ade..dec23f6277b 100644
>--- a/libstdc++-v3/include/debug/string
>+++ b/libstdc++-v3/include/debug/string
>@@ -41,6 +41,14 @@
> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
> ._M_message(#_Cond)._M_error()
>
>+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>+# define _GLIBCXX_CPP11_AND_CXX11_ABI 1
>+# define _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(Statement) Statement
This takes an expression, not a statement.
I think it would be better to use more descriptive names for these:
# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
(And don't forget to change the #undef lines too).
>+#if __cplusplus >= 201103L
>+
>+namespace std _GLIBCXX_VISIBILITY(default)
>+{
>+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>+
>+ // DR 1182.
>+
>+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
>+ /// std::hash specialization for string.
>+ template<>
>+ struct hash<__gnu_debug::string>
>+ : public __hash_base<size_t, __gnu_debug::string>
I think we could just define on partial specialization that works for
all cases:
template<typename _CharT>
struct hash<__gnu_debug::basic_string<_CharT>>
: public hash<std::basic_string<_CharT>>
{ };
If we're being pedantic, this isn't strictly-conforming because it
will inherit the argument_type member from the base class, which will
be the non-debug string rather than the debug one. I don't think I
care about that.
If we cared, we'd need to do something like:
template<typename _CharT, typename = void>
struct __debug_str_hash
{ };
template<typename _CharT>
struct __debug_str_hash<_CharT,
__void_t<typename hash<std::basic_string<_CharT>>::argument_type>>
: hash<std::basic_string<_CharT>>
{
using argument_type = __gnu_debug::basic_string<_CharT>;
size_t operator()(const argument_type& __s) const noexcept
{ return hash<std::basic_string<_CharT>>()(__s); }
};
template<typename _CharT>
struct hash<__gnu_debug::basic_string<_CharT>>
: public __debug_str_hash<_CharT>
{ };
I don't think we should care.
>+ {
>+ size_t
>+ operator()(const __gnu_debug::string& __s) const noexcept
>+ { return std::hash<std::string>()(__s); }
>+ };
>+
>+ template<>
>+ struct __is_fast_hash<hash<__gnu_debug::string>> : std::false_type
>+ { };
And:
template<typename _CharT>
struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
: __is_fast_hash<hash<std::basic_string<_CharT>>>
{ };
>diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
>index c0e8975ce4a..7f8792bbd76 100644
>--- a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
>+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash_char8_t.cc
Why only make this change for the char8_t version? Why not test
hash<__gnu_debug::string> as well?
>@@ -18,10 +18,19 @@
> // { dg-options "-std=gnu++2a" }
> // { dg-do run { target c++2a } }
>
>-#include <string>
> #include <memory_resource>
> #include <testsuite_hooks.h>
>
>+#ifdef _GLIBCXX_DEBUG
>+# include <debug/string>
>+
>+using namespace __gnu_debug;
This means we no longer test the hash<std::u8string> specialization in
debug mode.
Please leave this test unchanged and add a separate file (e.g.
21_strings/basic_string/hash/debug.cc) which tests all the
hash<__gnu_debug::*string> specializations.
It's stage 4, but this only affects a type that is not used anywhere
by default, not even when Debug Mode is active, so I think this can
still potentially go in for GCC 11.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-19 19:41 ` Jonathan Wakely
@ 2021-03-20 21:32 ` François Dumont
2021-03-23 15:42 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: François Dumont @ 2021-03-20 21:32 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 6599 bytes --]
Following your feedback here is the simplified version. I grouped it
with the patch I submitted before.
On 19/03/21 8:41 pm, Jonathan Wakely wrote:
> On 16/03/21 21:55 +0100, François Dumont via Libstdc++ wrote:
>> Following:
>>
>> https://gcc.gnu.org/pipermail/libstdc++/2021-March/052158.html
>>
>> Here is the patch to complete __gnu_debug::basic_string support.
>> Contrarily to what I thought code in std::basic_string to generate a
>> basic_string_view works just fine for __gnu_debug::basic_string.
>>
>> libstdc++: [_GLIBCXX_DEBUG] Add __gnu_debug
>> u8string/u16string/u32string
>>
>> Complete __gnu_debug::basic_string support so that it can be used
>> as a
>> transparent replacement of std::basic_string.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/debug/string
>> (basic_string(const _CharT*, const _Allocator&)): Remove
>> assign call.
>> (basic_string<>::insert(const_iterator, _InputIte,
>> _InputIte)): Try to
>> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
>> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
>> (__gnu_debug::u16string, __gnu_debug::u32string): New.
>> [!_GLIBCXX_COMPATIBILITY_CXX0X](std::hash<__gnu_debug::string>): New.
>> [!_GLIBCXX_COMPATIBILITY_CXX0X][_GLIBCXX_USE_WCHAR_T](std::hash<__gnu_debug::wstring>):
>> New.
>> [_GLIBCXX_USE_CHAR8_T](std::hash<__gnu_debug::u8string>): New.
>> (std::hash<__gnu_debug::u16string>): New.
>> (std::hash<__gnu_debug::u32string>): New.
>> * testsuite/21_strings/basic_string/hash/hash_char8_t.cc:
>> Adapt for
>> __gnu_debug basic_string.
>>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/debug/string
>> b/libstdc++-v3/include/debug/string
>> index d6eb5280ade..dec23f6277b 100644
>> --- a/libstdc++-v3/include/debug/string
>> +++ b/libstdc++-v3/include/debug/string
>> @@ -41,6 +41,14 @@
>> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
>> ._M_message(#_Cond)._M_error()
>>
>> +#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>> +# define _GLIBCXX_CPP11_AND_CXX11_ABI 1
>> +# define _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(Statement) Statement
>
> This takes an expression, not a statement.
I've been inspired by _GLIBCXX_DEBUG_ONLY
>
> I think it would be better to use more descriptive names for these:
>
> # define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
> # define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
>
> (And don't forget to change the #undef lines too).
>
>> +#if __cplusplus >= 201103L
>> +
>> +namespace std _GLIBCXX_VISIBILITY(default)
>> +{
>> +_GLIBCXX_BEGIN_NAMESPACE_VERSION
>> +
>> + // DR 1182.
>> +
>> +#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
>> + /// std::hash specialization for string.
>> + template<>
>> + struct hash<__gnu_debug::string>
>> + : public __hash_base<size_t, __gnu_debug::string>
>
> I think we could just define on partial specialization that works for
> all cases:
Yes, sounds better. But I relied on std::__hash_base which gives
directly the correct definition.
> Why only make this change for the char8_t version? Why not test
> hash<__gnu_debug::string> as well?
This file also test std::string and so also __gnu_debug::string.
libstdc++: Fix and complete __gnu_debug::basic_string implementation
Fix and complete __gnu_debug::basic_string so that it can be used
as a transparent
replacement of std::basic_string.
libstdc++-v3/ChangeLog:
* include/debug/string
(basic_string(const _CharT*, const _Allocator&)): Remove
assign call.
(basic_string<>::insert(const_iterator, _InputIte,
_InputIte)): Try to
remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
[_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
(__gnu_debug::u16string, __gnu_debug::u32string): New.
(std::hash<__gnu_debug::basic_string<>>): New partial
specialization.
(std::__is_fast_hash<__gnu_debug::basic_string<>>): Likewise.
(basic_string(const basic_string&, const _Alloc&)): Define
even if !_GLIBCXX_USE_CXX11_ABI.
(basic_string(basic_string&&, const _Alloc&)): Likewise and
add noexcept qualification.
(basic_string<>::erase): Adapt to take __const_iterator.
* testsuite/21_strings/basic_string/hash/debug.cc: New test.
* testsuite/21_strings/basic_string/hash/debug_char8_t.cc:
New test.
*
testsuite/21_strings/basic_string/requirements/citerators.cc: Adapt to
test __gnu_debug::string
when _GLIBCXX_DEBUG is defined.
*
testsuite/21_strings/basic_string/requirements/dr438/constructor.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/exception/basic.cc: Likewise.
*
testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc:
Likewise.
*
testsuite/21_strings/basic_string/requirements/typedefs.cc: Likewise.
* testsuite/util/exception/safety.h
(erase_base<__gnu_debug::basic_string<>>): New partial
specialization.
(insert_base<__gnu_debug::basic_string<>>): Likewise.
* testsuite/util/testsuite_container_traits.h
(traits<__gnu_debug::basic_string<>>): Likewise.
Tested under Linux x86_64.
Ok to commit ?
François
[-- Attachment #2: debug_string.patch --]
[-- Type: text/x-patch, Size: 22680 bytes --]
diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
index 172179530aa..f665c759557 100644
--- a/libstdc++-v3/include/debug/string
+++ b/libstdc++-v3/include/debug/string
@@ -41,6 +41,14 @@
__gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
._M_message(#_Cond)._M_error()
+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
+#else
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 0
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr)
+#endif
+
namespace __gnu_debug
{
/** Checks that __s is non-NULL or __n == 0, and then returns __s. */
@@ -123,21 +131,21 @@ namespace __gnu_debug
using _Base::npos;
- basic_string()
- _GLIBCXX_NOEXCEPT_IF(std::is_nothrow_default_constructible<_Base>::value)
- : _Base() { }
-
// 21.3.1 construct/copy/destroy:
+
explicit
basic_string(const _Allocator& __a) _GLIBCXX_NOEXCEPT
: _Base(__a) { }
#if __cplusplus < 201103L
+ basic_string() : _Base() { }
+
basic_string(const basic_string& __str)
: _Base(__str) { }
~basic_string() { }
#else
+ basic_string() = default;
basic_string(const basic_string&) = default;
basic_string(basic_string&&) = default;
@@ -146,13 +154,15 @@ namespace __gnu_debug
: _Base(__l, __a)
{ }
-#if _GLIBCXX_USE_CXX11_ABI
basic_string(const basic_string& __s, const _Allocator& __a)
: _Base(__s, __a) { }
basic_string(basic_string&& __s, const _Allocator& __a)
- : _Base(std::move(__s), __a) { }
-#endif
+ noexcept( noexcept(
+ _Base(std::declval<_Base>()), std::declval<const _Allocator&>()) )
+ : _Safe(std::move(__s._M_safe()), __a),
+ _Base(std::move(__s._M_base()), __a)
+ { }
~basic_string() = default;
@@ -178,7 +188,7 @@ namespace __gnu_debug
basic_string(const _CharT* __s, const _Allocator& __a = _Allocator())
: _Base(__glibcxx_check_string_constructor(__s), __a)
- { this->assign(__s); }
+ { }
basic_string(size_type __n, _CharT __c,
const _Allocator& __a = _Allocator())
@@ -635,15 +645,22 @@ namespace __gnu_debug
__glibcxx_check_insert_range(__p, __first, __last, __dist);
typename _Base::iterator __res;
-#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+#if ! _GLIBCXX_INSERT_RETURNS_ITERATOR
+ const size_type __offset = __p.base() - _Base::begin();
+#endif
if (__dist.second >= __dp_sign)
- __res = _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
- __gnu_debug::__unsafe(__last));
+ {
+ _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(__res =)
+ _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
+ __gnu_debug::__unsafe(__last));
+ }
else
- __res = _Base::insert(__p.base(), __first, __last);
-#else
- const size_type __offset = __p.base() - _Base::begin();
- _Base::insert(__p.base(), __first, __last);
+ {
+ _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(__res =)
+ _Base::insert(__p.base(), __first, __last);
+ }
+
+#if ! _GLIBCXX_INSERT_RETURNS_ITERATOR
__res = _Base::begin() + __offset;
#endif
this->_M_invalidate_all();
@@ -676,7 +693,7 @@ namespace __gnu_debug
}
iterator
- erase(iterator __position)
+ erase(__const_iterator __position)
{
__glibcxx_check_erase(__position);
typename _Base::iterator __res = _Base::erase(__position.base());
@@ -685,7 +702,7 @@ namespace __gnu_debug
}
iterator
- erase(iterator __first, iterator __last)
+ erase(__const_iterator __first, __const_iterator __last)
{
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 151. can't currently clear() empty container
@@ -1285,6 +1302,19 @@ namespace __gnu_debug
typedef basic_string<wchar_t> wstring;
#endif
+#ifdef _GLIBCXX_USE_CHAR8_T
+ /// A string of @c char8_t
+ typedef basic_string<char8_t> u8string;
+#endif
+
+#if __cplusplus >= 201103L
+ /// A string of @c char16_t
+ typedef basic_string<char16_t> u16string;
+
+ /// A string of @c char32_t
+ typedef basic_string<char32_t> u32string;
+#endif
+
template<typename _CharT, typename _Traits, typename _Allocator>
struct _Insert_range_from_self_is_safe<
__gnu_debug::basic_string<_CharT, _Traits, _Allocator> >
@@ -1292,4 +1322,31 @@ namespace __gnu_debug
} // namespace __gnu_debug
+#if __cplusplus >= 201103L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+ /// std::hash specialization for __gnu_debug::basic_string.
+ template<typename _CharT>
+ struct hash<__gnu_debug::basic_string<_CharT>>
+ : public __hash_base<size_t, __gnu_debug::basic_string<_CharT>>
+ {
+ size_t
+ operator()(const __gnu_debug::basic_string<_CharT>& __s) const noexcept
+ { return std::hash<std::basic_string<_CharT>>()(__s); }
+ };
+
+ template<typename _CharT>
+ struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
+ : std::false_type
+ { };
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif /* C++11 */
+
+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR
+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY
+
#endif
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
new file mode 100644
index 00000000000..84c989590b7
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
@@ -0,0 +1,53 @@
+// Copyright (C) 2021 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-options "-std=gnu++17" }
+// { dg-do run { target c++17 } }
+
+#include <debug/string>
+#include <memory_resource>
+#include <testsuite_hooks.h>
+
+// C++17 24.3.5 [basic.string.hash]
+// If S is one of these string types, SV is the corresponding string view type,
+// and s is an object of type S, then hash<S>()(s) == hash<SV>()(SV(s)).
+
+template<typename S>
+ bool
+ test(const S& s)
+ {
+ using std::hash;
+ using SV = std::basic_string_view<typename S::value_type>;
+ return hash<S>()(s) == hash<SV>()(SV(s));
+ }
+
+void
+test01()
+{
+ VERIFY( test(__gnu_debug::string("a narrow string")) );
+ VERIFY( test(__gnu_debug::u16string(u"a utf-16 string")) );
+ VERIFY( test(__gnu_debug::u32string(U"a utf-32 string")) );
+#if _GLIBCXX_USE_WCHAR_T
+ VERIFY( test(__gnu_debug::wstring(L"a wide string")) );
+#endif
+}
+
+int
+main()
+{
+ test01();
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug_char8_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug_char8_t.cc
new file mode 100644
index 00000000000..b0c1eac2df3
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug_char8_t.cc
@@ -0,0 +1,59 @@
+// Copyright (C) 2019-2021 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-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include <debug/string>
+#include <memory_resource>
+#include <testsuite_hooks.h>
+
+// C++2a N4810 21.3.5 [basic.string.hash]
+// If S is one of these string types, SV is the corresponding string view type,
+// and s is an object of type S, then hash<S>()(s) == hash<SV>()(SV(s)).
+
+template<typename S>
+ bool
+ test(const S& s)
+ {
+ using std::hash;
+ using SV = std::basic_string_view<typename S::value_type>;
+ return hash<S>()(s) == hash<SV>()(SV(s));
+ }
+
+void
+test01()
+{
+ VERIFY( test(__gnu_debug::string("a narrow string")) );
+ VERIFY( test(__gnu_debug::u8string(u8"a utf-8 string")) );
+}
+
+void
+test02()
+{
+ using std::hash;
+ __gnu_debug::string native("a string, a string, my stringdom for a string");
+ __gnu_debug::u8string utf8(u8"a string, a string, my stringdom for a string");
+ VERIFY( hash<__gnu_debug::string>()(native) == hash<__gnu_debug::u8string>()(utf8) );
+}
+
+int
+main()
+{
+ test01();
+ test02();
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
index 99bf5726dcc..69d4a8d0e51 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
@@ -18,14 +18,21 @@
// with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
#include <testsuite_containers.h>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
+
int main()
{
- __gnu_test::citerator<std::string> test1;
+ __gnu_test::citerator<string> test1;
#ifdef _GLIBCXX_USE_WCHAR_T
- __gnu_test::citerator<std::wstring> test2;
+ __gnu_test::citerator<wstring> test2;
#endif
return 0;
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
index 82d42ebb6a6..104fd653642 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
@@ -19,9 +19,17 @@
// { dg-do compile }
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+
+using namespace __gnu_debug;
+#else
+# include <string>
+
+using namespace std;
+#endif
void f()
{
- std::string s(10, 1);
+ string s(10, 1);
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
index 2b6e27432e8..f40dda2b5bc 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
@@ -20,9 +20,16 @@
// with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
#include <exception/safety.h>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
+
void
value()
{
@@ -31,7 +38,7 @@ value()
typedef char value_type;
typedef __gnu_cxx::throw_allocator_limit<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ typedef basic_string<value_type, traits_type, allocator_type> test_type;
__gnu_test::basic_safety<test_type> test;
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
index 07d2a2e2074..dc9bdb298ae 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
@@ -20,16 +20,23 @@
// with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
#include <exception/safety.h>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
+
void
char_allocator()
{
typedef char value_type;
typedef __gnu_cxx::throw_allocator_random<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ typedef basic_string<value_type, traits_type, allocator_type> test_type;
__gnu_test::generation_prohibited<test_type> test;
}
@@ -39,7 +46,7 @@ wchar_allocator()
typedef wchar_t value_type;
typedef __gnu_cxx::throw_allocator_random<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ typedef basic_string<value_type, traits_type, allocator_type> test_type;
__gnu_test::generation_prohibited<test_type> test;
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
index ce99dab915e..818c487c9c9 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
@@ -20,9 +20,16 @@
// with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
#include <exception/safety.h>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
+
void
value()
{
@@ -31,7 +38,7 @@ value()
typedef char value_type;
typedef __gnu_cxx::throw_allocator_limit<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ typedef basic_string<value_type, traits_type, allocator_type> test_type;
__gnu_test::propagation_consistent<test_type> test;
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
index 275354c1f4c..6910933253a 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
@@ -17,6 +17,12 @@
// along with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
-template class std::basic_string<char>;
+template class basic_string<char>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
index 2b5139de533..aadb9ef580c 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
@@ -17,6 +17,12 @@
// along with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
-template class std::basic_string<char16_t>;
+template class basic_string<char16_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
index 0fa685e9094..62760f7180f 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
@@ -17,6 +17,12 @@
// along with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
-template class std::basic_string<char32_t>;
+template class basic_string<char32_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
index db88b0f47a3..c38f31429da 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
@@ -18,6 +18,12 @@
// along with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
-template class std::basic_string<char8_t>;
+template class basic_string<char8_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
index 7f16ca4cb5b..a0565baee54 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
@@ -17,6 +17,12 @@
// along with this library; see the file COPYING3. If not see
// <http://www.gnu.org/licenses/>.
-#include <string>
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+using namespace __gnu_debug;
+#else
+# include <string>
+using namespace std;
+#endif
-template class std::basic_string<wchar_t>;
+template class basic_string<wchar_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
index 2fc313b9cbf..b90eb020db4 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
@@ -18,10 +18,19 @@
// <http://www.gnu.org/licenses/>.
#include <testsuite_containers.h>
-#include <string>
+
+#ifdef _GLIBCXX_DEBUG
+# include <debug/string>
+
+using namespace __gnu_debug;
+#else
+# include <string>
+
+using namespace std;
+#endif
// Check container for required typedefs.
-__gnu_test::types<std::string> t1;
+__gnu_test::types<string> t1;
#ifdef _GLIBCXX_USE_WCHAR_T
-__gnu_test::types<std::wstring> t2;
+__gnu_test::types<wstring> t2;
#endif
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 6c91e740e0d..9309af15405 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -266,6 +266,20 @@ namespace __gnu_test
iterator (container_type::* _F_erase_point)(iterator);
iterator (container_type::* _F_erase_range)(iterator, iterator);
+ erase_base()
+ : _F_erase_point(&container_type::erase),
+ _F_erase_range(&container_type::erase) { }
+ };
+
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct erase_base<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>>
+ {
+ typedef __gnu_debug::basic_string<_Tp1, _Tp2, _Tp3> container_type;
+ typedef typename container_type::iterator iterator;
+
+ iterator (container_type::* _F_erase_point)(iterator);
+ iterator (container_type::* _F_erase_range)(iterator, iterator);
+
erase_base()
: _F_erase_point(&container_type::erase),
_F_erase_range(&container_type::erase) { }
@@ -681,6 +695,24 @@ namespace __gnu_test
typedef typename container_type::const_iterator const_iterator;
typedef typename container_type::value_type value_type;
+#if _GLIBCXX_USE_CXX11_ABI == 0 || __cplusplus < 201103L
+ iterator (container_type::* _F_insert_point)(iterator, value_type);
+#else
+ iterator (container_type::* _F_insert_point)(const_iterator,
+ value_type);
+#endif
+
+ insert_base() : _F_insert_point(&container_type::insert) { }
+ };
+
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct insert_base<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>>
+ {
+ typedef __gnu_debug::basic_string<_Tp1, _Tp2, _Tp3> container_type;
+ typedef typename container_type::iterator iterator;
+ typedef typename container_type::const_iterator const_iterator;
+ typedef typename container_type::value_type value_type;
+
#if _GLIBCXX_USE_CXX11_ABI == 0 || __cplusplus < 201103L
iterator (container_type::* _F_insert_point)(iterator, value_type);
#else
diff --git a/libstdc++-v3/testsuite/util/testsuite_container_traits.h b/libstdc++-v3/testsuite/util/testsuite_container_traits.h
index fb1f558011c..0bc7a2aa53a 100644
--- a/libstdc++-v3/testsuite/util/testsuite_container_traits.h
+++ b/libstdc++-v3/testsuite/util/testsuite_container_traits.h
@@ -22,6 +22,7 @@
#include <bits/stdc++.h>
#include <ext/vstring.h>
+#include <debug/string>
namespace __gnu_test
{
@@ -128,6 +129,17 @@ namespace __gnu_test
typedef std::true_type has_insert;
};
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct traits<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>> : public traits_base
+ {
+ typedef std::true_type is_container;
+ typedef std::true_type is_reversible;
+ typedef std::true_type is_allocator_aware;
+
+ typedef std::true_type has_erase;
+ typedef std::true_type has_insert;
+ };
+
template<typename _Tp1, typename _Tp2, typename _Tp3,
template <typename, typename, typename> class _Tp4>
struct traits<__gnu_cxx::__versa_string<_Tp1, _Tp2, _Tp3, _Tp4>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-20 21:32 ` François Dumont
@ 2021-03-23 15:42 ` Jonathan Wakely
2021-03-24 21:48 ` François Dumont
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-23 15:42 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 20/03/21 22:32 +0100, François Dumont wrote:
>Following your feedback here is the simplified version. I grouped it
>with the patch I submitted before.
>
>
>On 19/03/21 8:41 pm, Jonathan Wakely wrote:
>>On 16/03/21 21:55 +0100, François Dumont via Libstdc++ wrote:
>>>Following:
>>>
>>>https://gcc.gnu.org/pipermail/libstdc++/2021-March/052158.html
>>>
>>>Here is the patch to complete __gnu_debug::basic_string support.
>>>Contrarily to what I thought code in std::basic_string to generate
>>>a basic_string_view works just fine for __gnu_debug::basic_string.
>>>
>>> libstdc++: [_GLIBCXX_DEBUG] Add __gnu_debug
>>>u8string/u16string/u32string
>>>
>>> Complete __gnu_debug::basic_string support so that it can be
>>>used as a
>>> transparent replacement of std::basic_string.
>>>
>>> libstdc++-v3/ChangeLog:
>>>
>>> * include/debug/string
>>> (basic_string(const _CharT*, const _Allocator&)):
>>>Remove assign call.
>>> (basic_string<>::insert(const_iterator, _InputIte,
>>>_InputIte)): Try to
>>> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
>>> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
>>> (__gnu_debug::u16string, __gnu_debug::u32string): New.
>>>[!_GLIBCXX_COMPATIBILITY_CXX0X](std::hash<__gnu_debug::string>): New.
>>>[!_GLIBCXX_COMPATIBILITY_CXX0X][_GLIBCXX_USE_WCHAR_T](std::hash<__gnu_debug::wstring>):
>>>New.
>>>[_GLIBCXX_USE_CHAR8_T](std::hash<__gnu_debug::u8string>): New.
>>> (std::hash<__gnu_debug::u16string>): New.
>>> (std::hash<__gnu_debug::u32string>): New.
>>> *
>>>testsuite/21_strings/basic_string/hash/hash_char8_t.cc: Adapt for
>>> __gnu_debug basic_string.
>>>
>>>Tested under Linux x86_64.
>>>
>>>Ok to commit ?
>>>
>>>François
>>>
>>
>>>diff --git a/libstdc++-v3/include/debug/string
>>>b/libstdc++-v3/include/debug/string
>>>index d6eb5280ade..dec23f6277b 100644
>>>--- a/libstdc++-v3/include/debug/string
>>>+++ b/libstdc++-v3/include/debug/string
>>>@@ -41,6 +41,14 @@
>>> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
>>> ._M_message(#_Cond)._M_error()
>>>
>>>+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>>>+# define _GLIBCXX_CPP11_AND_CXX11_ABI 1
>>>+# define _GLIBCXX_CPP11_AND_CXX11_ABI_ONLY(Statement) Statement
>>
>>This takes an expression, not a statement.
>
>I've been inspired by _GLIBCXX_DEBUG_ONLY
Which is generally used with complete statements, not just
expressions. See _GLIBCXX20_ONLY which is used for expressions, and
uses __expr for the argument name.
>>
>>I think it would be better to use more descriptive names for these:
>>
>># define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
>># define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
>>
>>(And don't forget to change the #undef lines too).
>>
>>>+#if __cplusplus >= 201103L
>>>+
>>>+namespace std _GLIBCXX_VISIBILITY(default)
>>>+{
>>>+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>+
>>>+ // DR 1182.
>>>+
>>>+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
>>>+ /// std::hash specialization for string.
>>>+ template<>
>>>+ struct hash<__gnu_debug::string>
>>>+ : public __hash_base<size_t, __gnu_debug::string>
>>
>>I think we could just define on partial specialization that works for
>>all cases:
>
>Yes, sounds better. But I relied on std::__hash_base which gives
>directly the correct definition.
But that is wrong.
The requirements in https://wg21.link/unord.hash say that std::hash<T>
must be disabled for unsupported types. With std::basic_string the
specialization std::hash<basic_string<int>> is disabled, because there
is no specialization for that type, so it uses the primary template of
std::hash, which is empty:
/// Primary class template hash, usable for enum types only.
// Use with non-enum types still SFINAES.
template<typename _Tp>
struct hash : __hash_enum<_Tp>
{ };
With your patch, std::hash<__gnu_debug::basic_string<int>> will not be
empty. It will provide argument_type and result_type and operator()
but calling operator() will fail to compile.
My suggestion doesn't have that problem. With my suggestion,
hash<_gnu_debug::basic_string<C>> is enabled if (and only if)
hash<std::basic_string<C>> is enabled.
>>Why only make this change for the char8_t version? Why not test
>>hash<__gnu_debug::string> as well?
>
>This file also test std::string and so also __gnu_debug::string.
Ah yes.
> libstdc++: Fix and complete __gnu_debug::basic_string implementation
>
> Fix and complete __gnu_debug::basic_string so that it can be used
>as a transparent
> replacement of std::basic_string.
>
> libstdc++-v3/ChangeLog:
>
> * include/debug/string
> (basic_string(const _CharT*, const _Allocator&)): Remove
>assign call.
> (basic_string<>::insert(const_iterator, _InputIte,
>_InputIte)): Try to
> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
> (__gnu_debug::u16string, __gnu_debug::u32string): New.
> (std::hash<__gnu_debug::basic_string<>>): New partial
>specialization.
>(std::__is_fast_hash<__gnu_debug::basic_string<>>): Likewise.
> (basic_string(const basic_string&, const _Alloc&)): Define
>even if !_GLIBCXX_USE_CXX11_ABI.
> (basic_string(basic_string&&, const _Alloc&)): Likewise
>and add noexcept qualification.
> (basic_string<>::erase): Adapt to take __const_iterator.
> * testsuite/21_strings/basic_string/hash/debug.cc: New test.
> * testsuite/21_strings/basic_string/hash/debug_char8_t.cc:
>New test.
> *
>testsuite/21_strings/basic_string/requirements/citerators.cc: Adapt to
>test __gnu_debug::string
> when _GLIBCXX_DEBUG is defined.
> *
>testsuite/21_strings/basic_string/requirements/dr438/constructor.cc:
>Likewise.
> *
>testsuite/21_strings/basic_string/requirements/exception/basic.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc:
>Likewise.
> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc:
>Likewise.
> *
>testsuite/21_strings/basic_string/requirements/typedefs.cc: Likewise.
> * testsuite/util/exception/safety.h
>(erase_base<__gnu_debug::basic_string<>>): New partial
> specialization.
> (insert_base<__gnu_debug::basic_string<>>): Likewise.
> * testsuite/util/testsuite_container_traits.h
>(traits<__gnu_debug::basic_string<>>): Likewise.
>
>
>Tested under Linux x86_64.
>
>Ok to commit ?
>
>François
>
>diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
>index 172179530aa..f665c759557 100644
>--- a/libstdc++-v3/include/debug/string
>+++ b/libstdc++-v3/include/debug/string
>@@ -41,6 +41,14 @@
> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
> ._M_message(#_Cond)._M_error()
>
>+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
>+#else
>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 0
>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr)
>+#endif
>+
> namespace __gnu_debug
> {
> /** Checks that __s is non-NULL or __n == 0, and then returns __s. */
>@@ -123,21 +131,21 @@ namespace __gnu_debug
>
> using _Base::npos;
>
>- basic_string()
>- _GLIBCXX_NOEXCEPT_IF(std::is_nothrow_default_constructible<_Base>::value)
>- : _Base() { }
>-
> // 21.3.1 construct/copy/destroy:
>+
> explicit
> basic_string(const _Allocator& __a) _GLIBCXX_NOEXCEPT
> : _Base(__a) { }
>
> #if __cplusplus < 201103L
>+ basic_string() : _Base() { }
>+
> basic_string(const basic_string& __str)
> : _Base(__str) { }
>
> ~basic_string() { }
> #else
>+ basic_string() = default;
> basic_string(const basic_string&) = default;
> basic_string(basic_string&&) = default;
>
>@@ -146,13 +154,15 @@ namespace __gnu_debug
> : _Base(__l, __a)
> { }
>
>-#if _GLIBCXX_USE_CXX11_ABI
> basic_string(const basic_string& __s, const _Allocator& __a)
> : _Base(__s, __a) { }
>
> basic_string(basic_string&& __s, const _Allocator& __a)
>- : _Base(std::move(__s), __a) { }
>-#endif
>+ noexcept( noexcept(
>+ _Base(std::declval<_Base>()), std::declval<const _Allocator&>()) )
There is a closing ')' in the wrong place here. This checks whether
_Base is nothrow_move_constructible and whether std::declval is
nothrow.
You could just use
noexcept(is_nothrow_constructible<_Base, _Base, const _Allocator&>::value)
or:
noexcept(noexcept(_Base(static_cast<_Base&&>(__s), __a)))
It's a bit confusing that we have a noexcept specifier that only
depends on _Base when the _Safe base class can also throw:
>+ : _Safe(std::move(__s._M_safe()), __a),
>+ _Base(std::move(__s._M_base()), __a)
>+ { }
In fact, it is conditionally noexcept with the same condition as the
_Base type, so checking if the _Base construction is non-throwing is
correct. But confusing.
>+ /// std::hash specialization for __gnu_debug::basic_string.
>+ template<typename _CharT>
>+ struct hash<__gnu_debug::basic_string<_CharT>>
>+ : public __hash_base<size_t, __gnu_debug::basic_string<_CharT>>
>+ {
>+ size_t
>+ operator()(const __gnu_debug::basic_string<_CharT>& __s) const noexcept
>+ { return std::hash<std::basic_string<_CharT>>()(__s); }
>+ };
>+
>+ template<typename _CharT>
>+ struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
>+ : std::false_type
This says it's always a slow hash, rather than deferring to
__is_fast_hash<hash<std::basic_string<C>>>.
That means __is_fast_hash is false for __gnu_debug::basic_string<int>
but true for std::basic_string<int>. In practice it probably doesn't
matter, but it's inconsistent.
>+ { };
>+
>+_GLIBCXX_END_NAMESPACE_VERSION
>+}
>+#endif /* C++11 */
>+
>+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR
>+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY
>+
> #endif
>diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>new file mode 100644
>index 00000000000..84c989590b7
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>@@ -0,0 +1,53 @@
>+// Copyright (C) 2021 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-options "-std=gnu++17" }
>+// { dg-do run { target c++17 } }
-std=gnu++17 is the default now, so should not be given explicitly.
You could combine this test and debug/hash_char8_t.cc by adding the
char8_t parts guarded by _GLIBCXX_USE_CHAR8_T. When the test is run
with a -std=gnu++20 it will test the char8_t parts (which is why we
don't want the redundant -std=gnu++17, because it would prevent it
from being run with -std=gnu++20).
>diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>index 99bf5726dcc..69d4a8d0e51 100644
>--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>@@ -18,14 +18,21 @@
> // with this library; see the file COPYING3. If not see
> // <http://www.gnu.org/licenses/>.
>
>-#include <string>
> #include <testsuite_containers.h>
>
>+#ifdef _GLIBCXX_DEBUG
>+# include <debug/string>
>+using namespace __gnu_debug;
>+#else
>+# include <string>
>+using namespace std;
>+#endif
This has the same problem I pointed out previously. With this change,
we don't run this test for std::string in debug mode. When debug
mode is active, we test a different type not std::string.
That means if we introduce a bug to std::string that only affects
debug mode, we might not notice, because we're stopped testing
std::string in debug mode.
If you want to test __gnu_debug::basic_string then those tests should
be added as new tests, not by replacing existing tests that are
already testing something different.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-23 15:42 ` Jonathan Wakely
@ 2021-03-24 21:48 ` François Dumont
2021-03-25 13:05 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: François Dumont @ 2021-03-24 21:48 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 12105 bytes --]
On 23/03/21 4:42 pm, Jonathan Wakely wrote:
> On 20/03/21 22:32 +0100, François Dumont wrote:
>> Following your feedback here is the simplified version. I grouped it
>> with the patch I submitted before.
>>
>>
>> On 19/03/21 8:41 pm, Jonathan Wakely wrote:
>>> I think we could just define on partial specialization that works for
>>> all cases:
>>
>> Yes, sounds better. But I relied on std::__hash_base which gives
>> directly the correct definition.
>
> But that is wrong.
>
> The requirements in https://wg21.link/unord.hash say that std::hash<T>
> must be disabled for unsupported types. With std::basic_string the
> specialization std::hash<basic_string<int>> is disabled, because there
> is no specialization for that type, so it uses the primary template of
> std::hash, which is empty:
>
> /// Primary class template hash, usable for enum types only.
> // Use with non-enum types still SFINAES.
> template<typename _Tp>
> struct hash : __hash_enum<_Tp>
> { };
>
> With your patch, std::hash<__gnu_debug::basic_string<int>> will not be
> empty. It will provide argument_type and result_type and operator()
> but calling operator() will fail to compile.
>
> My suggestion doesn't have that problem. With my suggestion,
> hash<_gnu_debug::basic_string<C>> is enabled if (and only if)
> hash<std::basic_string<C>> is enabled.
Ok, I adopted your approach then.
>
>> libstdc++: Fix and complete __gnu_debug::basic_string implementation
>>
>> Fix and complete __gnu_debug::basic_string so that it can be used
>> as a transparent
>> replacement of std::basic_string.
>>
>> libstdc++-v3/ChangeLog:
>>
>> * include/debug/string
>> (basic_string(const _CharT*, const _Allocator&)): Remove
>> assign call.
>> (basic_string<>::insert(const_iterator, _InputIte,
>> _InputIte)): Try to
>> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
>> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
>> (__gnu_debug::u16string, __gnu_debug::u32string): New.
>> (std::hash<__gnu_debug::basic_string<>>): New partial specialization.
>> (std::__is_fast_hash<__gnu_debug::basic_string<>>): Likewise.
>> (basic_string(const basic_string&, const _Alloc&)):
>> Define even if !_GLIBCXX_USE_CXX11_ABI.
>> (basic_string(basic_string&&, const _Alloc&)): Likewise
>> and add noexcept qualification.
>> (basic_string<>::erase): Adapt to take __const_iterator.
>> * testsuite/21_strings/basic_string/hash/debug.cc: New test.
>> *
>> testsuite/21_strings/basic_string/hash/debug_char8_t.cc: New test.
>> *
>> testsuite/21_strings/basic_string/requirements/citerators.cc: Adapt
>> to test __gnu_debug::string
>> when _GLIBCXX_DEBUG is defined.
>> *
>> testsuite/21_strings/basic_string/requirements/dr438/constructor.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/exception/basic.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc:
>> Likewise.
>> *
>> testsuite/21_strings/basic_string/requirements/typedefs.cc: Likewise.
>> * testsuite/util/exception/safety.h
>> (erase_base<__gnu_debug::basic_string<>>): New partial
>> specialization.
>> (insert_base<__gnu_debug::basic_string<>>): Likewise.
>> * testsuite/util/testsuite_container_traits.h
>> (traits<__gnu_debug::basic_string<>>): Likewise.
>>
>>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>
>> diff --git a/libstdc++-v3/include/debug/string
>> b/libstdc++-v3/include/debug/string
>> index 172179530aa..f665c759557 100644
>> --- a/libstdc++-v3/include/debug/string
>> +++ b/libstdc++-v3/include/debug/string
>> @@ -41,6 +41,14 @@
>> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
>> ._M_message(#_Cond)._M_error()
>>
>> +#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>> +# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
>> +# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
>> +#else
>> +# define _GLIBCXX_INSERT_RETURNS_ITERATOR 0
>> +# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr)
>> +#endif
>> +
>> namespace __gnu_debug
>> {
>> /** Checks that __s is non-NULL or __n == 0, and then returns __s. */
>> @@ -123,21 +131,21 @@ namespace __gnu_debug
>>
>> using _Base::npos;
>>
>> - basic_string()
>> -
>> _GLIBCXX_NOEXCEPT_IF(std::is_nothrow_default_constructible<_Base>::value)
>> - : _Base() { }
>> -
>> // 21.3.1 construct/copy/destroy:
>> +
>> explicit
>> basic_string(const _Allocator& __a) _GLIBCXX_NOEXCEPT
>> : _Base(__a) { }
>>
>> #if __cplusplus < 201103L
>> + basic_string() : _Base() { }
>> +
>> basic_string(const basic_string& __str)
>> : _Base(__str) { }
>>
>> ~basic_string() { }
>> #else
>> + basic_string() = default;
>> basic_string(const basic_string&) = default;
>> basic_string(basic_string&&) = default;
>>
>> @@ -146,13 +154,15 @@ namespace __gnu_debug
>> : _Base(__l, __a)
>> { }
>>
>> -#if _GLIBCXX_USE_CXX11_ABI
>> basic_string(const basic_string& __s, const _Allocator& __a)
>> : _Base(__s, __a) { }
>>
>> basic_string(basic_string&& __s, const _Allocator& __a)
>> - : _Base(std::move(__s), __a) { }
>> -#endif
>> + noexcept( noexcept(
>> + _Base(std::declval<_Base>()), std::declval<const _Allocator&>()) )
>
> There is a closing ')' in the wrong place here. This checks whether
> _Base is nothrow_move_constructible and whether std::declval is
> nothrow.
Well spotted and fixed in this patch.
The only problem left is that it is a copy/paste of __gnu_debug::vector
implementation. I'll submit a patch for this and maybe for other debug
containers that are just missing their noexception qualification.
>
> You could just use
> noexcept(is_nothrow_constructible<_Base, _Base, const
> _Allocator&>::value)
> or:
> noexcept(noexcept(_Base(static_cast<_Base&&>(__s), __a)))
>
> It's a bit confusing that we have a noexcept specifier that only
> depends on _Base when the _Safe base class can also throw:
>
>> + : _Safe(std::move(__s._M_safe()), __a),
>> + _Base(std::move(__s._M_base()), __a)
>> + { }
>
> In fact, it is conditionally noexcept with the same condition as the
> _Base type, so checking if the _Base construction is non-throwing is
> correct. But confusing.
>
>> + /// std::hash specialization for __gnu_debug::basic_string.
>> + template<typename _CharT>
>> + struct hash<__gnu_debug::basic_string<_CharT>>
>> + : public __hash_base<size_t, __gnu_debug::basic_string<_CharT>>
>> + {
>> + size_t
>> + operator()(const __gnu_debug::basic_string<_CharT>& __s) const
>> noexcept
>> + { return std::hash<std::basic_string<_CharT>>()(__s); }
>> + };
>> +
>> + template<typename _CharT>
>> + struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
>> + : std::false_type
>
> This says it's always a slow hash, rather than deferring to
> __is_fast_hash<hash<std::basic_string<C>>>.
>
> That means __is_fast_hash is false for __gnu_debug::basic_string<int>
> but true for std::basic_string<int>. In practice it probably doesn't
> matter, but it's inconsistent.
>
>> + { };
>> +
>> +_GLIBCXX_END_NAMESPACE_VERSION
>> +}
>> +#endif /* C++11 */
>> +
>> +#undef _GLIBCXX_INSERT_RETURNS_ITERATOR
>> +#undef _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY
>> +
>> #endif
>> diff --git
>> a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>> b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>> new file mode 100644
>> index 00000000000..84c989590b7
>> --- /dev/null
>> +++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>> @@ -0,0 +1,53 @@
>> +// Copyright (C) 2021 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-options "-std=gnu++17" }
>> +// { dg-do run { target c++17 } }
>
> -std=gnu++17 is the default now, so should not be given explicitly.
>
> You could combine this test and debug/hash_char8_t.cc by adding the
> char8_t parts guarded by _GLIBCXX_USE_CHAR8_T. When the test is run
> with a -std=gnu++20 it will test the char8_t parts (which is why we
> don't want the redundant -std=gnu++17, because it would prevent it
> from being run with -std=gnu++20).
>
>
>> diff --git
>> a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>> b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>>
>> index 99bf5726dcc..69d4a8d0e51 100644
>> ---
>> a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>> +++
>> b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>> @@ -18,14 +18,21 @@
>> // with this library; see the file COPYING3. If not see
>> // <http://www.gnu.org/licenses/>.
>>
>> -#include <string>
>> #include <testsuite_containers.h>
>>
>> +#ifdef _GLIBCXX_DEBUG
>> +# include <debug/string>
>> +using namespace __gnu_debug;
>> +#else
>> +# include <string>
>> +using namespace std;
>> +#endif
>
> This has the same problem I pointed out previously. With this change,
> we don't run this test for std::string in debug mode. When debug
> mode is active, we test a different type not std::string.
>
> That means if we introduce a bug to std::string that only affects
> debug mode, we might not notice, because we're stopped testing
> std::string in debug mode.
>
> If you want to test __gnu_debug::basic_string then those tests should
> be added as new tests, not by replacing existing tests that are
> already testing something different.
>
So I added tests on __gnu_debug::basic_string along with the ones on
std::basic_string.
The nice effect of this is that it found a bug in exception safety
testsuite utils, we could be trying to erase the past-the-end iterator.
I still need to find out why, when running test on
__gnu_debug::basic_string after the std::basic_string one, the
generate(sz) call always returns sz.
Tested under Linux x86_64.
Ok to commit ?
François
[-- Attachment #2: debug_string.patch --]
[-- Type: text/x-patch, Size: 20927 bytes --]
diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
index 172179530aa..8744a55be64 100644
--- a/libstdc++-v3/include/debug/string
+++ b/libstdc++-v3/include/debug/string
@@ -41,6 +41,14 @@
__gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
._M_message(#_Cond)._M_error()
+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
+#else
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 0
+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr)
+#endif
+
namespace __gnu_debug
{
/** Checks that __s is non-NULL or __n == 0, and then returns __s. */
@@ -123,21 +131,21 @@ namespace __gnu_debug
using _Base::npos;
- basic_string()
- _GLIBCXX_NOEXCEPT_IF(std::is_nothrow_default_constructible<_Base>::value)
- : _Base() { }
-
// 21.3.1 construct/copy/destroy:
+
explicit
basic_string(const _Allocator& __a) _GLIBCXX_NOEXCEPT
: _Base(__a) { }
#if __cplusplus < 201103L
+ basic_string() : _Base() { }
+
basic_string(const basic_string& __str)
: _Base(__str) { }
~basic_string() { }
#else
+ basic_string() = default;
basic_string(const basic_string&) = default;
basic_string(basic_string&&) = default;
@@ -146,13 +154,15 @@ namespace __gnu_debug
: _Base(__l, __a)
{ }
-#if _GLIBCXX_USE_CXX11_ABI
basic_string(const basic_string& __s, const _Allocator& __a)
: _Base(__s, __a) { }
basic_string(basic_string&& __s, const _Allocator& __a)
- : _Base(std::move(__s), __a) { }
-#endif
+ noexcept(
+ std::is_nothrow_constructible<_Base, _Base, const _Allocator&>::value )
+ : _Safe(std::move(__s._M_safe()), __a),
+ _Base(std::move(__s._M_base()), __a)
+ { }
~basic_string() = default;
@@ -178,7 +188,7 @@ namespace __gnu_debug
basic_string(const _CharT* __s, const _Allocator& __a = _Allocator())
: _Base(__glibcxx_check_string_constructor(__s), __a)
- { this->assign(__s); }
+ { }
basic_string(size_type __n, _CharT __c,
const _Allocator& __a = _Allocator())
@@ -635,15 +645,22 @@ namespace __gnu_debug
__glibcxx_check_insert_range(__p, __first, __last, __dist);
typename _Base::iterator __res;
-#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
+#if ! _GLIBCXX_INSERT_RETURNS_ITERATOR
+ const size_type __offset = __p.base() - _Base::begin();
+#endif
if (__dist.second >= __dp_sign)
- __res = _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
- __gnu_debug::__unsafe(__last));
+ {
+ _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(__res =)
+ _Base::insert(__p.base(), __gnu_debug::__unsafe(__first),
+ __gnu_debug::__unsafe(__last));
+ }
else
- __res = _Base::insert(__p.base(), __first, __last);
-#else
- const size_type __offset = __p.base() - _Base::begin();
- _Base::insert(__p.base(), __first, __last);
+ {
+ _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(__res =)
+ _Base::insert(__p.base(), __first, __last);
+ }
+
+#if ! _GLIBCXX_INSERT_RETURNS_ITERATOR
__res = _Base::begin() + __offset;
#endif
this->_M_invalidate_all();
@@ -676,7 +693,7 @@ namespace __gnu_debug
}
iterator
- erase(iterator __position)
+ erase(__const_iterator __position)
{
__glibcxx_check_erase(__position);
typename _Base::iterator __res = _Base::erase(__position.base());
@@ -685,7 +702,7 @@ namespace __gnu_debug
}
iterator
- erase(iterator __first, iterator __last)
+ erase(__const_iterator __first, __const_iterator __last)
{
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 151. can't currently clear() empty container
@@ -1285,6 +1302,19 @@ namespace __gnu_debug
typedef basic_string<wchar_t> wstring;
#endif
+#ifdef _GLIBCXX_USE_CHAR8_T
+ /// A string of @c char8_t
+ typedef basic_string<char8_t> u8string;
+#endif
+
+#if __cplusplus >= 201103L
+ /// A string of @c char16_t
+ typedef basic_string<char16_t> u16string;
+
+ /// A string of @c char32_t
+ typedef basic_string<char32_t> u32string;
+#endif
+
template<typename _CharT, typename _Traits, typename _Allocator>
struct _Insert_range_from_self_is_safe<
__gnu_debug::basic_string<_CharT, _Traits, _Allocator> >
@@ -1292,4 +1322,27 @@ namespace __gnu_debug
} // namespace __gnu_debug
+#if __cplusplus >= 201103L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+ /// std::hash specialization for __gnu_debug::basic_string.
+ template<typename _CharT>
+ struct hash<__gnu_debug::basic_string<_CharT>>
+ : public hash<std::basic_string<_CharT>>
+ { };
+
+ template<typename _CharT>
+ struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
+ : __is_fast_hash<hash<std::basic_string<_CharT>>>
+ { };
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif /* C++11 */
+
+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR
+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY
+
#endif
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
new file mode 100644
index 00000000000..596bff9f8c6
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
@@ -0,0 +1,69 @@
+// Copyright (C) 2021 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 { target c++17 } }
+
+#include <debug/string>
+#include <memory_resource>
+#include <testsuite_hooks.h>
+
+// C++17 24.3.5 [basic.string.hash]
+// If S is one of these string types, SV is the corresponding string view type,
+// and s is an object of type S, then hash<S>()(s) == hash<SV>()(SV(s)).
+
+template<typename S>
+ bool
+ test(const S& s)
+ {
+ using std::hash;
+ using SV = std::basic_string_view<typename S::value_type>;
+ return hash<S>()(s) == hash<SV>()(SV(s));
+ }
+
+void
+test01()
+{
+ VERIFY( test(__gnu_debug::string("a narrow string")) );
+#if _GLIBCXX_USE_CHAR8_T
+ VERIFY( test(__gnu_debug::u8string(u8"a narrow string")) );
+#endif
+ VERIFY( test(__gnu_debug::u16string(u"a utf-16 string")) );
+ VERIFY( test(__gnu_debug::u32string(U"a utf-32 string")) );
+#if _GLIBCXX_USE_WCHAR_T
+ VERIFY( test(__gnu_debug::wstring(L"a wide string")) );
+#endif
+}
+
+#if _GLIBCXX_USE_CHAR8_T
+void
+test02()
+{
+ using std::hash;
+ __gnu_debug::string native("a string, a string, my stringdom for a string");
+ __gnu_debug::u8string utf8(u8"a string, a string, my stringdom for a string");
+ VERIFY( hash<__gnu_debug::string>()(native) == hash<__gnu_debug::u8string>()(utf8) );
+}
+#endif
+
+int
+main()
+{
+ test01();
+#if _GLIBCXX_USE_CHAR8_T
+ test02();
+#endif
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
index 99bf5726dcc..fca2a8940e9 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
@@ -19,13 +19,17 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
+
#include <testsuite_containers.h>
int main()
{
__gnu_test::citerator<std::string> test1;
+ __gnu_test::citerator<__gnu_debug::string> dtest1;
#ifdef _GLIBCXX_USE_WCHAR_T
__gnu_test::citerator<std::wstring> test2;
+ __gnu_test::citerator<__gnu_debug::wstring> dtest2;
#endif
return 0;
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
index 82d42ebb6a6..059e32aa970 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/dr438/constructor.cc
@@ -20,8 +20,10 @@
// { dg-do compile }
#include <string>
+#include <debug/string>
void f()
{
std::string s(10, 1);
+ __gnu_debug::string ds(10, 1);
}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
index 2b6e27432e8..2ff020d81d4 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/basic.cc
@@ -21,6 +21,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
#include <exception/safety.h>
void
@@ -31,8 +32,16 @@ value()
typedef char value_type;
typedef __gnu_cxx::throw_allocator_limit<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
- __gnu_test::basic_safety<test_type> test;
+
+ {
+ typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::basic_safety<test_type> test;
+ }
+
+ {
+ typedef __gnu_debug::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::basic_safety<test_type> test;
+ }
}
// Container requirement testing, exceptional behavior
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
index 07d2a2e2074..02d3242db59 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc
@@ -21,6 +21,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
#include <exception/safety.h>
void
@@ -29,8 +30,16 @@ char_allocator()
typedef char value_type;
typedef __gnu_cxx::throw_allocator_random<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
- __gnu_test::generation_prohibited<test_type> test;
+
+ {
+ typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::generation_prohibited<test_type> test;
+ }
+
+ {
+ typedef __gnu_debug::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::generation_prohibited<test_type> test;
+ }
}
void
@@ -39,8 +48,16 @@ wchar_allocator()
typedef wchar_t value_type;
typedef __gnu_cxx::throw_allocator_random<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
- __gnu_test::generation_prohibited<test_type> test;
+
+ {
+ typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::generation_prohibited<test_type> test;
+ }
+
+ {
+ typedef __gnu_debug::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::generation_prohibited<test_type> test;
+ }
}
// Container requirement testing, exceptional behavior
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
index ce99dab915e..0526745cb91 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc
@@ -21,6 +21,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
#include <exception/safety.h>
void
@@ -31,8 +32,16 @@ value()
typedef char value_type;
typedef __gnu_cxx::throw_allocator_limit<value_type> allocator_type;
typedef std::char_traits<value_type> traits_type;
- typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
- __gnu_test::propagation_consistent<test_type> test;
+
+ {
+ typedef std::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::propagation_consistent<test_type> test;
+ }
+
+ {
+ typedef __gnu_debug::basic_string<value_type, traits_type, allocator_type> test_type;
+ __gnu_test::propagation_consistent<test_type> test;
+ }
}
// Container requirement testing, exceptional behavior
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
index 275354c1f4c..9d2ca54c5df 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc
@@ -18,5 +18,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
template class std::basic_string<char>;
+template class __gnu_debug::basic_string<char>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
index 2b5139de533..64030165d2b 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc
@@ -18,5 +18,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
template class std::basic_string<char16_t>;
+template class __gnu_debug::basic_string<char16_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
index 0fa685e9094..6e12fadf8cd 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc
@@ -18,5 +18,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
template class std::basic_string<char32_t>;
+template class __gnu_debug::basic_string<char32_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
index db88b0f47a3..abab7972b8d 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc
@@ -19,5 +19,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
template class std::basic_string<char8_t>;
+template class __gnu_debug::basic_string<char8_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
index 7f16ca4cb5b..1a25fa839d4 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc
@@ -18,5 +18,7 @@
// <http://www.gnu.org/licenses/>.
#include <string>
+#include <debug/string>
template class std::basic_string<wchar_t>;
+template class __gnu_debug::basic_string<wchar_t>;
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
index 2fc313b9cbf..ff5f98e7805 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/typedefs.cc
@@ -19,9 +19,12 @@
#include <testsuite_containers.h>
#include <string>
+#include <debug/string>
// Check container for required typedefs.
__gnu_test::types<std::string> t1;
+__gnu_test::types<__gnu_debug::string> dt1;
#ifdef _GLIBCXX_USE_WCHAR_T
__gnu_test::types<std::wstring> t2;
+__gnu_test::types<__gnu_debug::wstring> dt2;
#endif
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 6c91e740e0d..2d6d894f001 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -266,6 +266,20 @@ namespace __gnu_test
iterator (container_type::* _F_erase_point)(iterator);
iterator (container_type::* _F_erase_range)(iterator, iterator);
+ erase_base()
+ : _F_erase_point(&container_type::erase),
+ _F_erase_range(&container_type::erase) { }
+ };
+
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct erase_base<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>>
+ {
+ typedef __gnu_debug::basic_string<_Tp1, _Tp2, _Tp3> container_type;
+ typedef typename container_type::iterator iterator;
+
+ iterator (container_type::* _F_erase_point)(iterator);
+ iterator (container_type::* _F_erase_range)(iterator, iterator);
+
erase_base()
: _F_erase_point(&container_type::erase),
_F_erase_range(&container_type::erase) { }
@@ -312,7 +326,7 @@ namespace __gnu_test
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
// Makes it easier to think of this as __container.erase(i)
(__container.*_F_erase_point)(i);
@@ -340,7 +354,7 @@ namespace __gnu_test
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.before_begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
// Makes it easier to think of this as __container.erase(i)
(__container.*_F_erase_point)(i);
@@ -681,6 +695,24 @@ namespace __gnu_test
typedef typename container_type::const_iterator const_iterator;
typedef typename container_type::value_type value_type;
+#if _GLIBCXX_USE_CXX11_ABI == 0 || __cplusplus < 201103L
+ iterator (container_type::* _F_insert_point)(iterator, value_type);
+#else
+ iterator (container_type::* _F_insert_point)(const_iterator,
+ value_type);
+#endif
+
+ insert_base() : _F_insert_point(&container_type::insert) { }
+ };
+
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct insert_base<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>>
+ {
+ typedef __gnu_debug::basic_string<_Tp1, _Tp2, _Tp3> container_type;
+ typedef typename container_type::iterator iterator;
+ typedef typename container_type::const_iterator const_iterator;
+ typedef typename container_type::value_type value_type;
+
#if _GLIBCXX_USE_CXX11_ABI == 0 || __cplusplus < 201103L
iterator (container_type::* _F_insert_point)(iterator, value_type);
#else
diff --git a/libstdc++-v3/testsuite/util/testsuite_container_traits.h b/libstdc++-v3/testsuite/util/testsuite_container_traits.h
index fb1f558011c..0bc7a2aa53a 100644
--- a/libstdc++-v3/testsuite/util/testsuite_container_traits.h
+++ b/libstdc++-v3/testsuite/util/testsuite_container_traits.h
@@ -22,6 +22,7 @@
#include <bits/stdc++.h>
#include <ext/vstring.h>
+#include <debug/string>
namespace __gnu_test
{
@@ -128,6 +129,17 @@ namespace __gnu_test
typedef std::true_type has_insert;
};
+ template<typename _Tp1, typename _Tp2, typename _Tp3>
+ struct traits<__gnu_debug::basic_string<_Tp1, _Tp2, _Tp3>> : public traits_base
+ {
+ typedef std::true_type is_container;
+ typedef std::true_type is_reversible;
+ typedef std::true_type is_allocator_aware;
+
+ typedef std::true_type has_erase;
+ typedef std::true_type has_insert;
+ };
+
template<typename _Tp1, typename _Tp2, typename _Tp3,
template <typename, typename, typename> class _Tp4>
struct traits<__gnu_cxx::__versa_string<_Tp1, _Tp2, _Tp3, _Tp4>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-24 21:48 ` François Dumont
@ 2021-03-25 13:05 ` Jonathan Wakely
2021-03-25 14:48 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-25 13:05 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 24/03/21 22:48 +0100, François Dumont wrote:
>On 23/03/21 4:42 pm, Jonathan Wakely wrote:
>>On 20/03/21 22:32 +0100, François Dumont wrote:
>>>Following your feedback here is the simplified version. I grouped
>>>it with the patch I submitted before.
>>>
>>>
>>>On 19/03/21 8:41 pm, Jonathan Wakely wrote:
>>>>I think we could just define on partial specialization that works for
>>>>all cases:
>>>
>>>Yes, sounds better. But I relied on std::__hash_base which gives
>>>directly the correct definition.
>>
>>But that is wrong.
>>
>>The requirements in https://wg21.link/unord.hash say that std::hash<T>
>>must be disabled for unsupported types. With std::basic_string the
>>specialization std::hash<basic_string<int>> is disabled, because there
>>is no specialization for that type, so it uses the primary template of
>>std::hash, which is empty:
>>
>> /// Primary class template hash, usable for enum types only.
>> // Use with non-enum types still SFINAES.
>> template<typename _Tp>
>> struct hash : __hash_enum<_Tp>
>> { };
>>
>>With your patch, std::hash<__gnu_debug::basic_string<int>> will not be
>>empty. It will provide argument_type and result_type and operator()
>>but calling operator() will fail to compile.
>>
>>My suggestion doesn't have that problem. With my suggestion,
>>hash<_gnu_debug::basic_string<C>> is enabled if (and only if)
>>hash<std::basic_string<C>> is enabled.
>
>Ok, I adopted your approach then.
>
>
>>
>>> libstdc++: Fix and complete __gnu_debug::basic_string implementation
>>>
>>> Fix and complete __gnu_debug::basic_string so that it can be
>>>used as a transparent
>>> replacement of std::basic_string.
>>>
>>> libstdc++-v3/ChangeLog:
>>>
>>> * include/debug/string
>>> (basic_string(const _CharT*, const _Allocator&)):
>>>Remove assign call.
>>> (basic_string<>::insert(const_iterator, _InputIte,
>>>_InputIte)): Try to
>>> remove iterator debug layer even if !_GLIBCXX_USE_CXX11_ABI.
>>> [_GLIBCXX_USE_CHAR8_T] (__gnu_debug::u8string): New.
>>> (__gnu_debug::u16string, __gnu_debug::u32string): New.
>>>(std::hash<__gnu_debug::basic_string<>>): New partial specialization.
>>>(std::__is_fast_hash<__gnu_debug::basic_string<>>): Likewise.
>>> (basic_string(const basic_string&, const _Alloc&)):
>>>Define even if !_GLIBCXX_USE_CXX11_ABI.
>>> (basic_string(basic_string&&, const _Alloc&)):
>>>Likewise and add noexcept qualification.
>>> (basic_string<>::erase): Adapt to take __const_iterator.
>>> * testsuite/21_strings/basic_string/hash/debug.cc: New test.
>>> *
>>>testsuite/21_strings/basic_string/hash/debug_char8_t.cc: New test.
>>> *
>>>testsuite/21_strings/basic_string/requirements/citerators.cc:
>>>Adapt to test __gnu_debug::string
>>> when _GLIBCXX_DEBUG is defined.
>>> *
>>>testsuite/21_strings/basic_string/requirements/dr438/constructor.cc:
>>>Likewise.
>>> *
>>>testsuite/21_strings/basic_string/requirements/exception/basic.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/exception/generation_prohibited.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/exception/propagation_consistent.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char/1.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char16_t/1.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char32_t/1.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/char8_t/1.cc:
>>>Likewise.
>>> * testsuite/21_strings/basic_string/requirements/explicit_instantiation/wchar_t/1.cc:
>>>Likewise.
>>> *
>>>testsuite/21_strings/basic_string/requirements/typedefs.cc:
>>>Likewise.
>>> * testsuite/util/exception/safety.h
>>>(erase_base<__gnu_debug::basic_string<>>): New partial
>>> specialization.
>>>(insert_base<__gnu_debug::basic_string<>>): Likewise.
>>> * testsuite/util/testsuite_container_traits.h
>>>(traits<__gnu_debug::basic_string<>>): Likewise.
>>>
>>>
>>>Tested under Linux x86_64.
>>>
>>>Ok to commit ?
>>>
>>>François
>>>
>>
>>>diff --git a/libstdc++-v3/include/debug/string
>>>b/libstdc++-v3/include/debug/string
>>>index 172179530aa..f665c759557 100644
>>>--- a/libstdc++-v3/include/debug/string
>>>+++ b/libstdc++-v3/include/debug/string
>>>@@ -41,6 +41,14 @@
>>> __gnu_debug::_Error_formatter::_S_at(_File, _Line, _Func) \
>>> ._M_message(#_Cond)._M_error()
>>>
>>>+#if _GLIBCXX_USE_CXX11_ABI && __cplusplus >= 201103
>>>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 1
>>>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr) expr
>>>+#else
>>>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR 0
>>>+# define _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY(expr)
>>>+#endif
>>>+
>>>namespace __gnu_debug
>>>{
>>> /** Checks that __s is non-NULL or __n == 0, and then returns __s. */
>>>@@ -123,21 +131,21 @@ namespace __gnu_debug
>>>
>>> using _Base::npos;
>>>
>>>- basic_string()
>>>- _GLIBCXX_NOEXCEPT_IF(std::is_nothrow_default_constructible<_Base>::value)
>>>- : _Base() { }
>>>-
>>> // 21.3.1 construct/copy/destroy:
>>>+
>>> explicit
>>> basic_string(const _Allocator& __a) _GLIBCXX_NOEXCEPT
>>> : _Base(__a) { }
>>>
>>>#if __cplusplus < 201103L
>>>+ basic_string() : _Base() { }
>>>+
>>> basic_string(const basic_string& __str)
>>> : _Base(__str) { }
>>>
>>> ~basic_string() { }
>>>#else
>>>+ basic_string() = default;
>>> basic_string(const basic_string&) = default;
>>> basic_string(basic_string&&) = default;
>>>
>>>@@ -146,13 +154,15 @@ namespace __gnu_debug
>>> : _Base(__l, __a)
>>> { }
>>>
>>>-#if _GLIBCXX_USE_CXX11_ABI
>>> basic_string(const basic_string& __s, const _Allocator& __a)
>>> : _Base(__s, __a) { }
>>>
>>> basic_string(basic_string&& __s, const _Allocator& __a)
>>>- : _Base(std::move(__s), __a) { }
>>>-#endif
>>>+ noexcept( noexcept(
>>>+ _Base(std::declval<_Base>()), std::declval<const _Allocator&>()) )
>>
>>There is a closing ')' in the wrong place here. This checks whether
>>_Base is nothrow_move_constructible and whether std::declval is
>>nothrow.
>
>Well spotted and fixed in this patch.
>
>The only problem left is that it is a copy/paste of
>__gnu_debug::vector implementation. I'll submit a patch for this and
>maybe for other debug containers that are just missing their
>noexception qualification.
>
>
>>
>>You could just use
>> noexcept(is_nothrow_constructible<_Base, _Base, const
>>_Allocator&>::value)
>>or:
>> noexcept(noexcept(_Base(static_cast<_Base&&>(__s), __a)))
>>
>>It's a bit confusing that we have a noexcept specifier that only
>>depends on _Base when the _Safe base class can also throw:
>>
>>>+ : _Safe(std::move(__s._M_safe()), __a),
>>>+ _Base(std::move(__s._M_base()), __a)
>>>+ { }
>>
>>In fact, it is conditionally noexcept with the same condition as the
>>_Base type, so checking if the _Base construction is non-throwing is
>>correct. But confusing.
>>
>>>+ /// std::hash specialization for __gnu_debug::basic_string.
>>>+ template<typename _CharT>
>>>+ struct hash<__gnu_debug::basic_string<_CharT>>
>>>+ : public __hash_base<size_t, __gnu_debug::basic_string<_CharT>>
>>>+ {
>>>+ size_t
>>>+ operator()(const __gnu_debug::basic_string<_CharT>& __s)
>>>const noexcept
>>>+ { return std::hash<std::basic_string<_CharT>>()(__s); }
>>>+ };
>>>+
>>>+ template<typename _CharT>
>>>+ struct __is_fast_hash<hash<__gnu_debug::basic_string<_CharT>>>
>>>+ : std::false_type
>>
>>This says it's always a slow hash, rather than deferring to
>>__is_fast_hash<hash<std::basic_string<C>>>.
>>
>>That means __is_fast_hash is false for __gnu_debug::basic_string<int>
>>but true for std::basic_string<int>. In practice it probably doesn't
>>matter, but it's inconsistent.
>>
>>>+ { };
>>>+
>>>+_GLIBCXX_END_NAMESPACE_VERSION
>>>+}
>>>+#endif /* C++11 */
>>>+
>>>+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR
>>>+#undef _GLIBCXX_INSERT_RETURNS_ITERATOR_ONLY
>>>+
>>>#endif
>>>diff --git
>>>a/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>>>b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>>>new file mode 100644
>>>index 00000000000..84c989590b7
>>>--- /dev/null
>>>+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/debug.cc
>>>@@ -0,0 +1,53 @@
>>>+// Copyright (C) 2021 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-options "-std=gnu++17" }
>>>+// { dg-do run { target c++17 } }
>>
>>-std=gnu++17 is the default now, so should not be given explicitly.
>>
>>You could combine this test and debug/hash_char8_t.cc by adding the
>>char8_t parts guarded by _GLIBCXX_USE_CHAR8_T. When the test is run
>>with a -std=gnu++20 it will test the char8_t parts (which is why we
>>don't want the redundant -std=gnu++17, because it would prevent it
>>from being run with -std=gnu++20).
>>
>>
>>>diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>>>
>>>index 99bf5726dcc..69d4a8d0e51 100644
>>>--- a/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>>>+++ b/libstdc++-v3/testsuite/21_strings/basic_string/requirements/citerators.cc
>>>@@ -18,14 +18,21 @@
>>>// with this library; see the file COPYING3. If not see
>>>// <http://www.gnu.org/licenses/>.
>>>
>>>-#include <string>
>>>#include <testsuite_containers.h>
>>>
>>>+#ifdef _GLIBCXX_DEBUG
>>>+# include <debug/string>
>>>+using namespace __gnu_debug;
>>>+#else
>>>+# include <string>
>>>+using namespace std;
>>>+#endif
>>
>>This has the same problem I pointed out previously. With this change,
>>we don't run this test for std::string in debug mode. When debug
>>mode is active, we test a different type not std::string.
>>
>>That means if we introduce a bug to std::string that only affects
>>debug mode, we might not notice, because we're stopped testing
>>std::string in debug mode.
>>
>>If you want to test __gnu_debug::basic_string then those tests should
>>be added as new tests, not by replacing existing tests that are
>>already testing something different.
>>
>So I added tests on __gnu_debug::basic_string along with the ones on
>std::basic_string.
Oh, I like this appraoch. Thanks.
>The nice effect of this is that it found a bug in exception safety
>testsuite utils, we could be trying to erase the past-the-end
>iterator.
Nice.
>I still need to find out why, when running test on
>__gnu_debug::basic_string after the std::basic_string one, the
>generate(sz) call always returns sz.
The "random" generator will always return the same sequence of numbers
every time you run the test. It uses a default-constructed
std::mt19937 without a seed, so the sequence of random numbers is 100%
reproducable.
>Tested under Linux x86_64.
>
>Ok to commit ?
OK thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-25 13:05 ` Jonathan Wakely
@ 2021-03-25 14:48 ` Jonathan Wakely
2021-03-25 15:29 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-25 14:48 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
On 25/03/21 13:05 +0000, Jonathan Wakely wrote:
>On 24/03/21 22:48 +0100, François Dumont wrote:
>>I still need to find out why, when running test on
>>__gnu_debug::basic_string after the std::basic_string one, the
>>generate(sz) call always returns sz.
>
>The "random" generator will always return the same sequence of numbers
>every time you run the test. It uses a default-constructed
>std::mt19937 without a seed, so the sequence of random numbers is 100%
>reproducable.
This patch allows those random engines to be seeded, so that we can test
with different random numbers.
It's already found a bug:
GLIBCXX_SEED_TEST_RNG=-941908610 make check RUNTESTFLAGS=conformance.exp=23_containers/forward_list/requirements/exception/generation_prohibited.cc
Using random seed 3353058686
FAIL: 23_containers/forward_list/requirements/exception/generation_prohibited.cc execution test
We need to investigate that.
Any objections to pushing this?
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4178 bytes --]
commit 172f60c7067155beaa3c8c768677f7c26b7d4da9
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 25 13:51:08 2021
libstdc++: Allow seeding random engines in testsuite
The testsuite utilities that use random numbers use a
default-constructed mersenne_twister_engine, meaning the values are
reproducable. This adds support for seeding them, controlledby an
environment variable. Defining GLIBCXX_SEED_TEST_RNG=val in the
environment will cause the engines to be seeded with atoi(val) if that
is non-zero, or with a value read from std::random_device otherwise.
libstdc++-v3/ChangeLog:
* testsuite/util/exception/safety.h (setup_base::generate):
Support seeding random engine.
* testsuite/util/testsuite_containergen.h (test_containers):
Likewise.
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 6c91e740e0d..16a784e4908 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -22,6 +22,8 @@
#include <testsuite_container_traits.h>
#include <ext/throw_allocator.h>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
// Container requirement testing.
namespace __gnu_test
@@ -33,27 +35,34 @@ namespace __gnu_test
typedef std::uniform_int_distribution<size_type> distribution_type;
typedef std::mt19937 engine_type;
+ static engine_type
+ get_engine()
+ {
+ engine_type engine;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ unsigned s = std::atoi(v);
+ if (s == 0)
+ s = std::random_device{}();
+ std::printf("Using random seed %u\n", s);
+ std::fflush(stdout);
+ engine.seed(s);
+ }
+ return engine;
+ }
+
// Return randomly generated integer on range [0, __max_size].
static size_type
generate(size_type __max_size)
{
- // Make the generator static...
- const engine_type engine;
- const distribution_type distribution;
- static auto generator = std::bind(distribution, engine,
- std::placeholders::_1);
+ using param_type = typename distribution_type::param_type;
- // ... but set the range for this particular invocation here.
- const typename distribution_type::param_type p(0, __max_size);
- size_type random = generator(p);
- if (random < distribution.min() || random > distribution.max())
- std::__throw_out_of_range_fmt(__N("setup_base::generate\n"
- "random number generated is: %zu "
- "out of range [%zu, %zu]\n"),
- (size_t)random,
- (size_t)distribution.min(),
- (size_t)distribution.max());
- return random;
+ // Make the engine and distribution static...
+ static engine_type engine = get_engine();
+ static distribution_type distribution;
+ return distribution(engine, param_type{0, __max_size});
}
// Given an instantiating type, return a unique value.
diff --git a/libstdc++-v3/testsuite/util/testsuite_containergen.h b/libstdc++-v3/testsuite/util/testsuite_containergen.h
index a2156733ec6..5bbe620d59d 100644
--- a/libstdc++-v3/testsuite/util/testsuite_containergen.h
+++ b/libstdc++-v3/testsuite/util/testsuite_containergen.h
@@ -20,6 +20,8 @@
#include <testsuite_container_traits.h>
#include <random>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
namespace __gnu_test
{
@@ -63,6 +65,18 @@ namespace __gnu_test
{
std::mt19937_64 random_gen;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ unsigned s = std::atoi(v);
+ if (s == 0)
+ s = std::random_device{}();
+ std::printf("Using random seed %u\n", s);
+ std::fflush(stdout);
+ random_gen.seed(s);
+ }
+
#ifdef SIMULATOR_TEST
int loops = 10;
#else
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-25 14:48 ` Jonathan Wakely
@ 2021-03-25 15:29 ` Jonathan Wakely
2021-03-25 17:45 ` François Dumont
2021-03-29 20:25 ` François Dumont
0 siblings, 2 replies; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-25 15:29 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]
On 25/03/21 14:48 +0000, Jonathan Wakely wrote:
>On 25/03/21 13:05 +0000, Jonathan Wakely wrote:
>>On 24/03/21 22:48 +0100, François Dumont wrote:
>>>I still need to find out why, when running test on
>>>__gnu_debug::basic_string after the std::basic_string one, the
>>>generate(sz) call always returns sz.
>>
>>The "random" generator will always return the same sequence of numbers
>>every time you run the test. It uses a default-constructed
>>std::mt19937 without a seed, so the sequence of random numbers is 100%
>>reproducable.
>
>This patch allows those random engines to be seeded, so that we can test
>with different random numbers.
>
>It's already found a bug:
>
>GLIBCXX_SEED_TEST_RNG=-941908610 make check RUNTESTFLAGS=conformance.exp=23_containers/forward_list/requirements/exception/generation_prohibited.cc
>
>Using random seed 3353058686
>FAIL: 23_containers/forward_list/requirements/exception/generation_prohibited.cc execution test
>
>We need to investigate that.
Oh, it's the same generate(sz) bug as you already found. But I've
found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
I think we should also add a check for non-empty containers to those
test functions, and ensure we don't try to erase from empty
containers (see attached).
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4801 bytes --]
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 16a784e4908..2e5d8acae00 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -43,12 +43,12 @@ namespace __gnu_test
{
// A single seed value is much smaller than the mt19937 state size,
// but we're not trying to be cryptographically secure here.
- unsigned s = std::atoi(v);
+ int s = std::atoi(v);
if (s == 0)
- s = std::random_device{}();
- std::printf("Using random seed %u\n", s);
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
std::fflush(stdout);
- engine.seed(s);
+ engine.seed((unsigned)s);
}
return engine;
}
@@ -318,10 +318,13 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // Container::erase(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
// Makes it easier to think of this as __container.erase(i)
(__container.*_F_erase_point)(i);
@@ -346,12 +349,15 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // forward_list::erase_after(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.before_begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
- // Makes it easier to think of this as __container.erase(i)
+ // Makes it easier to think of this as __container.erase_after(i)
(__container.*_F_erase_point)(i);
}
catch(const __gnu_cxx::forced_error&)
@@ -414,14 +420,19 @@ namespace __gnu_test
{
const size_type sz = std::distance(__container.begin(),
__container.end());
- size_type s1 = generate(sz);
- size_type s2 = generate(sz);
+ // forward_list::erase_after(pos, last) requires a pos != last
+ if (sz == 0)
+ return; // Caller doesn't check for this, not a logic error.
+
+ size_type s1 = generate(sz - 1);
+ size_type s2 = generate(sz - 1);
auto i1 = __container.before_begin();
auto i2 = __container.before_begin();
std::advance(i1, std::min(s1, s2));
- std::advance(i2, std::max(s1, s2));
+ std::advance(i2, std::max(s1, s2) + 1);
- // Makes it easier to think of this as __container.erase(i1, i2).
+ // Makes it easier to think of this as
+ // __container.erase_after(i1, i2).
(__container.*_F_erase_range)(i1, i2);
}
catch(const __gnu_cxx::forced_error&)
@@ -1463,16 +1474,25 @@ namespace __gnu_test
// constructor or assignment operator of value_type throws.
if (!traits<container_type>::has_throwing_erase::value)
{
- typename base_type::erase_point erasep;
- erasep(container);
+ if (!container.empty())
+ {
+ typename base_type::erase_point erasep;
+ erasep(container);
+ }
typename base_type::erase_range eraser;
eraser(container);
}
- typename base_type::pop_front popf;
- popf(container);
- typename base_type::pop_back popb;
- popb(container);
+ if (!container.empty())
+ {
+ typename base_type::pop_front popf;
+ popf(container);
+ }
+ if (!container.empty())
+ {
+ typename base_type::pop_back popb;
+ popb(container);
+ }
typename base_type::iterator_ops iops;
iops(container);
diff --git a/libstdc++-v3/testsuite/util/testsuite_containergen.h b/libstdc++-v3/testsuite/util/testsuite_containergen.h
index 5bbe620d59d..c468c7f4415 100644
--- a/libstdc++-v3/testsuite/util/testsuite_containergen.h
+++ b/libstdc++-v3/testsuite/util/testsuite_containergen.h
@@ -69,12 +69,12 @@ namespace __gnu_test
{
// A single seed value is much smaller than the mt19937 state size,
// but we're not trying to be cryptographically secure here.
- unsigned s = std::atoi(v);
+ int s = std::atoi(v);
if (s == 0)
- s = std::random_device{}();
- std::printf("Using random seed %u\n", s);
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
std::fflush(stdout);
- random_gen.seed(s);
+ random_gen.seed((unsigned)s);
}
#ifdef SIMULATOR_TEST
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-25 15:29 ` Jonathan Wakely
@ 2021-03-25 17:45 ` François Dumont
2021-03-25 18:22 ` Jonathan Wakely
2021-03-29 20:25 ` François Dumont
1 sibling, 1 reply; 12+ messages in thread
From: François Dumont @ 2021-03-25 17:45 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 25/03/21 4:29 pm, Jonathan Wakely wrote:
> On 25/03/21 14:48 +0000, Jonathan Wakely wrote:
>> On 25/03/21 13:05 +0000, Jonathan Wakely wrote:
>>> On 24/03/21 22:48 +0100, François Dumont wrote:
>>>> I still need to find out why, when running test on
>>>> __gnu_debug::basic_string after the std::basic_string one, the
>>>> generate(sz) call always returns sz.
>>>
>>> The "random" generator will always return the same sequence of numbers
>>> every time you run the test. It uses a default-constructed
>>> std::mt19937 without a seed, so the sequence of random numbers is 100%
>>> reproducable.
>>
>> This patch allows those random engines to be seeded, so that we can test
>> with different random numbers.
>>
>> It's already found a bug:
>>
>> GLIBCXX_SEED_TEST_RNG=-941908610 make check
>> RUNTESTFLAGS=conformance.exp=23_containers/forward_list/requirements/exception/generation_prohibited.cc
>>
>> Using random seed 3353058686
>> FAIL:
>> 23_containers/forward_list/requirements/exception/generation_prohibited.cc
>> execution test
>>
>> We need to investigate that.
>
> Oh, it's the same generate(sz) bug as you already found. But I've
> found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
>
> I think we should also add a check for non-empty containers to those
> test functions, and ensure we don't try to erase from empty
> containers (see attached).
>
>
Yes, I also realized this empty container potential issue.
Please go ahead with any of your patches, I'll just adapt if you push first.
I will commit in a couple of hours.
François
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-25 17:45 ` François Dumont
@ 2021-03-25 18:22 ` Jonathan Wakely
0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Wakely @ 2021-03-25 18:22 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]
On 25/03/21 18:45 +0100, François Dumont via Libstdc++ wrote:
>On 25/03/21 4:29 pm, Jonathan Wakely wrote:
>>On 25/03/21 14:48 +0000, Jonathan Wakely wrote:
>>>On 25/03/21 13:05 +0000, Jonathan Wakely wrote:
>>>>On 24/03/21 22:48 +0100, François Dumont wrote:
>>>>>I still need to find out why, when running test on
>>>>>__gnu_debug::basic_string after the std::basic_string one, the
>>>>>generate(sz) call always returns sz.
>>>>
>>>>The "random" generator will always return the same sequence of numbers
>>>>every time you run the test. It uses a default-constructed
>>>>std::mt19937 without a seed, so the sequence of random numbers is 100%
>>>>reproducable.
>>>
>>>This patch allows those random engines to be seeded, so that we can test
>>>with different random numbers.
>>>
>>>It's already found a bug:
>>>
>>>GLIBCXX_SEED_TEST_RNG=-941908610 make check RUNTESTFLAGS=conformance.exp=23_containers/forward_list/requirements/exception/generation_prohibited.cc
>>>
>>>Using random seed 3353058686
>>>FAIL: 23_containers/forward_list/requirements/exception/generation_prohibited.cc
>>>execution test
>>>
>>>We need to investigate that.
>>
>>Oh, it's the same generate(sz) bug as you already found. But I've
>>found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
>>
>>I think we should also add a check for non-empty containers to those
>>test functions, and ensure we don't try to erase from empty
>>containers (see attached).
>>
>>
>Yes, I also realized this empty container potential issue.
>
>Please go ahead with any of your patches, I'll just adapt if you push first.
OK, thanks. I've pushed the attached patch. You should only need to
undo the generate(sz) changes in your patch.
>I will commit in a couple of hours.
Great, thanks.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7946 bytes --]
commit c7fc73ee459045edabb99816658f14f32d23bf92
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 25 13:51:08 2021
libstdc++: Allow seeding random engines in testsuite
The testsuite utilities that use random numbers use a
default-constructed mersenne_twister_engine, meaning the values are
reproducable. This adds support for seeding them, controlledby an
environment variable. Defining GLIBCXX_SEED_TEST_RNG=val in the
environment will cause the engines to be seeded with atoi(val) if that
is non-zero, or with a value read from std::random_device otherwise.
Running with different seeds revealed some bugs in the tests, where a
randomly selected iterator was past-the-end (which can't be erased), or
where the randomly populated container was empty, and then we tried to
remove elements from it unconditionally.
libstdc++-v3/ChangeLog:
* testsuite/util/exception/safety.h (setup_base::generate):
Support seeding random engine.
(erase_point, erase_range): Adjust range of random numbers to
ensure dereferenceable iterators are used where required.
(generation_prohibited::run): Do not try to erase from empty
containers.
* testsuite/util/testsuite_containergen.h (test_containers):
Support seeding random engine.
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 6c91e740e0d..2e5d8acae00 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -22,6 +22,8 @@
#include <testsuite_container_traits.h>
#include <ext/throw_allocator.h>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
// Container requirement testing.
namespace __gnu_test
@@ -33,27 +35,34 @@ namespace __gnu_test
typedef std::uniform_int_distribution<size_type> distribution_type;
typedef std::mt19937 engine_type;
+ static engine_type
+ get_engine()
+ {
+ engine_type engine;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ int s = std::atoi(v);
+ if (s == 0)
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
+ std::fflush(stdout);
+ engine.seed((unsigned)s);
+ }
+ return engine;
+ }
+
// Return randomly generated integer on range [0, __max_size].
static size_type
generate(size_type __max_size)
{
- // Make the generator static...
- const engine_type engine;
- const distribution_type distribution;
- static auto generator = std::bind(distribution, engine,
- std::placeholders::_1);
+ using param_type = typename distribution_type::param_type;
- // ... but set the range for this particular invocation here.
- const typename distribution_type::param_type p(0, __max_size);
- size_type random = generator(p);
- if (random < distribution.min() || random > distribution.max())
- std::__throw_out_of_range_fmt(__N("setup_base::generate\n"
- "random number generated is: %zu "
- "out of range [%zu, %zu]\n"),
- (size_t)random,
- (size_t)distribution.min(),
- (size_t)distribution.max());
- return random;
+ // Make the engine and distribution static...
+ static engine_type engine = get_engine();
+ static distribution_type distribution;
+ return distribution(engine, param_type{0, __max_size});
}
// Given an instantiating type, return a unique value.
@@ -309,10 +318,13 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // Container::erase(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
// Makes it easier to think of this as __container.erase(i)
(__container.*_F_erase_point)(i);
@@ -337,12 +349,15 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // forward_list::erase_after(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.before_begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
- // Makes it easier to think of this as __container.erase(i)
+ // Makes it easier to think of this as __container.erase_after(i)
(__container.*_F_erase_point)(i);
}
catch(const __gnu_cxx::forced_error&)
@@ -405,14 +420,19 @@ namespace __gnu_test
{
const size_type sz = std::distance(__container.begin(),
__container.end());
- size_type s1 = generate(sz);
- size_type s2 = generate(sz);
+ // forward_list::erase_after(pos, last) requires a pos != last
+ if (sz == 0)
+ return; // Caller doesn't check for this, not a logic error.
+
+ size_type s1 = generate(sz - 1);
+ size_type s2 = generate(sz - 1);
auto i1 = __container.before_begin();
auto i2 = __container.before_begin();
std::advance(i1, std::min(s1, s2));
- std::advance(i2, std::max(s1, s2));
+ std::advance(i2, std::max(s1, s2) + 1);
- // Makes it easier to think of this as __container.erase(i1, i2).
+ // Makes it easier to think of this as
+ // __container.erase_after(i1, i2).
(__container.*_F_erase_range)(i1, i2);
}
catch(const __gnu_cxx::forced_error&)
@@ -1454,16 +1474,25 @@ namespace __gnu_test
// constructor or assignment operator of value_type throws.
if (!traits<container_type>::has_throwing_erase::value)
{
- typename base_type::erase_point erasep;
- erasep(container);
+ if (!container.empty())
+ {
+ typename base_type::erase_point erasep;
+ erasep(container);
+ }
typename base_type::erase_range eraser;
eraser(container);
}
- typename base_type::pop_front popf;
- popf(container);
- typename base_type::pop_back popb;
- popb(container);
+ if (!container.empty())
+ {
+ typename base_type::pop_front popf;
+ popf(container);
+ }
+ if (!container.empty())
+ {
+ typename base_type::pop_back popb;
+ popb(container);
+ }
typename base_type::iterator_ops iops;
iops(container);
diff --git a/libstdc++-v3/testsuite/util/testsuite_containergen.h b/libstdc++-v3/testsuite/util/testsuite_containergen.h
index a2156733ec6..c468c7f4415 100644
--- a/libstdc++-v3/testsuite/util/testsuite_containergen.h
+++ b/libstdc++-v3/testsuite/util/testsuite_containergen.h
@@ -20,6 +20,8 @@
#include <testsuite_container_traits.h>
#include <random>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
namespace __gnu_test
{
@@ -63,6 +65,18 @@ namespace __gnu_test
{
std::mt19937_64 random_gen;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ int s = std::atoi(v);
+ if (s == 0)
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
+ std::fflush(stdout);
+ random_gen.seed((unsigned)s);
+ }
+
#ifdef SIMULATOR_TEST
int loops = 10;
#else
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-25 15:29 ` Jonathan Wakely
2021-03-25 17:45 ` François Dumont
@ 2021-03-29 20:25 ` François Dumont
2021-04-01 13:55 ` Jonathan Wakely
1 sibling, 1 reply; 12+ messages in thread
From: François Dumont @ 2021-03-29 20:25 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
On 25/03/21 4:29 pm, Jonathan Wakely wrote:
> Oh, it's the same generate(sz) bug as you already found. But I've
> found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
>
> I think we should also add a check for non-empty containers to those
> test functions, and ensure we don't try to erase from empty
> containers (see attached).
>
>
I had a look at this patch and on my side I was more thinking about
avoiding empty containers in the first place.
What do you think ?
François
[-- Attachment #2: exception_safety.patch --]
[-- Type: text/x-patch, Size: 1239 bytes --]
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 54449d2f7bb..6940f2f765d 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -55,14 +55,14 @@ namespace __gnu_test
// Return randomly generated integer on range [0, __max_size].
static size_type
- generate(size_type __max_size)
+ generate(size_type __max_size, size_type __min_size = 0)
{
using param_type = typename distribution_type::param_type;
// Make the engine and distribution static...
static engine_type engine = get_engine();
static distribution_type distribution;
- return distribution(engine, param_type{0, __max_size});
+ return distribution(engine, param_type{__min_size, __max_size});
}
// Given an instantiating type, return a unique value.
@@ -198,7 +198,8 @@ namespace __gnu_test
// Size test container.
const size_type max_elements = 100;
- size_type n = generate(max_elements);
+ const size_type min_elements = 10;
+ size_type n = generate(max_elements, min_elements);
// Construct new container.
make_container_n<container_type> made(n);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Complete __gnu_debug::basic_string
2021-03-29 20:25 ` François Dumont
@ 2021-04-01 13:55 ` Jonathan Wakely
0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Wakely @ 2021-04-01 13:55 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 29/03/21 22:25 +0200, François Dumont wrote:
>On 25/03/21 4:29 pm, Jonathan Wakely wrote:
>>Oh, it's the same generate(sz) bug as you already found. But I've
>>found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
>>
>>I think we should also add a check for non-empty containers to those
>>test functions, and ensure we don't try to erase from empty
>>containers (see attached).
>>
>>
>I had a look at this patch and on my side I was more thinking about
>avoiding empty containers in the first place.
>
>What do you think ?
I did consider this and decided against it for the reasons below.
>François
>
>diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
>index 54449d2f7bb..6940f2f765d 100644
>--- a/libstdc++-v3/testsuite/util/exception/safety.h
>+++ b/libstdc++-v3/testsuite/util/exception/safety.h
>@@ -55,14 +55,14 @@ namespace __gnu_test
>
> // Return randomly generated integer on range [0, __max_size].
> static size_type
>- generate(size_type __max_size)
>+ generate(size_type __max_size, size_type __min_size = 0)
> {
> using param_type = typename distribution_type::param_type;
>
> // Make the engine and distribution static...
> static engine_type engine = get_engine();
> static distribution_type distribution;
>- return distribution(engine, param_type{0, __max_size});
>+ return distribution(engine, param_type{__min_size, __max_size});
> }
>
> // Given an instantiating type, return a unique value.
>@@ -198,7 +198,8 @@ namespace __gnu_test
>
> // Size test container.
> const size_type max_elements = 100;
>- size_type n = generate(max_elements);
>+ const size_type min_elements = 10;
>+ size_type n = generate(max_elements, min_elements);
Why 10 though? Why not 1? Otherwise we never test the case where you
erase a single element from a container that only has a single
element.
And it would also mean we never test inserting into an empty
container, because it always starts with at least 10 elements.
So I decided that it was better to keep /most/ of the tests unchanged,
so they sometimes (depending on the RNG, which can now be made less
predictable by using a non-default seed) test with empty containers.
Only the operations that really can't work for empty containers need
to care about them. For other operations, we should keep testing the
empty case.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-01 13:57 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 20:55 [PATCH] Complete __gnu_debug::basic_string François Dumont
2021-03-19 19:41 ` Jonathan Wakely
2021-03-20 21:32 ` François Dumont
2021-03-23 15:42 ` Jonathan Wakely
2021-03-24 21:48 ` François Dumont
2021-03-25 13:05 ` Jonathan Wakely
2021-03-25 14:48 ` Jonathan Wakely
2021-03-25 15:29 ` Jonathan Wakely
2021-03-25 17:45 ` François Dumont
2021-03-25 18:22 ` Jonathan Wakely
2021-03-29 20:25 ` François Dumont
2021-04-01 13:55 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).