public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jonathan Wakely <redi@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org
Subject: [gcc r13-1037] libstdc++: Make std::hash<basic_string<>> allocator-agnostic (LWG 3705)
Date: Fri, 10 Jun 2022 14:09:43 +0000 (GMT)	[thread overview]
Message-ID: <20220610140943.E61C03889A65@sourceware.org> (raw)

https://gcc.gnu.org/g:b370ed0bf93ecf0ff51d29e7fc132c433b2aa1be

commit r13-1037-gb370ed0bf93ecf0ff51d29e7fc132c433b2aa1be
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jun 9 12:07:15 2022 +0100

    libstdc++: Make std::hash<basic_string<>> allocator-agnostic (LWG 3705)
    
    This new library issue was recently moved to Tentatively Ready by an LWG
    poll, so I'm making the change on trunk.
    
    As noted in PR libstc++/105907 the std::hash specializations for PMR
    strings were not treated as slow hashes by the unordered containers, so
    this change preserves that. The new specializations for custom
    allocators are also not treated as slow, for the same reason. For the
    versioned namespace (i.e. unstable ABI) we don't have to worry about
    that, so can enable hash code caching for all basic_string
    specializations.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/basic_string.h (__hash_str_base): New class
            template.
            (hash<basic_string<C, char_traits<C>, A>>): Define partial
            specialization for each of the standard character types.
            (hash<string>, hash<wstring>, hash<u8string>, hash<u16string>)
            (hash<u32string>): Remove explicit specializations.
            * include/std/string (__hash_string_base): Remove class
            template.
            (hash<pmr::string>, hash<pmr::wstring>, hash<pmr::u8string>)
            (hash<pmr::u16string>, hash<pmr::u32string>): Remove explicit
            specializations.
            * testsuite/21_strings/basic_string/hash/hash.cc: Test with
            custom allocators.
            * testsuite/21_strings/basic_string/hash/hash_char8_t.cc:
            Likewise.

Diff:
---
 libstdc++-v3/include/bits/basic_string.h           | 102 ++++++++++-----------
 libstdc++-v3/include/std/string                    |  33 -------
 .../testsuite/21_strings/basic_string/hash/hash.cc |  16 ++++
 .../21_strings/basic_string/hash/hash_char8_t.cc   |  12 +++
 4 files changed, 74 insertions(+), 89 deletions(-)

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 6041d05815b..f76ddf970c6 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -4226,85 +4226,75 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
-  // DR 1182.
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 3705. Hashability shouldn't depend on basic_string's allocator
 
-#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
-  /// std::hash specialization for string.
-  template<>
-    struct hash<string>
-    : public __hash_base<size_t, string>
+  template<typename _CharT, typename _Alloc,
+	   typename _StrT = basic_string<_CharT, char_traits<_CharT>, _Alloc>>
+    struct __str_hash_base
+    : public __hash_base<size_t, _StrT>
     {
       size_t
-      operator()(const string& __s) const noexcept
-      { return std::_Hash_impl::hash(__s.data(), __s.length()); }
+      operator()(const _StrT& __s) const noexcept
+      { return _Hash_impl::hash(__s.data(), __s.length() * sizeof(_CharT)); }
     };
 
-  template<>
-    struct __is_fast_hash<hash<string>> : std::false_type
+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
+  /// std::hash specialization for string.
+  template<typename _Alloc>
+    struct hash<basic_string<char, char_traits<char>, _Alloc>>
+    : public __str_hash_base<char, _Alloc>
     { };
 
   /// std::hash specialization for wstring.
-  template<>
-    struct hash<wstring>
-    : public __hash_base<size_t, wstring>
-    {
-      size_t
-      operator()(const wstring& __s) const noexcept
-      { return std::_Hash_impl::hash(__s.data(),
-                                     __s.length() * sizeof(wchar_t)); }
-    };
+  template<typename _Alloc>
+    struct hash<basic_string<wchar_t, char_traits<wchar_t>, _Alloc>>
+    : public __str_hash_base<wchar_t, _Alloc>
+    { };
 
-  template<>
-    struct __is_fast_hash<hash<wstring>> : std::false_type
+  template<typename _Alloc>
+    struct __is_fast_hash<hash<basic_string<wchar_t, char_traits<wchar_t>,
+					    _Alloc>>>
+    : std::false_type
     { };
 #endif /* _GLIBCXX_COMPATIBILITY_CXX0X */
 
 #ifdef _GLIBCXX_USE_CHAR8_T
   /// std::hash specialization for u8string.
-  template<>
-    struct hash<u8string>
-    : public __hash_base<size_t, u8string>
-    {
-      size_t
-      operator()(const u8string& __s) const noexcept
-      { return std::_Hash_impl::hash(__s.data(),
-                                     __s.length() * sizeof(char8_t)); }
-    };
-
-  template<>
-    struct __is_fast_hash<hash<u8string>> : std::false_type
+  template<typename _Alloc>
+    struct hash<basic_string<char8_t, char_traits<char8_t>, _Alloc>>
+    : public __str_hash_base<char8_t, _Alloc>
     { };
 #endif
 
   /// std::hash specialization for u16string.
