On 07/05/19 10:37 +0100, Jonathan Wakely wrote:
On 07/05/19 11:05 +0200, Christophe Lyon wrote:
On Sat, 4 May 2019 at 16:36, Jonathan Wakely <jwak...@redhat.com> wrote:

On 03/05/19 23:42 +0100, Jonathan Wakely wrote:
On 23/03/17 17:49 +0000, Jonathan Wakely wrote:
On 12/03/17 13:16 +0100, Daniel Krügler wrote:
The following is an *untested* patch suggestion, please verify.

Notes: My interpretation is that hash<error_condition> should be
defined outside of the _GLIBCXX_COMPATIBILITY_CXX0X block, please
double-check that course of action.

That's right.

I noticed that the preexisting hash<error_code> did directly refer to
the private members of error_code albeit those have public access
functions. For consistency I mimicked that existing style when
implementing hash<error_condition>.

I see no reason for that, so I've removed the friend declaration and
used the public member functions.

I'm going to do the same for hash<error_code> too. It can also use the
public members instead of being a friend.


Although this is a DR, I'm treating it as a new C++17 feature, so I've
adjusted the patch to only add the new specialization for C++17 mode.
We're too close to the GCC 7 release to be adding new things to the
default mode, even minor things like this. After GCC 7 is released we
can revisit it and decide if we want to enable it for all modes.

We never revisited that, and it's still only enabled for C++17 and up.
I guess that's OK, but we could enabled it for C++11 and 14 on trunk
if we want. Anybody care enough to argue for that?

Here's what I've tested and will be committing.



commit 90ca0fd91f5c65af370beb20af06bdca257aaf63
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Thu Mar 23 11:47:39 2017 +0000

  Implement LWG 2686, std::hash<error_condition>, for C++17
  2017-03-23  Daniel Kruegler  <daniel.krueg...@gmail.com>
     Implement LWG 2686, Why is std::hash specialized for error_code,
     but not error_condition?
     * include/std/system_error (hash<error_condition>): Define for C++17.
     * testsuite/20_util/hash/operators/size_t.cc (hash<error_condition>):
     Instantiate test for error_condition.
     * testsuite/20_util/hash/requirements/explicit_instantiation.cc
     (hash<error_condition>): Instantiate hash<error_condition>.

diff --git a/libstdc++-v3/include/std/system_error 
b/libstdc++-v3/include/std/system_error
index 6775a6e..ec7d25f 100644
--- a/libstdc++-v3/include/std/system_error
+++ b/libstdc++-v3/include/std/system_error
@@ -373,14 +373,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace

-#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
-
#include <bits/functional_hash.h>

namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION

+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
 // DR 1182.
 /// std::hash specialization for error_code.
 template<>
@@ -394,12 +393,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     return std::_Hash_impl::__hash_combine(__e._M_cat, __tmp);
     }
   };
+#endif // _GLIBCXX_COMPATIBILITY_CXX0X
+
+#if __cplusplus > 201402L
+  // DR 2686.
+  /// std::hash specialization for error_condition.
+  template<>
+    struct hash<error_condition>
+    : public __hash_base<size_t, error_condition>
+    {
+      size_t
+      operator()(const error_condition& __e) const noexcept
+      {
+     const size_t __tmp = std::_Hash_impl::hash(__e.value());
+     return std::_Hash_impl::__hash_combine(__e.category(), __tmp);

When I changed this from using __e._M_cat (as in Daniel's patch) to
__e.category() I introduced a bug, because the former is a pointer to
the error_category (and error_category objects are unique and so can
be identified by their address) and the latter is the object itself,
so we hash the bytes of an abstract base class instead of hashing the
pointer to it. Oops.

Patch coming up to fix that.

Here's the fix. Tested powerpc64le-linux, committed to trunk.

I'll backport this to 7, 8 and 9 as well.


Hi Jonathan,

Does the new test lack dg-require-filesystem-ts ?

It lacks it, because it doesn't use the filesystem library at all.

I'm seeing link failures on arm-eabi (using newlib):
Excess errors:
/libstdc++-v3/src/c++17/fs_ops.cc:806: undefined reference to `chdir'
/libstdc++-v3/src/c++17/fs_ops.cc:583: undefined reference to `mkdir'
/libstdc++-v3/src/c++17/fs_ops.cc:1134: undefined reference to `chmod'
/libstdc++-v3/src/c++17/../filesystem/ops-common.h:439: undefined
reference to `chmod'
/libstdc++-v3/src/c++17/fs_ops.cc:750: undefined reference to `pathconf'
/libstdc++-v3/src/c++17/fs_ops.cc:769: undefined reference to `getcwd'

Christophe

Is it definitely the new 19_diagnostics/error_condition/hash.cc test
that's giving this error?

I adjusted the pre-existing 27_io/filesystem/operations/absolute.cc
test in r270874, which seems a more likely culprit, but that already
has dg-require-filesystem-ts.


Reply via email to