Re: Fix std::pair std::is_copy_assignable behavior
Hi, "François Dumont" ha scritto: > Ok to commit ? No, it's definitely not Ok, we don't want to add std::is_copy_assignable specializations like this. Jon will send you more comments. Thanks, Paolo
Re: Fix std::pair std::is_copy_assignable behavior
On 04/17/2013 10:02 PM, Paolo Carlini wrote: Hi, On 4/17/13 8:43 PM, François Dumont wrote: On 04/17/2013 09:18 PM, Paolo Carlini wrote: On 4/17/13 8:10 PM, François Dumont wrote: Hi Here is an other proposal to fix std::is_copy_assignable>. Sorry, I'm still missing something very, very basic: which behavior is conforming, the current one or what we would get instead? If the former, is there a DR arguing for the latter? Paolo. The behavior I am targeting is std::is_copy_asignable> to be std::false_type for instance. I have added test for many other use cases. More generally I need that when std::is_copy_assignable is std::true_type then writing a = b, with a and b being T, does compile. Otherwise this patch just make std::pair match the Standard requirements like at 20.3.2.17. Do you want me to add a bug report in Bugzilla first ? I'm not talking about GCC's Bugzilla, I meant an ISO DR: if we don't have a DR and preferably a vague support of LWG people, I think we should not change the behavior of our std::is_copy_assignable only because it helps our implementation of other facilities. Paolo. Hi I check again the Standard and I really can't see any problem with the patch. As far as I understand it having std::is_copy_assignable being true is not a guaranty that the expression will compile but making std::pair is_copy_assignable friendly doesn't violate it neither. I also see that std::pair is already using std::enable_if to disable some constructors. I check the ChangeLog and it was not associated to a any ISO DR. I don't know why at that moment the same kind of modification hadn't been done for the assignment operators but the patch is not doing more than what has been done at that time. Ok to commit ? François
Re: Fix std::pair std::is_copy_assignable behavior
On 04/17/2013 10:02 PM, Paolo Carlini wrote: Hi, On 4/17/13 8:43 PM, François Dumont wrote: On 04/17/2013 09:18 PM, Paolo Carlini wrote: On 4/17/13 8:10 PM, François Dumont wrote: Hi Here is an other proposal to fix std::is_copy_assignable>. Sorry, I'm still missing something very, very basic: which behavior is conforming, the current one or what we would get instead? If the former, is there a DR arguing for the latter? Paolo. The behavior I am targeting is std::is_copy_asignable> to be std::false_type for instance. I have added test for many other use cases. More generally I need that when std::is_copy_assignable is std::true_type then writing a = b, with a and b being T, does compile. Otherwise this patch just make std::pair match the Standard requirements like at 20.3.2.17. Do you want me to add a bug report in Bugzilla first ? I'm not talking about GCC's Bugzilla, I meant an ISO DR: if we don't have a DR and preferably a vague support of LWG people, I think we should not change the behavior of our std::is_copy_assignable only because it helps our implementation of other facilities. Paolo. I really don't get it. Is current behavior really Standard compliant ? I don't think so and would like to fix it. The Standard says that pair& operator=(const pair&) requires that both is_copy_assignable and is_copy_assignable to be true. With std::pair, is_copy_assignable is false and then the operator is ill formed. It is if you try to call it but if you check is_copy_assignable> it says true. Do you see it as a correct behavior ? Do you really think that it requires an ISO DR ? François
Re: Fix std::pair std::is_copy_assignable behavior
Hi, On 4/17/13 8:43 PM, François Dumont wrote: On 04/17/2013 09:18 PM, Paolo Carlini wrote: On 4/17/13 8:10 PM, François Dumont wrote: Hi Here is an other proposal to fix std::is_copy_assignable>. Sorry, I'm still missing something very, very basic: which behavior is conforming, the current one or what we would get instead? If the former, is there a DR arguing for the latter? Paolo. The behavior I am targeting is std::is_copy_asignable> to be std::false_type for instance. I have added test for many other use cases. More generally I need that when std::is_copy_assignable is std::true_type then writing a = b, with a and b being T, does compile. Otherwise this patch just make std::pair match the Standard requirements like at 20.3.2.17. Do you want me to add a bug report in Bugzilla first ? I'm not talking about GCC's Bugzilla, I meant an ISO DR: if we don't have a DR and preferably a vague support of LWG people, I think we should not change the behavior of our std::is_copy_assignable only because it helps our implementation of other facilities. Paolo.
Re: Fix std::pair std::is_copy_assignable behavior
On 04/17/2013 09:18 PM, Paolo Carlini wrote: On 4/17/13 8:10 PM, François Dumont wrote: Hi Here is an other proposal to fix std::is_copy_assignable>. Sorry, I'm still missing something very, very basic: which behavior is conforming, the current one or what we would get instead? If the former, is there a DR arguing for the latter? Paolo. The behavior I am targeting is std::is_copy_asignable> to be std::false_type for instance. I have added test for many other use cases. More generally I need that when std::is_copy_assignable is std::true_type then writing a = b, with a and b being T, does compile. Otherwise this patch just make std::pair match the Standard requirements like at 20.3.2.17. Do you want me to add a bug report in Bugzilla first ? François
Re: Fix std::pair std::is_copy_assignable behavior
On 4/17/13 8:10 PM, François Dumont wrote: Hi Here is an other proposal to fix std::is_copy_assignable>. Sorry, I'm still missing something very, very basic: which behavior is conforming, the current one or what we would get instead? If the former, is there a DR arguing for the latter? Paolo.
Re: Fix std::pair std::is_copy_assignable behavior
Hi
Here is an other proposal to fix std::is_copy_assignable>.
This is not perfect because I have adapted it to current compiler
behavior but it is still better than current behavior and enough to
commit the unordered C++11 allocator adaptation afterward. It will give
me more time to work on the Standard modification proposal to avoid the
partial template specialization used for the moment.
Thanks to current resolution of DR 1402 we can already define the
move assignment operator as default, if deleted the template move
assignment operator will be considered and potentially used instead.
2013-04-17 François Dumont
* include/bits/stl_pair.h (operator=(const pair&)): Add noexcept
qualification.
(operator=(pair&&)): Use default implementation.
(template<> operator=(const pair&)): Add noexcept
qualification. Enable if is_assignable true for both
parameter types.
(template<> operator=(pair<>&&)): Add noexcept
qualification. Enable if is_assignable true for both
parameter types.
(std::is_copy_assignable<>, std::is_move_assignable<>): Add
partial specialization.
* testsuite/20_util/pair/is_move_assignable.cc: New.
* testsuite/20_util/pair/is_copy_assignable.cc: Likewise.
* testsuite/20_util/pair/is_assignable.cc: Likewise.
* testsuite/20_util/pair/is_nothrow_move_assignable.cc: Likewise.
* testsuite/20_util/pair/assign_neg.cc: Likewise.
* testsuite/20_util/pair/is_nothrow_copy_assignable.cc: Likewise.
* testsuite/20_util/pair/assign.cc: Likewise.
Tested under Linux x86_64.
Ok to commit ?
François
Index: include/bits/stl_pair.h
===
--- include/bits/stl_pair.h (revision 197829)
+++ include/bits/stl_pair.h (working copy)
@@ -156,6 +156,8 @@
pair&
operator=(const pair& __p)
+ noexcept(__and_,
+ is_nothrow_copy_assignable<_T2>>::value)
{
first = __p.first;
second = __p.second;
@@ -163,18 +165,15 @@
}
pair&
- operator=(pair&& __p)
- noexcept(__and_,
- is_nothrow_move_assignable<_T2>>::value)
- {
- first = std::forward(__p.first);
- second = std::forward(__p.second);
- return *this;
- }
+ operator=(pair&&) = default;
template
- pair&
+ typename enable_if<__and_,
+ is_assignable<_T2&, const _U2&>>::value,
+ pair&>::type
operator=(const pair<_U1, _U2>& __p)
+ noexcept(__and_,
+ is_nothrow_assignable<_T2&, const _U2&>>::value)
{
first = __p.first;
second = __p.second;
@@ -182,8 +181,12 @@
}
template
- pair&
+ typename enable_if<__and_,
+ is_assignable<_T2&, _U2&&>>::value,
+ pair&>::type
operator=(pair<_U1, _U2>&& __p)
+ noexcept(__and_,
+ is_nothrow_assignable<_T2&, _U2&&>>::value)
{
first = std::forward<_U1>(__p.first);
second = std::forward<_U2>(__p.second);
@@ -287,6 +290,19 @@
{ return pair<_T1, _T2>(__x, __y); }
#endif
+#if __cplusplus >= 201103L
+ template
+struct is_copy_assignable>
+ : public __and_,
+ std::is_copy_assignable<_T2>>::type
+{ };
+
+ template
+struct is_move_assignable>
+ : public __and_,
+ std::is_move_assignable<_T2>>::type
+{ };
+#endif
/// @}
_GLIBCXX_END_NAMESPACE_VERSION
Index: testsuite/20_util/pair/is_assignable.cc
===
--- testsuite/20_util/pair/is_assignable.cc (revision 0)
+++ testsuite/20_util/pair/is_assignable.cc (revision 0)
@@ -0,0 +1,48 @@
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+// Copyright (C) 2013 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
+//
Re: Fix std::pair std::is_copy_assignable behavior
On 04/14/2013 03:33 AM, Gabriel Dos Reis wrote: Does DR 1402 resolution generalization need a Standard committee validation first ? I cannot see why we would want otherwise :-) -- Gaby I rather wonder if gcc only accept modifications that has been validated by the Standard committee first or if we can first apply the modification to challenge it as much as possible while discussing about it at the same time. Of course I ask because the compiler modification has a relatively limited (bad) impact like libstdc++ testsuite have shown.
Re: Fix std::pair std::is_copy_assignable behavior
> Does DR 1402 resolution generalization need a Standard committee validation > first ? I cannot see why we would want otherwise :-) -- Gaby
Re: Fwd: Fix std::pair std::is_copy_assignable behavior
On 04/13/2013 09:21 PM, François Dumont wrote: Does DR 1402 resolution generalization need a Standard committee validation first ? In my opinion, it's much more clear to send the C++ front-end patch *separately* together with a simple C++-only (no library) testcase. I would also CC Jason. Paolo.
Fwd: Fix std::pair std::is_copy_assignable behavior
Hider
Here is a patch already posted to libstdc++ mailing but I am
resending following libstdc++ maintainers advises to add gcc-patches
mailing list.
This patch proposal is to fix the behavior of std::pair regarding
the std::is_*_assignable meta programming functions.
As announced it is requiring a compiler patch to extend DR 1402
resolution to all defaulted methods.
2013-04-12 François Dumont
* call.c (joust): Extend DR 1402 to all defaulted methods.
This modification is mandatory so that pair& operator=(const pair&)
can be defaulted whereas leaving gcc consider the other operator= in
some situations like std::pair. This way, with usage of
std::enable_if on the template operator=, we can control when p1= p2 is
a valid expression resulting in a correct behavior of
std::is_copy_assignable.
For the moment I preferred to add a dg-require-normal-mode option
in the single test that fail to compile because of the compiler
modification.
Does DR 1402 resolution generalization need a Standard committee
validation first ?
2013-04-13 François Dumont
* include/bits/stl_pair.h (operator=(const pair&)): Defaulted.
(operator=(pair&&)): Likewise.
(template<> operator=(const pair&)): Add noexcept
qualification. Enable if is_assignable true for both
parameters.
(template<> operator=(pair<>&&)): Add noexcept
qualification. Enable if is_assignable true for both
parameters.
* testsuite/23_containers/unordered_set/55043.cc: Add
dg-require-normal-mode.
* testsuite/20_util/pair/is_move_assignable.cc: New.
* testsuite/20_util/pair/is_copy_assignable.cc: Likewise.
* testsuite/20_util/pair/is_assignable.cc: Likewise.
* testsuite/20_util/pair/is_nothrow_move_assignable.cc: Likewise.
* testsuite/20_util/pair/assign_neg.cc: Likewise.
* testsuite/20_util/pair/is_nothrow_copy_assignable.cc: Likewise.
* testsuite/20_util/pair/assign.cc: Likewise.
François
Index: call.c
===
--- call.c (revision 197829)
+++ call.c (working copy)
@@ -8377,19 +8377,20 @@
&& (IS_TYPE_OR_DECL_P (cand1->fn)))
return 1;
- /* Prefer a non-deleted function over an implicitly deleted move
- constructor or assignment operator. This differs slightly from the
- wording for issue 1402 (which says the move op is ignored by overload
- resolution), but this way produces better error messages. */
+ /* Prefer a non-deleted function over an implicitly deleted one. This
+ differs slightly from the wording for issue 1402 because:
+ - it is extended to all defaulted functions, not only the ones with
+ move semantic
+ - it says the op is ignored by overload resolution while we are
+ only making it a worst candidate, but this way produces better error
+ messages. */
if (TREE_CODE (cand1->fn) == FUNCTION_DECL
&& TREE_CODE (cand2->fn) == FUNCTION_DECL
&& DECL_DELETED_FN (cand1->fn) != DECL_DELETED_FN (cand2->fn))
{
- if (DECL_DELETED_FN (cand1->fn) && DECL_DEFAULTED_FN (cand1->fn)
- && move_fn_p (cand1->fn))
+ if (DECL_DELETED_FN (cand1->fn) && DECL_DEFAULTED_FN (cand1->fn))
return -1;
- if (DECL_DELETED_FN (cand2->fn) && DECL_DEFAULTED_FN (cand2->fn)
- && move_fn_p (cand2->fn))
+ if (DECL_DELETED_FN (cand2->fn) && DECL_DEFAULTED_FN (cand2->fn))
return 1;
}
Index: include/bits/stl_pair.h
===
--- include/bits/stl_pair.h (revision 197829)
+++ include/bits/stl_pair.h (working copy)
@@ -155,26 +155,18 @@
pair(piecewise_construct_t, tuple<_Args1...>, tuple<_Args2...>);
pair&
- operator=(const pair& __p)
- {
- first = __p.first;
- second = __p.second;
- return *this;
- }
+ operator=(const pair&) = default;
pair&
- operator=(pair&& __p)
- noexcept(__and_,
- is_nothrow_move_assignable<_T2>>::value)
- {
- first = std::forward(__p.first);
- second = std::forward(__p.second);
- return *this;
- }
+ operator=(pair&&) = default;
template
- pair&
+ typename enable_if<__and_,
+ is_assignable<_T2&, const _U2&>>::value,
+ pair&>::type
operator=(const pair<_U1, _U2>& __p)
+ noexcept(__and_,
+ is_nothrow_assignable<_T2&, const _U2&>>::value)
{
first = __p.first;
second = __p.second;
@@ -182,8 +174,12 @@
}
template
- pair&
+ typename enable_if<__and_,
+ is_assignable<_T2&, _U2&&>>::value,
+ pair&>::type
operator=(pair<_U1, _U2>&& __p)
+ noexcept(__and_,
+ is_nothrow_assignable<_T2&, _U2&&>>::value)
{
first = std::forward<_U1>(__p.first);
second = std::forward<_U2>(__p.second);
Index: testsuite/23_containers/unordered_set/55043.cc
===
--- testsuite/23_containers/unordered_set/55043.cc (revision 197829)
+++ test
