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.
commit 43b5da521c2857f60eaaad90bbaf149ee6704797
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Sat May 4 14:50:20 2019 +0100
Fix std::hash<std::error_condition>
The hash value should be based on the identity (i.e. address) of the
error_category member, not its object representation (i.e. underlying
bytes).
* include/std/system_error (error_code): Remove friend declaration
for hash<error_code>.
(hash<error_code>::operator()): Use public member functions to access
value and category.
(hash<error_condition>::operator()): Use address of category, not
its object representation.
* src/c++11/compatibility-c++0x.cc (hash<error_code>::operator()):
Use public member functions to access value and category.
* testsuite/19_diagnostics/error_condition/hash.cc: New test.
diff --git a/libstdc++-v3/include/std/system_error b/libstdc++-v3/include/std/system_error
index b7891cbaa86..a60c96accb2 100644
--- a/libstdc++-v3/include/std/system_error
+++ b/libstdc++-v3/include/std/system_error
@@ -193,8 +193,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// DR 804.
private:
- friend class hash<error_code>;
-
int _M_value;
const error_category* _M_cat;
};
@@ -394,13 +392,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_t
operator()(const error_code& __e) const noexcept
{
- const size_t __tmp = std::_Hash_impl::hash(__e._M_value);
- return std::_Hash_impl::__hash_combine(__e._M_cat, __tmp);
+ const size_t __tmp = std::_Hash_impl::hash(__e.value());
+ return std::_Hash_impl::__hash_combine(&__e.category(), __tmp);
}
};
#endif // _GLIBCXX_COMPATIBILITY_CXX0X
-#if __cplusplus > 201402L
+#if __cplusplus >= 201703L
// DR 2686.
/// std::hash specialization for error_condition.
template<>
@@ -411,7 +409,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
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);
+ return std::_Hash_impl::__hash_combine(&__e.category(), __tmp);
}
};
#endif
diff --git a/libstdc++-v3/src/c++11/compatibility-c++0x.cc b/libstdc++-v3/src/c++11/compatibility-c++0x.cc
index d57fdd23bcf..25ebe43e093 100644
--- a/libstdc++-v3/src/c++11/compatibility-c++0x.cc
+++ b/libstdc++-v3/src/c++11/compatibility-c++0x.cc
@@ -117,8 +117,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
size_t
hash<error_code>::operator()(error_code __e) const
{
- const size_t __tmp = std::_Hash_impl::hash(__e._M_value);
- return std::_Hash_impl::__hash_combine(__e._M_cat, __tmp);
+ const size_t __tmp = std::_Hash_impl::hash(__e.value());
+ return std::_Hash_impl::__hash_combine(&__e.category(), __tmp);
}
// gcc-4.7.0
diff --git a/libstdc++-v3/testsuite/19_diagnostics/error_condition/hash.cc b/libstdc++-v3/testsuite/19_diagnostics/error_condition/hash.cc
new file mode 100644
index 00000000000..3bc05611418
--- /dev/null
+++ b/libstdc++-v3/testsuite/19_diagnostics/error_condition/hash.cc
@@ -0,0 +1,51 @@
+// Copyright (C) 2019 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 <system_error>
+#include <testsuite_hooks.h>
+
+struct error_cat : std::error_category
+{
+ error_cat(std::string s) : _name(s) { }
+ std::string _name;
+ const char* name() const noexcept override { return _name.c_str(); }
+ std::string message(int) const override { return "error"; }
+};
+
+void
+test01()
+{
+ std::hash<std::error_condition> h;
+ error_cat kitty("kitty"), moggy("moggy");
+ std::error_condition cond1(99, kitty);
+ VERIFY( h(cond1) == h(cond1) );
+ std::error_condition cond2(99, kitty);
+ VERIFY( h(cond1) == h(cond2) );
+ std::error_condition cond3(88, kitty);
+ VERIFY( h(cond1) != h(cond3) );
+ std::error_condition cond4(99, moggy);
+ VERIFY( h(cond1) != h(cond4) );
+}
+
+int
+main()
+{
+ test01();
+}
