The following program is correct in C++11, but uses a deprecated language feature. According section D.3, paragraph 1: "The implicit definition of a copy constructor as defaulted is deprecated if the class has a user-declared copy assignment operator or a user-declared destructor. The implicit definition of a copy assignment operator as defaulted is deprecated if the class has a user-declared copy constructor or a user-declared destructor." G++ should emit a warning in C++11 mode. ==================== struct W { int a; ~W() { a = 9; } }; int main() { W w {}; W v = w; }
Out of curiosity, are you aware of other compilers emitting such warning? With which wording, in case? Because I don't seem to be able to get one from current clang and icc.
No. Other compilers (Clang and VS 2010) do not emit such warning either.
I see. Have to double check our policy in this area, I'm not at all sure that in general anything in Section D gets a warning.
I don't think anything in appendix D apart from auto_ptr does. The warning would be useful to help find places where defaulted definitions could be added, but I'm not sure I'd want it enabled by -Wall. YMMV.
(In reply to Jonathan Wakely from comment #4) -pedantic would seem like a good candidate, no?
(In reply to Andrzej Krzemienski from comment #2) > No. Other compilers (Clang and VS 2010) do not emit such warning either. Correction: The newest version of Clang does give a warning: "definition of implicit copy constructor for 'W' is deprecated because it has a user-declared destructor [-Wdeprecated]"
Just one remark. A warning in this situation is not to just warn about any deprecated feature, but to indicate something that is very likely to be a bug (It used to be legal in C++03, so you couldn't legally warn about it; now the deprecation is just an additional incentive). My guess is that having this copy constructor implicitly defined when you have a custom destructor is a potential bug in half of the cases. (Well, "half" is my guess.)
I found this thread because I went Googling after I discovered a bug in my code resulting from the copy operations being generated despite my having written a destructor. I was angry that my compiler (Microsoft) hadn't emitted a warning. Please add this warning if you haven't already.
(In reply to Andrzej Krzemienski from comment #7) > (It used to be legal in C++03, so you couldn't legally warn about it; That's not true, the compiler can (and does) warn about legal code. I'm confirming this, we will want the warning at some point, and it would allow us to improve this part of the -Weffc++ warnings: * Item 11: Define a copy constructor and an assignment operator for classes with dynamically allocated memory. (see PR 16166 for more details) Maybe we could call this warning -Wdeprecated-special-members, and have it enabled -Weffc++ and in C++11 also by -Wdeprecated
I guess, by now "item 11" should read "either delete or define ..."
Created attachment 43637 [details] P
Here's an implementation to play with. It breaks lots of tests in the library, which I'm not going to continue poking at now. The deque warning in particular seems tricky to address.
Thanks! I can take care of the rest of the library, but for now this makes all the deque tests pass: --- a/libstdc++-v3/include/bits/stl_deque.h +++ b/libstdc++-v3/include/bits/stl_deque.h @@ -149,9 +149,23 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _Deque_iterator() _GLIBCXX_NOEXCEPT : _M_cur(), _M_first(), _M_last(), _M_node() { } +#if __cplusplus < 201103L + // Conversion from iterator to const_iterator. _Deque_iterator(const iterator& __x) _GLIBCXX_NOEXCEPT : _M_cur(__x._M_cur), _M_first(__x._M_first), _M_last(__x._M_last), _M_node(__x._M_node) { } +#else + // Conversion from iterator to const_iterator. + template<typename _Iter, + typename = _Require<is_same<_Self, const_iterator>, + is_same<_Iter, iterator>>> + _Deque_iterator(const _Iter& __x) noexcept + : _M_cur(__x._M_cur), _M_first(__x._M_first), + _M_last(__x._M_last), _M_node(__x._M_node) { } + + _Deque_iterator(const _Deque_iterator&) = default; + _Deque_iterator& operator=(const _Deque_iterator&) = default; +#endif iterator _M_const_cast() const _GLIBCXX_NOEXCEPT diff --git a/libstdc++-v3/include/ext/throw_allocator.h b/libstdc++-v3/include/ext/throw_allocator.h index 5d9caa2bcae..f773a2d79ba 100644 --- a/libstdc++-v3/include/ext/throw_allocator.h +++ b/libstdc++-v3/include/ext/throw_allocator.h @@ -403,6 +403,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION struct condition_base { virtual ~condition_base() { }; +#if __cplusplus >= 201103L + condition_base() = default; + condition_base(const condition_base&) = default; + condition_base& operator=(const condition_base&) = default; +#endif };
Created attachment 44135 [details] updated patch Here's an updated implementation, which is limited to -Wall, so it doesn't affect the building or testing of the library. I'm sure you will still want to adjust the library to avoid warnings when users try to build with -Wall. On looking more closely at some of the places I was getting warnings in the compiler, I noticed that they often seemed to be actual errors. For instance, if someone copies an aa_tree and then destroys one of the aa_trees, the other is left with a pointer to garbage. I also realized that we want to avoid warning when the copy is elided, particularly given C++17 mandatory copy elision; if the prvalue directly initializes a variable, there's no problematic copying. An interesting subcategory I noticed when I had the warning on by default is classes with a virtual destructor, such as the exception hierarchy. The warning calls attention to potential slicing problems, and so I think we don't want to add defaulted copy ops; it's good for users to be encouraged to e.g. catch by reference.
Author: jason Date: Fri May 18 20:02:48 2018 New Revision: 260381 URL: https://gcc.gnu.org/viewcvs?rev=260381&root=gcc&view=rev Log: PR c++/58407 - deprecated implicit copy ops. gcc/c-family/ * c.opt (Wdeprecated-copy): New flag. gcc/cp/ * call.c (build_over_call): Warn about deprecated trivial fns. * class.c (classtype_has_user_copy_or_dtor): New. (type_build_ctor_call): Check TREE_DEPRECATED. (type_build_dtor_call): Likewise. * decl2.c (cp_warn_deprecated_use): Move from tree.c. Add checks. Return bool. Handle -Wdeprecated-copy. (mark_used): Use it. * decl.c (grokdeclarator): Remove redundant checks. * typeck2.c (build_functional_cast): Likewise. * method.c (lazily_declare_fn): Mark deprecated copy ops. * init.c (build_aggr_init): Only set TREE_USED if there are side-effects. libitm/ * beginend.cc (save): Disable -Werror=deprecated-copy. Added: trunk/gcc/testsuite/g++.dg/cpp0x/depr-copy1.C Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c.opt trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/class.c trunk/gcc/cp/cp-tree.h trunk/gcc/cp/decl.c trunk/gcc/cp/decl2.c trunk/gcc/cp/init.c trunk/gcc/cp/method.c trunk/gcc/cp/tree.c trunk/gcc/cp/typeck2.c trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/g++.old-deja/g++.other/warn6.C trunk/libitm/ChangeLog trunk/libitm/beginend.cc
(In reply to Jason Merrill from comment #14) > An interesting subcategory I noticed when I had the warning on by default is > classes with a virtual destructor, such as the exception hierarchy. The > warning calls attention to potential slicing problems, and so I think we > don't want to add defaulted copy ops; it's good for users to be encouraged > to e.g. catch by reference. For the record, I changed my mind about this; a slicing warning should only apply to copies from an object of unknown dynamic type. -Wdeprecated-copy added for GCC 9.
Hi, Does this change mean now it's effectively impossible to have abstract base classes under -Wall without adding boilerplate? Consider something like the following: class Base { public: virtual ~Base() {} virtual void foo() = 0; }; class Derived : Base { public: ~Derived(); void foo() override; }; Base needs to have a virtual destructor since it has virtual member functions (or half the world will give you warnings). Any attempts now to copy Derived through the implicit copy constructor will give a warning, since the synthesis of Base::Base(const Base &) is deprecated. The only way I've found to suppress this is to add Base::Base(const Base &) = default; However, this in turn disables the synthesis of Base::Base(), and also Base::operator=(const Base &). So I need: Base() = default; Base(const Base &) = default; Base(Base &&) = default; Base &operator= (const Base &) = default; Base &operator= (Base &&) = default; for something that doesn't have a single member! Am I missing something?
(In reply to Steinar H. Gunderson from comment #17) > Base needs to have a virtual destructor since it has virtual member > functions (or half the world will give you warnings). Or a protected destructor, but that doesn't change anything relevant to your problem. > Any attempts now to copy Derived through the implicit copy constructor will > give a warning, since the synthesis of Base::Base(const Base &) is > deprecated. The only way I've found to suppress this is to add > > Base::Base(const Base &) = default; > > However, this in turn disables the synthesis of Base::Base(), and also > Base::operator=(const Base &). So I need: > > Base() = default; > Base(const Base &) = default; > Base(Base &&) = default; > Base &operator= (const Base &) = default; > Base &operator= (Base &&) = default; > > for something that doesn't have a single member! Yes. The deprecation means a future version of C++ might make your type non-copyable. To explicitly say it's copyable (and make the code futureproof) you need to add those defaulted members. If you don't want to do that, you can use -Wno-deprecated-copy to suppress the warnings.
Thanks for confirming; so GCC is absolutely right here, it's the standard that makes a choice with surprising ramifications (to me, at least). I wonder if I should try to ask someone in the standards committee to make an exception for pure virtual destructors...
(In reply to Steinar H. Gunderson from comment #19) > Thanks for confirming; so GCC is absolutely right here, it's the standard > that makes a choice with surprising ramifications (to me, at least). I Yes. GCC could choose not to warn about that deprecation, but the whole point of this bug report is that somebody was asking for GCC to do exactly that. > wonder if I should try to ask someone in the standards committee to make an > exception for pure virtual destructors... I've heard worse ideas. The whole topic of deprecating those members is already a bit contentious.
P.S. it's arguable whether abstract base classes should be copyable in the first place: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-copy-virtual https://github.com/isocpp/CppCoreGuidelines/issues/1151
C.67 would seem only to apply to non-abstract base classes, no? The code doesn't compile if B has pure virtual member functions. (Well, it doesn't compile as-is already, but change (d) to (*d) and it does.)
Right, sorry, I added that last comment too hastily.
So with this the rule-of-three is now the rule-of-four or six?
(In reply to Allan Jensen from comment #24) > So with this the rule-of-three is now the rule-of-four or six? The Rule of Zero: https://blog.rmf.io/cxx11/rule-of-zero
That blog post seems to advocate using std::unique_ptr for pretty much everything, which unfortunately doesn't always work. See e.g. slide 8 of https://github.com/CppCon/CppCon2018/blob/master/Presentations/woes_of_scope_guards_and_unique_resource/woes_of_scope_guards_and_unique_resource__peter_sommerlad__cppcon_2018.pdf -- C++20 will have std::unique_resource, but we're not there yet. Straying off course for the bug, though...