Re: [PATCH v2] [4.6] shared_ptr needs explicit copy constructor
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
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
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
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
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
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