[Bug c++/115420] New: Default constructor of unordered_map deleted

[Bug c++/115420] New: Default constructor of unordered_map deleted

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

            Bug ID: 115420
           Summary: Default constructor of unordered_map deleted
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: krebbel at gcc dot gnu.org
  Target Milestone: ---

t.cc:

#include <unordered_map>
#include <condition_variable>

class Foo {
  std::unordered_map<std::string, uint64_t> bar;
  Foo() : bar() {}
};

g++ -std=c++11 -c t.cc

fails with:

t.cc: In constructor ‘Foo::Foo()’:                                              
t.cc:6:11: error: use of deleted function ‘std::unordered_map<_Key, _Tp, _Hash,
_Pred, _Alloc>::unordered_map() [with _Key = std::__cxx11::basic_string<char>;
_Tp = long unsigned int; _Hash = std::hash<std::__cxx11::basic_string<char> >;
_Pred = std::equal_to<std::__cxx11::basic_string<char> >; _Alloc =
std::allocator<std::pair<const std::__cxx11::basic_string<char>, long unsigned
int> >]’
    6 |   Foo() : bar() {}
      |           ^~~~~
In file included from
/home2/andreas/bisect/gcc-02dab998665-install/include/c++/13.0.0/unordered_map:41,
                 from t.cc:1:
/home2/andreas/bisect/gcc-02dab998665-install/include/c++/13.0.0/bits/unordered_map.h:146:7:
note: ‘std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map()
[with _Key = std::__cxx11::basic_string<char>; _Tp = long unsigned int; _Hash =
std::hash<std::__cxx11::basic_string<char> >; _Pred =
std::equal_to<std::__cxx11::basic_string<char> >; _Alloc =
std::allocator<std::pair<const std::__cxx11::basic_string<char>, long unsigned
int> >]’ is implicitly deleted because the default definition would be
ill-formed:
  146 |       unordered_map() = default;
      |       ^~~~~~~~~~~~~

starting with:

commit 227351345d0caa596eff8325144f15b15f704c08
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 12 13:03:01 2023 +0000

    libstdc++: Do not include <system_error> in concurrency headers

    The <condition_variable>, <mutex>, and <shared_mutex> headers use
    std::errc constants, but don't use std::system_error itself. They only
    use the __throw_system_error(int) function, which is defined in
    <bits/functexcept.h>.

    By including the header for the errc constants instead of the whole of
    <system_error> we avoid depending on the whole std::string definition.

    libstdc++-v3/ChangeLog:

            * include/bits/std_mutex.h: Remove <system_error> include.
            * include/std/condition_variable: Add <bits/error_constants.h>
            include.
            * include/std/mutex: Likewise.
            * include/std/shared_mutex: Likewise.

The testcase works fine with gcc 12 and clang. 

There are similar BZs on that topic but these do not seem to fit exactly, as I
understand it (e.g. 107022)

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
You need to do:
#include <string>


This is documented already at https://gcc.gnu.org/gcc-13/porting_to.html too.

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Andreas Krebbel <krebbel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED
         Resolution|INVALID                     |FIXED

