[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 abandoned this revision. xbolva00 added a comment. https://reviews.llvm.org/D70342 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68185/new/ https://reviews.llvm.org/D68185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 added a comment. Yes, i will prepare a new patch and add him as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68185/new/ https://reviews.llvm.org/D68185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

dblaikie added a comment. In D68185#1742809 , @xbolva00 wrote: > Ah, Clang has it under -Wdeprecated. > https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated. > > But combo -Wall -Wextra does not enable this warning, sadly. > > This should be

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 added a comment. Ah, Clang has it under -Wdeprecated. https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated. But combo -Wall -Wextra does not enable this warning, sadly. This should be extracted to -Wdeprecated-copy and put into -Wextra. Seems fine for you? CHANGES

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

dblaikie added a comment. Yeah, I'm OK with warning on the deprecated cases (especially for symmetry with GCC's warning) - not sure why that didn't occur to me/why I didn't mention it in my previous comment... Deprecation wording is, in C++17, 15.8.1 [class.copy.ctor] p6, and 15.8.2

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 added a comment. Should we reconsider this as a clang warning? I found that the newest GCC can diagnose it. GCC 9+: -Wdeprecated-copy, implied by -Wextra, warns about the C++11 deprecation of implicitly declared copy constructor and assignment operator if one of them is

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

dblaikie added a comment. +1 for clang-tidy. GCC's closest to this is -Weffc++ which limits the rule of 3 warning to only "classes that have dynamic memory allocation" (though I'm not sure exactly what conditions it uses to decide that - it doesn't warn on anything in the test cases provided

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 abandoned this revision. xbolva00 added a comment. Yeah, if we want to check rule of 3, clang-tidy is better choice. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68185/new/ https://reviews.llvm.org/D68185 ___ cfe-commits mailing

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

thakis added a comment. Since https://en.cppreference.com/w/cpp/language/rule_of_three says "almost always" (note "almost") I'd say this probably belongs more in clang-tidy territory – I'm guessing false positive rate is very high for this. Can you collect some true / false positive statistics

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 added a comment. >> if I provide a destructor but implicitly default my copy operations, isn't >> that just as bad, Rule-of-Three-wise? Yeah, I am wondering about this too. whether we should check the whole Rule-of-Three. Suggestions from other reviewers? CHANGES SINCE LAST ACTION

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

Quuxplusone added a comment. Please add test cases showing the intended behavior for - when the copy constructor is explicitly defaulted but the copy assignment operator is {implicitly defaulted, implicitly deleted, user-provided} - when the copy assignment operator is explicitly defaulted but

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 created this revision. xbolva00 added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. xbolva00 updated this revision to Diff 222301. Clang version of https://www.viva64.com/en/w/v690/ Also requested by Chromium developers:

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

xbolva00 updated this revision to Diff 222301. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68185/new/ https://reviews.llvm.org/D68185 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclCXX.cpp test/SemaCXX/warn-custom-copy.cpp Index: