Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Jonathan Wakely
On 3 January 2012 19:17, Chase Douglas wrote:

 PR c++/50500
 * include/bits/shared_ptr.h: Add lazy copy ops even if there's a move

That is the ChangeLog for the front-end part of 50500, isn't it?
Should be something like Default copy ctor and assignment.

Otherwise this is OK to check in, thanks.

(Do you need me to do the actual check in or can Matthias do it?)


Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Chase Douglas
On 01/03/2012 12:34 PM, Jonathan Wakely wrote:
 On 3 January 2012 19:17, Chase Douglas wrote:

 PR c++/50500
 * include/bits/shared_ptr.h: Add lazy copy ops even if there's a move
 
 That is the ChangeLog for the front-end part of 50500, isn't it?
 Should be something like Default copy ctor and assignment.

I just copied the changelog text from commit 180159. That was the
suggested approach in the style guidelines, though I do agree that
Default copy ctor and assignment makes more sense. What do you suggest
I do?

 Otherwise this is OK to check in, thanks.
 
 (Do you need me to do the actual check in or can Matthias do it?)

I don't know if Matthias can do it. I copied him to keep him in the loop
since he is the toolchain maintainer for Ubuntu and I would like to get
this into the next release. If you have access, I would suggest you do
it since Matthias hasn't been involved in the generation or review of
this patch.

Thanks!

-- Chase


Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Jonathan Wakely
On 3 January 2012 20:45, Chase Douglas wrote:
 On 01/03/2012 12:34 PM, Jonathan Wakely wrote:
 On 3 January 2012 19:17, Chase Douglas wrote:

 PR c++/50500
 * include/bits/shared_ptr.h: Add lazy copy ops even if there's a move

 That is the ChangeLog for the front-end part of 50500, isn't it?
 Should be something like Default copy ctor and assignment.

 I just copied the changelog text from commit 180159. That was the
 suggested approach in the style guidelines, though I do agree that
 Default copy ctor and assignment makes more sense. What do you suggest
 I do?

Use the text from the libstdc++-v3/ChangeLog for the shared_prt.h change:
http://gcc.gnu.org/viewcvs/trunk/libstdc%2B%2B-v3/ChangeLog?r1=180159r2=180158pathrev=180159

You can look in libstdc++-v3/ChangeLog to find that too.

Jason's commit message was the text from gcc/cp/ChangeLog, but he
touched several parts (the C++ front-end, the C++ library and the
testsuite) which all have their own ChangeLog files.


 Otherwise this is OK to check in, thanks.

 (Do you need me to do the actual check in or can Matthias do it?)

 I don't know if Matthias can do it. I copied him to keep him in the loop
 since he is the toolchain maintainer for Ubuntu and I would like to get
 this into the next release. If you have access, I would suggest you do
 it since Matthias hasn't been involved in the generation or review of
 this patch.

OK, I'll do it.

Thanks.


Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Jonathan Wakely
I get testsuite failures with this change applied:

FAIL: 20_util/shared_ptr/cons/43820_neg.cc  (test for errors, line 858)
FAIL: 20_util/shared_ptr/cons/43820_neg.cc (test for excess errors)
FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 354)
FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 1085)
FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc (test for excess errors)

Did you not see them when testing?

Here's what I'm checking in

2012-01-03  Chase Douglas  chase.doug...@canonical.com
Jonathan Wakely  jwakely@gmail.com

* include/bits/shared_ptr.h: Default copy ctor and assignment.
* include/bits/shared_ptr_base.h: Likewise.
* testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust dg-error
line numbers.
* testsuite/20_util/weak_ptr/comparison/cmp_neg.cc: Likewise.
Index: include/bits/shared_ptr.h
===
--- include/bits/shared_ptr.h   (revision 182460)
+++ include/bits/shared_ptr.h   (working copy)
@@ -100,6 +100,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr shared_ptr()
   : __shared_ptr_Tp() { }
 
