diff mbox

[v3] PR libstdc++/78389

Message ID CAFk2RUYTg4BJGFnCQWYMGkBK6+KKtLETKMBKp+bS1-OuR2W1OA@mail.gmail.com
State New
Headers show

Commit Message

Ville Voutilainen Jan. 15, 2017, 5:07 p.m. UTC
On 15 January 2017 at 19:01, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> On 15 January 2017 at 18:42, Tim Song <t.canens.cpp@gmail.com> wrote:
>> On rereading the patch today, the size calculation for merge() appears
>> to be backwards. [__first2, __last2) consists of the nodes not
>> transferred into *this, so the new size of __x should be __dist while
>> this->size() should be incremented by (__orig_size - __dist).
>
> Ah, yes, I'm an idiot. Fixing...

2017-01-15  Ville Voutilainen  <ville.voutilainen@gmail.com>

    PR libstdc++/78389
    Fix backwards size adjustments.
    * include/bits/list.tcc (merge(list&&)):
    Fix backwards size adjustments.
    (merge(list&&, _StrictWeakOrdering)): Likewise.
    * testsuite/23_containers/list/operations/78389.cc: Add
    better test for the sizes.

Comments

Ville Voutilainen Jan. 15, 2017, 5:22 p.m. UTC | #1
On 15 January 2017 at 19:07, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> On 15 January 2017 at 19:01, Ville Voutilainen
> <ville.voutilainen@gmail.com> wrote:
>> On 15 January 2017 at 18:42, Tim Song <t.canens.cpp@gmail.com> wrote:
>>> On rereading the patch today, the size calculation for merge() appears
>>> to be backwards. [__first2, __last2) consists of the nodes not
>>> transferred into *this, so the new size of __x should be __dist while
>>> this->size() should be incremented by (__orig_size - __dist).
>>
>> Ah, yes, I'm an idiot. Fixing...
>
> 2017-01-15  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>     PR libstdc++/78389
>     Fix backwards size adjustments.
>     * include/bits/list.tcc (merge(list&&)):
>     Fix backwards size adjustments.
>     (merge(list&&, _StrictWeakOrdering)): Likewise.
>     * testsuite/23_containers/list/operations/78389.cc: Add
>     better test for the sizes.

Hmm, and yeah, your test uses a different throw-after number, so I
should change the tests to do the same. :)
Jonathan Wakely Jan. 16, 2017, 11:24 a.m. UTC | #2
On 15/01/17 19:07 +0200, Ville Voutilainen wrote:
>    PR libstdc++/78389
>    Fix backwards size adjustments.

I don't think repeating this text here and ...

>    * include/bits/list.tcc (merge(list&&)):
>    Fix backwards size adjustments.

... here is useful.

More useful would be a good Git-style commit log with a separate
description on the first line. So the svn commit would have a subject
line, followed by a blank line, followed by the content of the
ChangeLog entry e.g.

PR78389 fix backwards size adjustments

	PR libstdc++/78389
	* include/bits/list.tcc (merge(list&&)): Fix backwards size
	adjustments.
	(merge(list&&, _StrictWeakOrdering)): Likewise.
	* testsuite/23_containers/list/operations/78389.cc: Add
	better test for the sizes.

See the output of "git log --pretty=oneline --abbrev-commit" for why
that's useful (and how messy and unhelpful that output is when commits
don't have a separate summary on the first line). See also
http://chris.beams.io/posts/git-commit/

Anyway ...

OK for trunk with the additional changes to use better magic numbers
in the tests.
Jonathan Wakely Jan. 16, 2017, 11:33 a.m. UTC | #3
On 16/01/17 11:24 +0000, Jonathan Wakely wrote:
>OK for trunk with the additional changes to use better magic numbers
>in the tests.

Oh, and for the branches too.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/list.tcc b/libstdc++-v3/include/bits/list.tcc
index 5be49a8..d80d569 100644
--- a/libstdc++-v3/include/bits/list.tcc
+++ b/libstdc++-v3/include/bits/list.tcc
@@ -406,8 +406,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  __catch(...)
 	    {
 	      size_t __dist = std::distance(__first2, __last2);
-	      this->_M_inc_size(__dist);
-	      __x._M_set_size(__orig_size - __dist);
+	      this->_M_inc_size(__orig_size - __dist);
+	      __x._M_set_size(__dist);
 	      __throw_exception_again;
 	    }
 	}
@@ -454,8 +454,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	    __catch(...)
 	      {
 		size_t __dist = std::distance(__first2, __last2);
-		this->_M_inc_size(__dist);
-		__x._M_set_size(__orig_size - __dist);
+		this->_M_inc_size(__orig_size - __dist);
+		__x._M_set_size(__dist);
 		__throw_exception_again;
 	      }
 	  }
diff --git a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
index 1cf9b0c..3002ba6 100644
--- a/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/operations/78389.cc
@@ -61,6 +61,8 @@  int main()
   } catch (...) {
   }
   VERIFY(a.size() == 8 && b.size() == 4);
+  VERIFY(a.size() == std::distance(a.begin(), a.end()) &&
+	 b.size() == std::distance(b.begin(), b.end()));
   std::list<X> ax{1, 2, 3, 4};
   std::list<X> bx{5, 6, 7, 8, 9, 10, 11, 12};
   throw_after_X = 5;
@@ -69,6 +71,8 @@  int main()
   } catch (...) {
   }
   VERIFY(ax.size() == 8 && bx.size() == 4);
+  VERIFY(ax.size() == std::distance(ax.begin(), ax.end()) &&
+	 bx.size() == std::distance(bx.begin(), bx.end()));
   std::list<int> ay{5, 6, 7, 8, 9, 10, 11, 12};
   try {
     ay.sort(ThrowingComparator{5});