* [PATCH] Implement LWG 2686, hash<error_condition>
@ 2017-03-12 12:16 Daniel Krügler
2017-03-21 21:26 ` Daniel Krügler
2017-03-23 17:50 ` Jonathan Wakely
0 siblings, 2 replies; 12+ messages in thread
From: Daniel Krügler @ 2017-03-12 12:16 UTC (permalink / raw)
To: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 478 bytes --]
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.
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>.
- Daniel
[-- Attachment #2: lwg2686.patch --]
[-- Type: application/octet-stream, Size: 2334 bytes --]
Index: include/std/system_error
===================================================================
--- include/std/system_error (revision 246076)
+++ include/std/system_error (working copy)
@@ -268,6 +268,8 @@
// DR 804.
private:
+ friend class hash<error_condition>;
+
int _M_value;
const error_category* _M_cat;
};
@@ -372,8 +374,6 @@
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
-#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
-
#include <bits/functional_hash.h>
namespace std _GLIBCXX_VISIBILITY(default)
@@ -380,6 +380,7 @@
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
+#ifndef _GLIBCXX_COMPATIBILITY_CXX0X
// DR 1182.
/// std::hash specialization for error_code.
template<>
@@ -393,12 +394,25 @@
return std::_Hash_impl::__hash_combine(__e._M_cat, __tmp);
}
};
+#endif // _GLIBCXX_COMPATIBILITY_CXX0X
+ // 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._M_value);
+ return std::_Hash_impl::__hash_combine(__e._M_cat, __tmp);
+ }
+ };
+
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
-#endif // _GLIBCXX_COMPATIBILITY_CXX0X
-
#endif // C++11
#endif // _GLIBCXX_SYSTEM_ERROR
Index: testsuite/20_util/hash/operators/size_t.cc
===================================================================
--- testsuite/20_util/hash/operators/size_t.cc (revision 246076)
+++ testsuite/20_util/hash/operators/size_t.cc (working copy)
@@ -43,6 +43,7 @@
void test01()
{
do_test<std::error_code>();
+ do_test<std::error_condition>();
}
int main()
Index: testsuite/20_util/hash/requirements/explicit_instantiation.cc
===================================================================
--- testsuite/20_util/hash/requirements/explicit_instantiation.cc (revision 246076)
+++ testsuite/20_util/hash/requirements/explicit_instantiation.cc (working copy)
@@ -40,6 +40,7 @@
template class std::hash<void*>;
template class std::hash<std::string>;
template class std::hash<std::error_code>;
+template class std::hash<std::error_condition>;
#ifdef _GLIBCXX_USE_WCHAR_T
template class std::hash<wchar_t>;
[-- Attachment #3: ChangeLog_lwg2686.patch --]
[-- Type: application/octet-stream, Size: 785 bytes --]
Index: ChangeLog
===================================================================
--- ChangeLog (revision 246076)
+++ ChangeLog (working copy)
@@ -1,3 +1,13 @@
+2017-03-12 Daniel Kruegler <daniel.kruegler@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.
+ * 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>.
+
2017-03-12 Ville Voutilainen <ville.voutilainen@gmail.com>
Implement LWG 2934, optional<const T> doesn't compare with T.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2017-03-12 12:16 [PATCH] Implement LWG 2686, hash<error_condition> Daniel Krügler
@ 2017-03-21 21:26 ` Daniel Krügler
2017-03-23 17:50 ` Jonathan Wakely
1 sibling, 0 replies; 12+ messages in thread
From: Daniel Krügler @ 2017-03-21 21:26 UTC (permalink / raw)
To: libstdc++, gcc-patches List
2017-03-12 13:16 GMT+01:00 Daniel Krügler <daniel.kruegler@gmail.com>:
> 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.
>
> 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>.
>
> - Daniel
I would just point out that I'm on vacations from Friday on for two
weeks, so if any changes to this patch are requested, I will work on
them after my return.
Thanks,
- Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2017-03-12 12:16 [PATCH] Implement LWG 2686, hash<error_condition> Daniel Krügler
2017-03-21 21:26 ` Daniel Krügler
@ 2017-03-23 17:50 ` Jonathan Wakely
[not found] ` <20190503224255.GI2599@redhat.com>
1 sibling, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2017-03-23 17:50 UTC (permalink / raw)
To: Daniel Krügler; +Cc: libstdc++, gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]
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.
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.
Here's what I've tested and will be committing.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 3178 bytes --]
commit 90ca0fd91f5c65af370beb20af06bdca257aaf63
Author: Jonathan Wakely <jwakely@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.kruegler@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);
+ }
+ };
+#endif
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace
-#endif // _GLIBCXX_COMPATIBILITY_CXX0X
-
#endif // C++11
#endif // _GLIBCXX_SYSTEM_ERROR
diff --git a/libstdc++-v3/testsuite/20_util/hash/operators/size_t.cc b/libstdc++-v3/testsuite/20_util/hash/operators/size_t.cc
index ad02843..cc191d6 100644
--- a/libstdc++-v3/testsuite/20_util/hash/operators/size_t.cc
+++ b/libstdc++-v3/testsuite/20_util/hash/operators/size_t.cc
@@ -43,6 +43,9 @@ template<typename T>
void test01()
{
do_test<std::error_code>();
+#if __cplusplus > 201402L
+ do_test<std::error_condition>();
+#endif
}
int main()
diff --git a/libstdc++-v3/testsuite/20_util/hash/requirements/explicit_instantiation.cc b/libstdc++-v3/testsuite/20_util/hash/requirements/explicit_instantiation.cc
index e9e5ea1..d01829a 100644
--- a/libstdc++-v3/testsuite/20_util/hash/requirements/explicit_instantiation.cc
+++ b/libstdc++-v3/testsuite/20_util/hash/requirements/explicit_instantiation.cc
@@ -40,6 +40,9 @@ template class std::hash<long double>;
template class std::hash<void*>;
template class std::hash<std::string>;
template class std::hash<std::error_code>;
+#if __cplusplus > 201402L
+template class std::hash<std::error_condition>;
+#endif
#ifdef _GLIBCXX_USE_WCHAR_T
template class std::hash<wchar_t>;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
[not found] ` <20190503224255.GI2599@redhat.com>
@ 2019-05-04 14:36 ` Jonathan Wakely
2019-05-07 9:06 ` Christophe Lyon
2019-05-07 15:27 ` Jonathan Wakely
0 siblings, 2 replies; 12+ messages in thread
From: Jonathan Wakely @ 2019-05-04 14:36 UTC (permalink / raw)
To: Daniel Krügler; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3956 bytes --]
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 <jwakely@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.kruegler@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.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4777 bytes --]
commit 43b5da521c2857f60eaaad90bbaf149ee6704797
Author: Jonathan Wakely <jwakely@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();
+}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-04 14:36 ` Jonathan Wakely
@ 2019-05-07 9:06 ` Christophe Lyon
2019-05-07 9:37 ` Jonathan Wakely
2019-05-07 15:27 ` Jonathan Wakely
1 sibling, 1 reply; 12+ messages in thread
From: Christophe Lyon @ 2019-05-07 9:06 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Daniel Krügler, libstdc++, gcc Patches
On Sat, 4 May 2019 at 16:36, Jonathan Wakely <jwakely@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 <jwakely@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.kruegler@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 ?
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-07 9:06 ` Christophe Lyon
@ 2019-05-07 9:37 ` Jonathan Wakely
2019-05-07 10:07 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2019-05-07 9:37 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Daniel Krügler, libstdc++, gcc Patches
On 07/05/19 11:05 +0200, Christophe Lyon wrote:
>On Sat, 4 May 2019 at 16:36, Jonathan Wakely <jwakely@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 <jwakely@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.kruegler@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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-07 9:37 ` Jonathan Wakely
@ 2019-05-07 10:07 ` Jonathan Wakely
2019-05-07 12:22 ` Christophe Lyon
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2019-05-07 10:07 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Daniel Krügler, libstdc++, gcc Patches
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 <jwakely@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 <jwakely@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.kruegler@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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-07 10:07 ` Jonathan Wakely
@ 2019-05-07 12:22 ` Christophe Lyon
2019-05-09 14:43 ` Szabolcs Nagy
0 siblings, 1 reply; 12+ messages in thread
From: Christophe Lyon @ 2019-05-07 12:22 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Daniel Krügler, libstdc++, gcc Patches
On Tue, 7 May 2019 at 12:07, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> 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 <jwakely@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 <jwakely@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.kruegler@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.
>
>
Yes, and there full errors are:
/home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
In function `std::filesystem::current_path(std::filesystem::__cxx11::path
const&, std::error_code&)':
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:806:
undefined reference to `chdir'
/home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
In function `(anonymous
namespace)::create_dir(std::filesystem::__cxx11::path const&,
std::filesystem::perms, std::error_code&)':
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:583:
undefined reference to `mkdir'
/home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
In function `std::filesystem::permissions(std::filesystem::__cxx11::path
const&, std::filesystem::perms, std::filesystem::perm_options,
std::error_code&)':
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:1134:
undefined reference to `chmod'
/home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
In function `std::filesystem::do_copy_file(char const*, char const*,
std::filesystem::copy_options_existing_file, stat*, stat*,
std::error_code&)':
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/../filesystem/ops-common.h:439:
undefined reference to `chmod'
/home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
In function `std::filesystem::current_path[abi:cxx11](std::error_code&)':
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:750:
undefined reference to `pathconf'
/home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:769:
undefined reference to `getcwd'
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-04 14:36 ` Jonathan Wakely
2019-05-07 9:06 ` Christophe Lyon
@ 2019-05-07 15:27 ` Jonathan Wakely
1 sibling, 0 replies; 12+ messages in thread
From: Jonathan Wakely @ 2019-05-07 15:27 UTC (permalink / raw)
To: Daniel Krügler; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]
On 04/05/19 15:36 +0100, Jonathan Wakely 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 <jwakely@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.kruegler@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>.
I'm adding a similar test for hash<error_code> too.
Tested powerpc64le-linux, committing to trunk shortly.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2143 bytes --]
commit 4034dddf0dbfc20ff9069602a419a95b09de20f6
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue May 7 11:09:00 2019 +0100
Add test for std::hash<std::error_code>
Copied from 19_diagnostics/error_condition/hash.cc added recently.
* testsuite/19_diagnostics/error_code/hash.cc: New test.
diff --git a/libstdc++-v3/testsuite/19_diagnostics/error_code/hash.cc b/libstdc++-v3/testsuite/19_diagnostics/error_code/hash.cc
new file mode 100644
index 00000000000..2014e676878
--- /dev/null
+++ b/libstdc++-v3/testsuite/19_diagnostics/error_code/hash.cc
@@ -0,0 +1,50 @@
+// 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-do run { target c++11 } }
+
+#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_code> h;
+ error_cat kitty("kitty"), moggy("moggy");
+ std::error_code cond1(99, kitty);
+ VERIFY( h(cond1) == h(cond1) );
+ std::error_code cond2(99, kitty);
+ VERIFY( h(cond1) == h(cond2) );
+ std::error_code cond3(88, kitty);
+ VERIFY( h(cond1) != h(cond3) );
+ std::error_code cond4(99, moggy);
+ VERIFY( h(cond1) != h(cond4) );
+}
+
+int
+main()
+{
+ test01();
+}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-07 12:22 ` Christophe Lyon
@ 2019-05-09 14:43 ` Szabolcs Nagy
2019-05-09 15:17 ` Jonathan Wakely
0 siblings, 1 reply; 12+ messages in thread
From: Szabolcs Nagy @ 2019-05-09 14:43 UTC (permalink / raw)
To: Christophe Lyon, Jonathan Wakely
Cc: nd, Daniel Krügler, libstdc++, gcc Patches
On 07/05/2019 13:21, Christophe Lyon wrote:
> On Tue, 7 May 2019 at 12:07, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> 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 <jwakely@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 <jwakely@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.kruegler@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 looked at this and ld -M reports
/B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o)
hash.o (std::_V2::error_category::default_error_condition(int) const)
/B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o)
/B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o) (void std::__cxx11::basic_string<char, std::char_traits<char>,
std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag))
...
i.e. hash.o pulls system_error.o in because of
std::_V2::error_category::default_error_condition(int) const
and system_error.o pulls fs_ops.o in because of
std::__cxx11::basic_string...
symbol reference.
it seems fs_ops.o is the first object in libstdc++.a
that provides a (weak) definition for
_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12_M_constructIPKcEEvT_S8_St20forward_iterator_tag
>>
>> 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.
>>
>>
>
> Yes, and there full errors are:
> /home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
> In function `std::filesystem::current_path(std::filesystem::__cxx11::path
> const&, std::error_code&)':
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:806:
> undefined reference to `chdir'
> /home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
> In function `(anonymous
> namespace)::create_dir(std::filesystem::__cxx11::path const&,
> std::filesystem::perms, std::error_code&)':
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:583:
> undefined reference to `mkdir'
> /home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
> In function `std::filesystem::permissions(std::filesystem::__cxx11::path
> const&, std::filesystem::perms, std::filesystem::perm_options,
> std::error_code&)':
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:1134:
> undefined reference to `chmod'
> /home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
> In function `std::filesystem::do_copy_file(char const*, char const*,
> std::filesystem::copy_options_existing_file, stat*, stat*,
> std::error_code&)':
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/../filesystem/ops-common.h:439:
> undefined reference to `chmod'
> /home/christophe.lyon/src/GCC/builds/gcc-fsf-trunk/obj-arm-none-eabi/gcc3/arm-none-eabi/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o):
> In function `std::filesystem::current_path[abi:cxx11](std::error_code&)':
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:750:
> undefined reference to `pathconf'
> /home/christophe.lyon/src/GCC/sources/gcc-fsf/trunk/libstdc++-v3/src/c++17/fs_ops.cc:769:
> undefined reference to `getcwd'
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-09 14:43 ` Szabolcs Nagy
@ 2019-05-09 15:17 ` Jonathan Wakely
2019-05-29 11:09 ` Szabolcs Nagy
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Wakely @ 2019-05-09 15:17 UTC (permalink / raw)
To: Szabolcs Nagy
Cc: Christophe Lyon, Jonathan Wakely, nd, Daniel Krügler,
libstdc++,
gcc Patches
On Thu, 9 May 2019 at 15:43, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
>
> On 07/05/2019 13:21, Christophe Lyon wrote:
> > On Tue, 7 May 2019 at 12:07, Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> 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 <jwakely@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 <jwakely@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.kruegler@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 looked at this and ld -M reports
>
> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o)
> hash.o (std::_V2::error_category::default_error_condition(int) const)
> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o)
> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o) (void std::__cxx11::basic_string<char, std::char_traits<char>,
> std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag))
> ...
>
> i.e. hash.o pulls system_error.o in because of
>
> std::_V2::error_category::default_error_condition(int) const
>
> and system_error.o pulls fs_ops.o in because of
>
> std::__cxx11::basic_string...
>
> symbol reference.
>
> it seems fs_ops.o is the first object in libstdc++.a
> that provides a (weak) definition for
>
> _ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12_M_constructIPKcEEvT_S8_St20forward_iterator_tag
Ah, so maybe we need an explicit instantiation elsewhere.
Or completely disable all the stuff using chdir, mkdir etc for these
newlib targets, which is probably a good idea anyway.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Implement LWG 2686, hash<error_condition>
2019-05-09 15:17 ` Jonathan Wakely
@ 2019-05-29 11:09 ` Szabolcs Nagy
0 siblings, 0 replies; 12+ messages in thread
From: Szabolcs Nagy @ 2019-05-29 11:09 UTC (permalink / raw)
To: Jonathan Wakely
Cc: nd, Christophe Lyon, Jonathan Wakely, Daniel Krügler,
libstdc++,
gcc Patches
On 09/05/2019 16:16, Jonathan Wakely wrote:
> On Thu, 9 May 2019 at 15:43, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
>> On 07/05/2019 13:21, Christophe Lyon wrote:
>>> On Tue, 7 May 2019 at 12:07, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>> On 07/05/19 10:37 +0100, Jonathan Wakely wrote:
>>>>> On 07/05/19 11:05 +0200, Christophe Lyon wrote:
>>>>>> 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 looked at this and ld -M reports
>>
>> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o)
>> hash.o (std::_V2::error_category::default_error_condition(int) const)
>> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(fs_ops.o)
>> /B/aarch64-none-elf/libstdc++-v3/src/.libs/libstdc++.a(system_error.o) (void std::__cxx11::basic_string<char, std::char_traits<char>,
>> std::allocator<char> >::_M_construct<char const*>(char const*, char const*, std::forward_iterator_tag))
>> ...
>>
>> i.e. hash.o pulls system_error.o in because of
>>
>> std::_V2::error_category::default_error_condition(int) const
>>
>> and system_error.o pulls fs_ops.o in because of
>>
>> std::__cxx11::basic_string...
>>
>> symbol reference.
>>
>> it seems fs_ops.o is the first object in libstdc++.a
>> that provides a (weak) definition for
>>
>> _ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12_M_constructIPKcEEvT_S8_St20forward_iterator_tag
>
> Ah, so maybe we need an explicit instantiation elsewhere.
> Or completely disable all the stuff using chdir, mkdir etc for these
> newlib targets, which is probably a good idea anyway.
disabling fs things for baremetal makes sense.
but i would not expect system_error.o to depend
on fs_ops.o even on non-baremetal targets.
so whatever causes the dependency should be fixed
as well i think.
in this case the base_string_whatever should have
a definition either in system_error.o or in a .o
with minimal deps that is guaranteed to come before
fs_ops.o in libstdc++.a
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-05-29 10:55 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-12 12:16 [PATCH] Implement LWG 2686, hash<error_condition> Daniel Krügler
2017-03-21 21:26 ` Daniel Krügler
2017-03-23 17:50 ` Jonathan Wakely
[not found] ` <20190503224255.GI2599@redhat.com>
2019-05-04 14:36 ` Jonathan Wakely
2019-05-07 9:06 ` Christophe Lyon
2019-05-07 9:37 ` Jonathan Wakely
2019-05-07 10:07 ` Jonathan Wakely
2019-05-07 12:22 ` Christophe Lyon
2019-05-09 14:43 ` Szabolcs Nagy
2019-05-09 15:17 ` Jonathan Wakely
2019-05-29 11:09 ` Szabolcs Nagy
2019-05-07 15:27 ` 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).