+  shared_ptr(const shared_ptr) = default; // never throws
+
   /**
*  @brief  Construct a %shared_ptr that owns the pointer @a __p.
*  @param  __p  A pointer that is convertible to element_type*.
@@ -264,6 +266,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr shared_ptr(nullptr_t __p)
   : __shared_ptr_Tp(__p) { }
 
+  shared_ptr operator=(const shared_ptr) = default;
+
   templatetypename _Tp1
shared_ptr
operator=(const shared_ptr_Tp1 __r) // never throws
Index: include/bits/shared_ptr_base.h
===
--- include/bits/shared_ptr_base.h  (revision 182460)
+++ include/bits/shared_ptr_base.h  (working copy)
@@ -799,7 +799,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: _M_ptr(__p), _M_refcount(__r._M_refcount) // never throws
{ }
 
-  //  generated copy constructor, assignment, destructor are fine.
+  __shared_ptr(const __shared_ptr) = default; // never throws
+  __shared_ptr operator=(const __shared_ptr) = default; // never throws
 
   templatetypename _Tp1, typename = typename
   std::enable_ifstd::is_convertible_Tp1*, _Tp*::value::type
Index: testsuite/20_util/shared_ptr/cons/43820_neg.cc
===
--- testsuite/20_util/shared_ptr/cons/43820_neg.cc  (revision 182460)
+++ testsuite/20_util/shared_ptr/cons/43820_neg.cc  (working copy)
@@ -35,6 +35,6 @@ void test01()
   // { dg-error incomplete  { target *-*-* } 766 }
 
   std::shared_ptrX p9(ap());  // { dg-error here }
-  // { dg-error incomplete  { target *-*-* } 858 }
+  // { dg-error incomplete  { target *-*-* } 859 }
 
 }
Index: testsuite/20_util/weak_ptr/comparison/cmp_neg.cc
===
--- testsuite/20_util/weak_ptr/comparison/cmp_neg.cc(revision 182460)
+++ testsuite/20_util/weak_ptr/comparison/cmp_neg.cc(working copy)
@@ -42,8 +42,8 @@ main()
   return 0;
 }
 
-// { dg-warning note  { target *-*-* } 354 }
-// { dg-warning note  { target *-*-* } 1085 }
+// { dg-warning note  { target *-*-* } 358 }
+// { dg-warning note  { target *-*-* } 1086 }
 // { dg-warning note  { target *-*-* } 468 }
 // { dg-warning note  { target *-*-* } 586 }
 // { dg-warning note  { target *-*-* } 1049 }


Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Chase Douglas
On 01/03/2012 01:38 PM, Jonathan Wakely wrote:
 I get testsuite failures with this change applied:
 
 FAIL: 20_util/shared_ptr/cons/43820_neg.cc  (test for errors, line 858)
 FAIL: 20_util/shared_ptr/cons/43820_neg.cc (test for excess errors)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 354)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 1085)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc (test for excess errors)
 
 Did you not see them when testing?

They didn't affect me when I ran my own library's test suite, and they
didn't cause the packaged build for Ubuntu to fail. I saw there were
many test failures in the Ubuntu packaged version, so I didn't really
pick through them.

 Here's what I'm checking in
 
 2012-01-03  Chase Douglas  chase.doug...@canonical.com
 Jonathan Wakely  jwakely@gmail.com
 
 * include/bits/shared_ptr.h: Default copy ctor and assignment.
 * include/bits/shared_ptr_base.h: Likewise.
 * testsuite/20_util/shared_ptr/cons/43820_neg.cc: Adjust dg-error
 line numbers.
 * testsuite/20_util/weak_ptr/comparison/cmp_neg.cc: Likewise.

Ok, thanks. Next time I'll be sure to pay closer attention to the
testsuite output.


Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor

2012-01-03 Thread Jonathan Wakely
On 3 January 2012 21:48, Chase Douglas wrote:
 On 01/03/2012 01:38 PM, Jonathan Wakely wrote:
 I get testsuite failures with this change applied:

 FAIL: 20_util/shared_ptr/cons/43820_neg.cc  (test for errors, line 858)
 FAIL: 20_util/shared_ptr/cons/43820_neg.cc (test for excess errors)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 354)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc  (test for warnings, line 1085)
 FAIL: 20_util/weak_ptr/comparison/cmp_neg.cc (test for excess errors)

 Did you not see them when testing?

 They didn't affect me when I ran my own library's test suite, and they
 didn't cause the packaged build for Ubuntu to fail. I saw there were
 many test failures in the Ubuntu packaged version, so I didn't really
 pick through them.

We don't care about your library or about the Ubuntu package :)

If you're proposing a change to the FSF sources for GCC then you need
to build and test the change against the FSF sources, and the
libstdc++ testsuite is currently error-free on the 4.6 branch (e.g.
see this report
http://gcc.gnu.org/ml/gcc-testresults/2012-01/msg00208.html on the
gcc-testresults list) so we don't want a change which introduces
failures. Unfortunately those particular files need to be adjusted
whenever any lines are added or removed from shared_ptr.h