--- Comment #2 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Oh right, didn't make the connection from the error message to the header file
rearrangements. Thanks for the pointer!

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[krebbel at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Andreas Krebbel <krebbel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |INVALID

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[xry111 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xry111 at gcc dot gnu.org
           Keywords|                            |diagnostic

--- Comment #3 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
Arguably we should try to make a better diagnostic...

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2024-06-11
             Status|CLOSED                      |NEW
     Ever confirmed|0                           |1
         Resolution|INVALID                     |---

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
If we add this to _Hashtable:

      template<typename>
        struct _Check_hash_requirements : true_type
        {
          static_assert(is_copy_constructible<_Hash>::value,
                        "custom hash function must be copy constructible");
          static_assert(is_destructible<_Hash>::value,
                        "custom hash function must be destructible");
        };

      template<typename _Tp>
        struct _Check_hash_requirements<hash<_Tp>> : true_type
        {
          static_assert(is_default_constructible<hash<_Tp>>::value,
                        "the std::hash<key_type> specialization is disabled");
        };
      static_assert(_Check_hash_requirements<_Hash>::value, "");


Then the first error is much clearer where the problem lies:

In file included from
/home/jwakely/gcc/15/include/c++/15.0.0/bits/unordered_map.h:33,
                 from /home/jwakely/gcc/15/include/c++/15.0.0/unordered_map:41,
                 from un.cc:1:
/home/jwakely/gcc/15/include/c++/15.0.0/bits/hashtable.h: In instantiation of
'struct std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const
std::__cxx11::basic_string<char>, long unsigned int>,
std::allocator<std::pair<const std::__cxx11::basic_string<char>, long unsigned
int> >, std::__detail::_Select1st,
std::equal_to<std::__cxx11::basic_string<char> >,
std::hash<std::__cxx11::basic_string<char> >,
std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash,
std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true,
false, true>
>::_Check_hash_requirements<std::hash<std::__cxx11::basic_string<char> > >':
/home/jwakely/gcc/15/include/c++/15.0.0/bits/hashtable.h:221:54:   required
from 'class std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const
std::__cxx11::basic_string<char>, long unsigned int>,
std::allocator<std::pair<const std::__cxx11::basic_string<char>, long unsigned
int> >, std::__detail::_Select1st,
std::equal_to<std::__cxx11::basic_string<char> >,
std::hash<std::__cxx11::basic_string<char> >,
std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash,
std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true,
false, true> >'
  221 |       static_assert(_Check_hash_requirements<_Hash>::value, "");
      |                                                      ^~~~~
/home/jwakely/gcc/15/include/c++/15.0.0/bits/unordered_map.h:112:18:   required
from 'class std::unordered_map<std::__cxx11::basic_string<char>, long unsigned
int>'
  112 |       _Hashtable _M_h;
      |                  ^~~~
un.cc:5:45:   required from here
    5 |   std::unordered_map<std::string, uint64_t> bar;
      |                                             ^~~
/home/jwakely/gcc/15/include/c++/15.0.0/bits/hashtable.h:218:62: error: static
assertion failed: the std::hash<key_type> specialization is disabled
  218 |           static_assert(is_default_constructible<hash<_Tp>>::value,
      |                                                              ^~~~~

So let's reopen this as a diagnostic RFE for the library.

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
And if _Check_hash_requirements is defined at namespace scope not inside
_Hashtable then the first location is much easier to read:

/home/jwakely/gcc/15/include/c++/15.0.0/bits/hashtable.h: In instantiation of
'struct
std::_Check_hash_requirements<std::hash<std::__cxx11::basic_string<char> > >':

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We could even do:

/home/jwakely/gcc/15/include/c++/15.0.0/bits/hashtable.h:78:58: error: static
assertion failed: std::hash specialization is disabled; maybe the correct
header has not been included

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

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

commit r15-1256-gcc38bdf093c44918edff819ae6c73d03c726b341
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jun 11 11:08:12 2024 +0100

    libstdc++: Improve diagnostics for invalid std::hash specializations
[PR115420]

    When using a key type without a valid std::hash specialization the
    unordered containers give confusing diagnostics about the default
    constructor being deleted. Add a static_assert that will fail for
    disabled std::hash specializations (and for a subset of custom hash
    functions).

    libstdc++-v3/ChangeLog:

            PR libstdc++/115420
            * include/bits/hashtable.h (_Hashtable): Add static_assert to
            check that hash function is copy constructible.
            * testsuite/23_containers/unordered_map/115420.cc: New test.

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=48101
   Target Milestone|---                         |15.0
             Status|NEW                         |RESOLVED

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk. I won't backport, at least until we know it doesn't cause any
regressions for odd use cases.

[Bug libstdc++/115420] Default constructor of unordered_map deleted

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115420

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Odd use case found!

Repeating my analysis from https://bugs.gentoo.org/934688 regarding a new
compilation error in antlr-cpp-4.11.1:

Here's a minimal reproducer that fails with GCC trunk, unless you use -DFIX

#include <unordered_map>

template <typename Key, typename Value,
#ifdef FIX
            typename Hash = std::hash<Key>,
            typename Equal = std::equal_to<Key>,
            typename Allocator = std::allocator<std::pair<const Key, Value>>>
#else
            typename Hash = typename std::unordered_map<Key, Value>::hasher,
            typename Equal = typename std::unordered_map<Key,
Value>::key_equal,
            typename Allocator = typename std::unordered_map<Key,
Value>::allocator_type>
#endif
  using FlatHashMap = std::unordered_map<Key, Value, Hash, Equal, Allocator>;

struct S { };
struct Hash { std::size_t operator==(const S&) const { return 0; } };
struct Eq { bool operator()(const S&, const S&) const { return true; } };

FlatHashMap<S, int, Hash, Eq> m;


The problem is that the alias template FlatHashMap instantiates
std::unordered_map<Key, Value> to obtain any of the Hash, Equal or Allocator
types that are not provided explicitly.

When used by PredictionContextMergeCache, the Hash and Equal types are provided
explicitly (because antlr-cpp has custom hasher and comparison types for the
map's key type). But the Allocator type is not provided, so it uses the default
template argument, which is:

std::unordered_map<Key, Value>::allocator_type

This triggers the instantiation of std::unordered_map<Key, Value> which uses
*its* default hasher, which is std::hash<Key>, which is invalid, which triggers
the static_assert. That instantiation isn't actually wanted, they never use it,
they're just trying to get some nested types out of it.  The commit
https://github.com/antlr/antlr4/commit/9d7741d3fb1e0befe1ca32502a42a2809741053c
(which is in antlr-cpp-4.12.0) avoids instantiating it just to get some types
that can be obtained far more directly than by instantiating an unusable
specialization of std::unordered_map.

Basically the same problem as they encountered upstream with a static_assert
that I added (and then I think reverted) in GCC 7:
https://github.com/antlr/antlr4/pull/3885