-  template<>
-    struct hash<u16string>
-    : public __hash_base<size_t, u16string>
-    {
-      size_t
-      operator()(const u16string& __s) const noexcept
-      { return std::_Hash_impl::hash(__s.data(),
-                                     __s.length() * sizeof(char16_t)); }
-    };
-
-  template<>
-    struct __is_fast_hash<hash<u16string>> : std::false_type
+  template<typename _Alloc>
+    struct hash<basic_string<char16_t, char_traits<char16_t>, _Alloc>>
+    : public __str_hash_base<char16_t, _Alloc>
     { };
 
   /// std::hash specialization for u32string.
-  template<>
-    struct hash<u32string>
-    : public __hash_base<size_t, u32string>
-    {
-      size_t
-      operator()(const u32string& __s) const noexcept
-      { return std::_Hash_impl::hash(__s.data(),
-                                     __s.length() * sizeof(char32_t)); }
-    };
+  template<typename _Alloc>
+    struct hash<basic_string<char32_t, char_traits<char32_t>, _Alloc>>
+    : public __str_hash_base<char32_t, _Alloc>
+    { };
 
-  template<>
-    struct __is_fast_hash<hash<u32string>> : std::false_type
+#if ! _GLIBCXX_INLINE_VERSION
+  // PR libstdc++/105907 - __is_fast_hash affects unordered container ABI.
+  template<> struct __is_fast_hash<hash<string>> : std::false_type { };
+  template<> struct __is_fast_hash<hash<wstring>> : std::false_type { };
+  template<> struct __is_fast_hash<hash<u16string>> : std::false_type { };
+  template<> struct __is_fast_hash<hash<u32string>> : std::false_type { };
+#ifdef _GLIBCXX_USE_CHAR8_T
+  template<> struct __is_fast_hash<hash<u8string>> : std::false_type { };
+#endif
+#else
+  // For versioned namespace, assume every std::hash<basic_string<>> is slow.
+  template<typename _CharT, typename _Traits, typename _Alloc>
+    struct __is_fast_hash<hash<basic_string<_CharT, _Traits, _Alloc>>>
+    : std::false_type
     { };
+#endif
 
 #if __cplusplus >= 201402L
 
diff --git a/libstdc++-v3/include/std/string b/libstdc++-v3/include/std/string
index 4a0633067bd..37a4aaba9cd 100644
--- a/libstdc++-v3/include/std/string
+++ b/libstdc++-v3/include/std/string
@@ -69,39 +69,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     using u32string = basic_string<char32_t>;
     using wstring   = basic_string<wchar_t>;
   } // namespace pmr
-
-  template<typename _Str>
-    struct __hash_string_base
-    : public __hash_base<size_t, _Str>
-    {
-      size_t
-      operator()(const _Str& __s) const noexcept
-      { return hash<basic_string_view<typename _Str::value_type>>{}(__s); }
-    };
-
-  template<>
-    struct hash<pmr::string>
-    : public __hash_string_base<pmr::string>
-    { };
-#ifdef _GLIBCXX_USE_CHAR8_T
-  template<>
-    struct hash<pmr::u8string>
-    : public __hash_string_base<pmr::u8string>
-    { };
-#endif
-  template<>
-    struct hash<pmr::u16string>
-    : public __hash_string_base<pmr::u16string>
-    { };
-  template<>
-    struct hash<pmr::u32string>
-    : public __hash_string_base<pmr::u32string>
-    { };
-  template<>
-    struct hash<pmr::wstring>
-    : public __hash_string_base<pmr::wstring>
-    { };
-
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 #endif // C++17
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash.cc b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash.cc
index 4c16f0e6f40..7f3809926e5 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/hash/hash.cc
@@ -20,6 +20,7 @@
 #include <string>
 #include <memory_resource>
 #include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
 
 // C++17 24.3.5 [basic.string.hash]
 // If S is one of these string types, SV is the corresponding string view type,
@@ -54,9 +55,24 @@ test02()
 #endif
 }
 
+template<typename C>
+using String
+  = std::basic_string<C, std::char_traits<C>, __gnu_test::SimpleAllocator<C>>;
+
+void
+test03()
+{
+  // LWG 3705. Hashability shouldn't depend on basic_string's allocator
+  VERIFY( test(String<char>("a narrow string")) );
+  VERIFY( test(String<char16_t>(u"a utf-16 string")) );
+  VERIFY( test(String<char32_t>(U"a utf-32 string")) );
+  VERIFY( test(String<wchar_t>(L"a wide string")) );
+}
+
 int
 main()
 {
   test01();
   test02();
+  test03();
 }
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 217fe15c7fe..0ff98ad1caa 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
@@ -21,6 +21,7 @@
 #include <string>
 #include <memory_resource>
 #include <testsuite_hooks.h>
+#include <testsuite_allocator.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,
@@ -55,9 +56,20 @@ test02()
   VERIFY( hash<std::string>()(native) == hash<std::u8string>()(utf8) );
 }
 
+void
+test03()
+{
+  using Alloc = __gnu_test::SimpleAllocator<char8_t>;
+  using Stringu8 = std::basic_string<char8_t, std::char_traits<char8_t>, Alloc>;
+
+  // LWG 3705. Hashability shouldn't depend on basic_string's allocator
+  VERIFY( test(Stringu8(u8"a utf-8 string, with custom allocator")) );
+}
+
 int
 main()
 {
   test01();
   test02();
+  test03();
 }


                 reply	other threads:[~2022-06-10 14:09 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220610140943.E61C03889A65@sourceware.org \
    --to=redi@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    --cc=libstdc++-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).