I'll send the patches for trunk, gcc-4_7-branch, and gcc-4_6-branch in a separate thread later today ...
13 years, 1 month ago
(2012-04-02 22:25:02 UTC)
#1
I'll send the patches for trunk, gcc-4_7-branch, and gcc-4_6-branch in
a separate thread later today or tomorrow, but I need to get the
google branch fixed before that. Paolo suggested this simple fix was
the right approach for the gcc-4_6-branch, so I'm using it for
google/gcc-4_6 too. I'll be happy to fix up the google branch to match
whatever gets accepted for gcc-4_6-branch once that's decided.
Tested with `make check` on x86-64-linux, and I'll check this against
the build I originally noticed the problem in before committing it.
2012-04-02 Jeffrey Yasskin <jyasskin@google.com>
* libstdc++-v3/include/bits/stl_algo.h (__stable_partition_adaptive):
Avoid move-assigning an object to itself by explicitly testing
for identity.
* libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc:
Test vectors, which have a destructive self-move-assignment.
* libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc (test02):
Test with rvalstruct, which explicitly checks self-move-assignment.
Index: libstdc++-v3/include/bits/stl_algo.h
===================================================================
--- libstdc++-v3/include/bits/stl_algo.h (revision 186093)
+++ libstdc++-v3/include/bits/stl_algo.h (working copy)
@@ -1844,7 +1844,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
for (; __first != __last; ++__first)
if (__pred(*__first))
{
- *__result1 = _GLIBCXX_MOVE(*__first);
+ if (&*__result1 != &*__first)
+ *__result1 = _GLIBCXX_MOVE(*__first);
++__result1;
}
else
Index: libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc
===================================================================
--- libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc (revision
0)
+++ libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc (revision
0)
@@ -0,0 +1,43 @@
+// { dg-options "-std=gnu++0x" }
+
+// Copyright (C) 2012 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 Pred 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/>.
+
+// 25.2.12 [lib.alg.partitions] Partitions.
+
+#include <algorithm>
+#include <vector>
+#include <testsuite_hooks.h>
+
+bool true_vector_pred(const std::vector<int>&) { return true; }
+
+void
+test01()
+{
+ std::vector<std::vector<int> > v(1);
+ v[0].push_back(7);
+ VERIFY( v[0].size() == 1 );
+ std::stable_partition(v.begin(), v.end(), &true_vector_pred);
+ VERIFY( v[0].size() == 1 );
+}
+
+int
+main()
+{
+ test01();
+ return 0;
+}
Index: libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc
===================================================================
--- libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc (revision
186093)
+++ libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc (working
copy)
@@ -1,6 +1,6 @@
// { dg-options "-std=gnu++0x" }
-// Copyright (C) 2009, 2010 Free Software Foundation, Inc.
+// Copyright (C) 2009, 2010, 2012 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
@@ -39,6 +39,12 @@ const int A[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
const int B[] = {2, 4, 6, 8, 10, 12, 14, 16, 1, 3, 5, 7, 9, 11, 13, 15, 17};
const int N = sizeof(A) / sizeof(int);
+// Check that starting with a value the predicate returns true for
+// works too. (PR52822)
+const int A2[] = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17};
+const int B2[] = {2, 4, 6, 8, 10, 12, 14, 16, 3, 5, 7, 9, 11, 13, 15, 17};
+const int N2 = sizeof(A2) / sizeof(int);
+
struct Pred
{
bool
@@ -46,7 +52,8 @@ struct Pred
{ return (x.val % 2) == 0; }
};
-// 25.2.12 stable_partition()
+// 25.2.12 stable_partition(), starting with a value for which the
+// predicate returns false.
void
test01()
{
@@ -60,9 +67,25 @@ test01()
VERIFY( std::equal(s1, s1 + N, B) );
}
+// 25.2.12 stable_partition(), starting with a value for which the
+// predicate returns true.
+void
+test02()
+{
+ bool test __attribute__((unused)) = true;
+
+ rvalstruct s1[N2];
+ std::copy(A2, A2 + N2, s1);
+ Container con(s1, s1 + N2);
+
+ std::stable_partition(con.begin(), con.end(), Pred());
+ VERIFY( std::equal(s1, s1 + N2, B2) );
+}
+
int
main()
{
test01();
+ test02();
return 0;
}
--
This patch is available for review at http://codereview.appspot.com/5975063
My original test case passes with this change. On Mon, Apr 2, 2012 at 3:25 ...
13 years, 1 month ago
(2012-04-03 00:08:48 UTC)
#2
My original test case passes with this change.
On Mon, Apr 2, 2012 at 3:25 PM, Jeffrey Yasskin <jyasskin@google.com> wrote:
> I'll send the patches for trunk, gcc-4_7-branch, and gcc-4_6-branch in
> a separate thread later today or tomorrow, but I need to get the
> google branch fixed before that. Paolo suggested this simple fix was
> the right approach for the gcc-4_6-branch, so I'm using it for
> google/gcc-4_6 too. I'll be happy to fix up the google branch to match
> whatever gets accepted for gcc-4_6-branch once that's decided.
>
> Tested with `make check` on x86-64-linux, and I'll check this against
> the build I originally noticed the problem in before committing it.
>
> 2012-04-02 Jeffrey Yasskin <jyasskin@google.com>
>
> * libstdc++-v3/include/bits/stl_algo.h (__stable_partition_adaptive):
> Avoid move-assigning an object to itself by explicitly testing
> for identity.
> * libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc:
> Test vectors, which have a destructive self-move-assignment.
> * libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc
(test02):
> Test with rvalstruct, which explicitly checks self-move-assignment.
>
> Index: libstdc++-v3/include/bits/stl_algo.h
> ===================================================================
> --- libstdc++-v3/include/bits/stl_algo.h (revision 186093)
> +++ libstdc++-v3/include/bits/stl_algo.h (working copy)
> @@ -1844,7 +1844,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> for (; __first != __last; ++__first)
> if (__pred(*__first))
> {
> - *__result1 = _GLIBCXX_MOVE(*__first);
> + if (&*__result1 != &*__first)
> + *__result1 = _GLIBCXX_MOVE(*__first);
> ++__result1;
> }
> else
> Index: libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc
> ===================================================================
> --- libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc
(revision 0)
> +++ libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc
(revision 0)
> @@ -0,0 +1,43 @@
> +// { dg-options "-std=gnu++0x" }
> +
> +// Copyright (C) 2012 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 Pred 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/>.
> +
> +// 25.2.12 [lib.alg.partitions] Partitions.
> +
> +#include <algorithm>
> +#include <vector>
> +#include <testsuite_hooks.h>
> +
> +bool true_vector_pred(const std::vector<int>&) { return true; }
> +
> +void
> +test01()
> +{
> + std::vector<std::vector<int> > v(1);
> + v[0].push_back(7);
> + VERIFY( v[0].size() == 1 );
> + std::stable_partition(v.begin(), v.end(), &true_vector_pred);
> + VERIFY( v[0].size() == 1 );
> +}
> +
> +int
> +main()
> +{
> + test01();
> + return 0;
> +}
> Index: libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc
> ===================================================================
> --- libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc
(revision 186093)
> +++ libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc
(working copy)
> @@ -1,6 +1,6 @@
> // { dg-options "-std=gnu++0x" }
>
> -// Copyright (C) 2009, 2010 Free Software Foundation, Inc.
> +// Copyright (C) 2009, 2010, 2012 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
> @@ -39,6 +39,12 @@ const int A[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
> const int B[] = {2, 4, 6, 8, 10, 12, 14, 16, 1, 3, 5, 7, 9, 11, 13, 15, 17};
> const int N = sizeof(A) / sizeof(int);
>
> +// Check that starting with a value the predicate returns true for
> +// works too. (PR52822)
> +const int A2[] = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17};
> +const int B2[] = {2, 4, 6, 8, 10, 12, 14, 16, 3, 5, 7, 9, 11, 13, 15, 17};
> +const int N2 = sizeof(A2) / sizeof(int);
> +
> struct Pred
> {
> bool
> @@ -46,7 +52,8 @@ struct Pred
> { return (x.val % 2) == 0; }
> };
>
> -// 25.2.12 stable_partition()
> +// 25.2.12 stable_partition(), starting with a value for which the
> +// predicate returns false.
> void
> test01()
> {
> @@ -60,9 +67,25 @@ test01()
> VERIFY( std::equal(s1, s1 + N, B) );
> }
>
> +// 25.2.12 stable_partition(), starting with a value for which the
> +// predicate returns true.
> +void
> +test02()
> +{
> + bool test __attribute__((unused)) = true;
> +
> + rvalstruct s1[N2];
> + std::copy(A2, A2 + N2, s1);
> + Container con(s1, s1 + N2);
> +
> + std::stable_partition(con.begin(), con.end(), Pred());
> + VERIFY( std::equal(s1, s1 + N2, B2) );
> +}
> +
> int
> main()
> {
> test01();
> + test02();
> return 0;
> }
>
> --
> This patch is available for review at http://codereview.appspot.com/5975063
13 years, 1 month ago
(2012-04-03 13:13:08 UTC)
#3
On 4/2/12 6:25 PM, Jeffrey Yasskin wrote:
> 2012-04-02 Jeffrey Yasskin<jyasskin@google.com>
>
> * libstdc++-v3/include/bits/stl_algo.h (__stable_partition_adaptive):
> Avoid move-assigning an object to itself by explicitly testing
> for identity.
> * libstdc++-v3/testsuite/25_algorithms/stable_partition/pr52822.cc:
> Test vectors, which have a destructive self-move-assignment.
> * libstdc++-v3/testsuite/25_algorithms/stable_partition/moveable.cc (test02):
> Test with rvalstruct, which explicitly checks self-move-assignment.
This is OK for google/gcc-4_6, if trunk review requests changes, you can
backport them later.
Diego.
Issue 5975063: [google] Fix PR52822 in google's gcc-4.6 branch
Created 13 years, 1 month ago by Jeffrey Yasskin (google)
Modified 13 years, 1 month ago
Reviewers: Diego Novillo
Base URL: svn+ssh://gcc.gnu.org/svn/gcc/branches/google/gcc-4_6/
Comments: 0
