[Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

[xbolva00]

Summary

Solves https://bugs.llvm.org/show_bug.cgi?id=45634
Be more agressive than GCC with -Wdeprecated-copy. Also provide -W(no-)deprecated-copy-user-provided-copy/dtor options to on/off this behaviour.

[• Quuxplusone]

clang/include/clang/Basic/DiagnosticGroups.td

160

If we're going to provide these options at all, I think it would be more grammatical to call them -Wdeprecated-copy-with-deleted-copy and -Wdeprecated-copy-with-deleted-dtor.

The existing code is already confusing by talking about a "copy dtor" as if that's a thing; I think it makes it even worse to talk about "deprecated copy user provided," since the actually deprecated thing is the implicitly generated copy.

I get that we're trying to be terse, and also somewhat hierarchical in putting all the -Wdeprecated-copy-related warnings under -Wdeprecated-copy-foo-bar; but the current names are too far into the ungrammatical realm for me personally.

clang/include/clang/Basic/DiagnosticSemaKinds.td

571

This is the first time I've ever seen this idiom. As a person who often greps the wording of an error message to find out where it comes from, I would very strongly prefer to see the actual string literal here. But if this is established practice, then okay.

clang/lib/Sema/SemaDeclCXX.cpp

13935–13948

I wonder if this would be easier to read as

if (UserDeclaredOperation) {
    bool UDOIsUserProvided = UserDeclaredOperation->isUserProvided();
    bool UDOIsDestructor = isa<CXXDestructorDecl>(UserDeclaredOperation);
    bool IsCopyAssignment = !isa<CXXConstructorDecl>(CopyOp);
    unsigned DiagID =
        ( UDOIsUserProvided &&  UDOIsDestructor) ? diag::warn_deprecated_copy_dtor_operation_user_provided :
        ( UDOIsUserProvided && !UDOIsDestructor) ? diag::warn_deprecated_copy_operation_user_provided :
        (!UDOIsUserProvided &&  UDOIsDestructor) ? diag::warn_deprecated_copy_dtor_operation :
        diag::warn_deprecated_copy_operation;
    S.Diag(UserDeclaredOperation->getLocation(), DiagID)
        << RD << /*copy assignment*/ IsCopyAssignment;

clang/test/SemaCXX/deprecated-copy.cpp

5

I'm fairly confident this should just be two different test files. Also, if a test file has an un-ifdeffed // expected-no-diagnostics plus an un-ifdeffed // expected-note ..., which one wins?

[xbolva00]

Thanks for initial comments.

clang/include/clang/Basic/DiagnosticGroups.td

160

Yeah, better names are welcome :)

clang/include/clang/Basic/DiagnosticSemaKinds.td

571

I think this is established way how to duplicated warning text strings.

clang/lib/Sema/SemaDeclCXX.cpp

13935–13948

I have been thinking about similar form, so +1.

clang/test/SemaCXX/deprecated-copy.cpp

5

ifdef is here

and ifNdef is below :)

[xbolva00]

clang/test/SemaCXX/deprecated-copy.cpp

5

and DEPRECATED_COPY_DTOR is in own "code block"

[• Quuxplusone]

clang/test/SemaCXX/deprecated-copy.cpp

5

Ah, you're right, I had missed that line 18 was still controlled by the #ifdef DEPRECATED_COPY_DTOR condition.
I still think this should be three(!) different files. Line 2 doesn't even look right to me: shouldn't it be -Wdeprecated-copy -Wno-deprecated-copy-user-provided?

I just did a git grep 'RUN:' | grep '[-]Wno-' | grep -v '[-]W[^n]' and found a massive number of tests that put -Wno-foo on the command line without putting -Wbar anywhere on the same line; I suspect many of these are bugs.

[xbolva00]

Updated. Thanks for review.

[xbolva00]

More tests

[• Quuxplusone]

clang/include/clang/Basic/DiagnosticGroups.td

160

Do the current names match existing practice in GCC or anything?
I continue to opine that these options are poorly named. My best suggestion is
deprecated-copy, deprecated-copy-with-dtor, deprecated-copy-with-deleted-copy, deprecated-copy-with-deleted-dtor — operating on the (wrong?) assumption that absolutely the only difference between "user-declared" and "user-provided" corresponds to "user-declared as deleted."

Even if everything else remains the same, the internal identifier warn_deprecated_copy_dtor_operation_user_provided should certainly be warn_deprecated_copy_operation_dtor_user_provided — the phrase that goes together is "copy operation", not "dtor operation".

Your "deprecated-dtor-user-provided.cpp" passes -Wdeprecated-copy-dtor-user-provided, but the message text talks about "user-declared." Shouldn't the spelling of the option reflect the wording of the message text? This isn't important from the POV of someone who's just going to look at the message text and fix their code, but it's important from the POV of someone who's going to use -Wno- and wants to know exactly which bad situations they're ignoring. (Also, the name of the file should match the spelling of the option.)

clang/lib/Sema/SemaDeclCXX.cpp

13948

The /* copy assignment */ comment is probably no longer useful.

clang/test/SemaCXX/deprecated-copy.cpp

5

I've lost track of what I was trying to say here and whether I was right ;) but I assume we can consider this comment thread "resolved"?

clang/test/SemaCXX/deprecated-dtor-user-provided.cpp

6

Do you have a test case for when an operation is defaulted as deleted?
I actually don't understand Clang's/GCC's current behavior in this area.
https://godbolt.org/z/9h3qqP
Of these three situations, do any of them rely on deprecated behavior?
(And vice versa, what if we delete the copy constructor and try to use the implicit copy-assignment operator?)

clang/test/SemaCXX/deprecated.cpp

108

Nit: I would prefer to see the declarations of d1 and d2 on individual lines, both for style and to prove that d1's declaration triggers no warnings.

[xbolva00]

Updated, addressed review feedback.

[xbolva00]

Ping @Quuxplusone

clang/include/clang/Basic/DiagnosticGroups.td

160

Yeah, deprecated-copy and deprecated-copy-dtor is already defined by GCC.

I think deprecated-copy-user-provided-copy and deprecated-copy-user-provided-dtor could be good solution here, it is also quite easy to follow implementation of the checking logic in Sema with these names.

[• Quuxplusone]

The new individual test files are awesome. :)
The name of the -W options still aren't perfect IMHO, but maybe it will never be perfect, and meanwhile everything else looks great.

clang/include/clang/Basic/DiagnosticGroups.td

160

@xbolva00: I suggested -Wdeprecated-copy-with-user-provided-copy; you've got -Wdeprecated-copy-user-provided-copy. Could I persuade you to use the -with- versions?
I suppose where it really matters is -Wdeprecated-copy-dtor (what is a "copy destructor"?), but that's for GCC compatibility, so we can't change it. Right?

clang/test/SemaCXX/deprecated-copy-dtor.cpp

3

I think it's extremely unfortunate that -Wdeprecated-copy-dtor is not a subset of -Wdeprecated-copy. But again, GCC-compatibility binds our hands here AFAIK. https://godbolt.org/z/3r3ddvTfG

[xbolva00]

clang/include/clang/Basic/DiagnosticGroups.td

160

I can use with “-with” and create alias for -Wdeprecated-copy-dtor to keep gcc compatibility :)

[xbolva00]

Use suggested flag names.

[uabelho]

Hi,

With this commit I get failures in the following testcases when building check-runtimes on trunk:

Failed Tests (29):
  libc++ :: libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
  libc++ :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp
  libc++ :: std/containers/sequences/vector.bool/assign_copy.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/assign_move.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/copy.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/copy_alloc.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/emplace.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/erase_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/erase_iter_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_initializer_list.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_size_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/iterators.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/move.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/move_alloc.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/resize_size.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/resize_size_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/size.pass.cpp
  libc++ :: std/containers/sequences/vector/vector.cons/deduct.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/left_shift.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/op_eq_eq.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/right_shift.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_and.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_not.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_or.pass.cpp

They all get -Wdeprecated-copy warnigns and then with -Werror they fail.
Is this something you've seen or considered?

[xbolva00]

In D79714#2711871, @uabelho wrote:

Hi,

With this commit I get failures in the following testcases when building check-runtimes on trunk:

Failed Tests (29):
  libc++ :: libcxx/debug/containers/db_sequence_container_iterators.pass.cpp
  libc++ :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp
  libc++ :: std/containers/sequences/vector.bool/assign_copy.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/assign_move.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/copy.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/copy_alloc.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/emplace.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/erase_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/erase_iter_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_initializer_list.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_iter_iter.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_size_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/insert_iter_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/iterators.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/move.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/move_alloc.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/resize_size.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/resize_size_value.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp
  libc++ :: std/containers/sequences/vector.bool/size.pass.cpp
  libc++ :: std/containers/sequences/vector/vector.cons/deduct.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/left_shift.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/op_eq_eq.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/right_shift.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_and.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_not.pass.cpp
  libc++ :: std/utilities/template.bitset/bitset.operators/op_or.pass.cpp

They all get -Wdeprecated-copy warnigns and then with -Werror they fail.
Is this something you've seen or considered?

Can you try this fix?

diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index ddf277dea246..abf712e78a61 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -12,6 +12,7 @@ from libcxx.test.features import _isMSVC
 _warningFlags = [
   '-Werror',
   '-Wall',
+  '-Wno-deprecated-copy',
   '-Wextra',
   '-Wshadow',
   '-Wundef',

[uabelho]

In D79714#2711898, @xbolva00 wrote:

Can you try this fix?

diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index ddf277dea246..abf712e78a61 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -12,6 +12,7 @@ from libcxx.test.features import _isMSVC
 _warningFlags = [
   '-Werror',
   '-Wall',
+  '-Wno-deprecated-copy',
   '-Wextra',
   '-Wshadow',
   '-Wundef',

The above doesn't help but if I add it in the list after -Wextra the warning goes away. Can you submit something like that?
Thanks!

[xbolva00]

In D79714#2712393, @uabelho wrote:

In D79714#2711898, @xbolva00 wrote:

Can you try this fix?

diff --git a/libcxx/utils/libcxx/test/params.py b/libcxx/utils/libcxx/test/params.py
index ddf277dea246..abf712e78a61 100644
--- a/libcxx/utils/libcxx/test/params.py
+++ b/libcxx/utils/libcxx/test/params.py
@@ -12,6 +12,7 @@ from libcxx.test.features import _isMSVC
 _warningFlags = [
   '-Werror',
   '-Wall',
+  '-Wno-deprecated-copy',
   '-Wextra',
   '-Wshadow',
   '-Wundef',

The above doesn't help but if I add it in the list after -Wextra the warning goes away. Can you submit something like that?
Thanks!

Yes, I can. Thanks

[nemanjai]

This breaks Clang's ability to bootstrap LLVM with -Werror as evidenced by the buildbot failure https://lab.llvm.org/buildbot/#/builders/36/builds/7587.
Please revert or fix the googletest code that trips this warning. Pertinent file:

FAILED: unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o 
/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/clang++  -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iunittests/Support -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support -Iinclude -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/include -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o -MF unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o.d -o unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o -c /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support/FormatVariadicTest.cpp
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support/FormatVariadicTest.cpp:12:
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:62:
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest-param-test.h:193:
/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/internal/gtest-param-util-generated.h:107:8: error: definition of implicit copy constructor for 'ValueArray2<bool, bool>' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
  void operator=(const ValueArray2& other) = delete;
       ^

[xbolva00]

In D79714#2713311, @nemanjai wrote:

This breaks Clang's ability to bootstrap LLVM with -Werror as evidenced by the buildbot failure https://lab.llvm.org/buildbot/#/builders/36/builds/7587.
Please revert or fix the googletest code that trips this warning. Pertinent file:

FAILED: unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o 
/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/clang++  -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iunittests/Support -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support -Iinclude -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/include -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include -I/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googlemock/include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o -MF unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o.d -o unittests/Support/CMakeFiles/SupportTests.dir/FormatVariadicTest.cpp.o -c /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support/FormatVariadicTest.cpp
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/Support/FormatVariadicTest.cpp:12:
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:62:
In file included from /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest-param-test.h:193:
/home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/utils/unittest/googletest/include/gtest/internal/gtest-param-util-generated.h:107:8: error: definition of implicit copy constructor for 'ValueArray2<bool, bool>' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
  void operator=(const ValueArray2& other) = delete;
       ^

I pushed a fix to just disable this warning for googlemock/gtest.

[nemanjai]

In D79714#2713610, @xbolva00 wrote:

I pushed a fix to just disable this warning for googlemock/gtest.

There are still more of these. I will apply the same fix to utils/unittest/googlemock/include/gmock/gmock.h unless you think there's a problem with doing that.

[xbolva00]

In D79714#2713921, @nemanjai wrote:

In D79714#2713610, @xbolva00 wrote:

I pushed a fix to just disable this warning for googlemock/gtest.

There are still more of these. I will apply the same fix to utils/unittest/googlemock/include/gmock/gmock.h unless you think there's a problem with doing that.

Yeah, please go ahead :)

[nemanjai]

The fixes in https://reviews.llvm.org/D101214 are sufficient to bring the -Werror bootstrap back to green. Please review the patch.

[uabelho]

In D79714#2713610, @xbolva00 wrote:

I pushed a fix to just disable this warning for googlemock/gtest.

Unfortunately that fix breaks compiling llvm with clang8. I commented in
https://reviews.llvm.org/rG9658d045926545e62cc3f963fe611d7c5d0c9d98

[Abpostelnicu]

I'm seeing here something very strange, if the user provided copy assignment operator is provided but is = delete and the copy constructor is default this warning will still trigger. Is this something expected?
I'm referring at this issue from the mozilla code-base.

[• Quuxplusone]

In D79714#2735455, @Abpostelnicu wrote:

I'm seeing here something very strange, if the user provided copy assignment operator is provided but is = delete and the copy constructor is default this warning will still trigger. Is this something expected?
I'm referring at this issue from the mozilla code-base.

Yes, that implicit default is deprecated.
If you want your copy constructor to be defaulted, even in the presence of deleted SMFs, you should explicitly =default it. Here's an example: https://godbolt.org/z/GaGn1638E

clang/include/clang/Basic/DiagnosticGroups.td

 def FinalDtorNonFinalClass : DiagGroup<"final-dtor-non-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;
 
 def DeprecatedArrayCompare : DiagGroup<"deprecated-array-compare">;
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
-def DeprecatedCopy : DiagGroup<"deprecated-copy">;
-def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor">;
+def DeprecatedCopyUserProvided : DiagGroup<"deprecated-copy-user-provided">;
+def DeprecatedCopyDtorUserProvided : DiagGroup<"deprecated-copy-dtor-user-provided">;
+def DeprecatedCopy : DiagGroup<"deprecated-copy", [DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", [DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
 def UnguardedAvailability : DiagGroup<"unguarded-availability",
                                       [UnguardedAvailabilityNew]>;
 // partial-availability is an alias of unguarded-availability.
 def : DiagGroup<"partial-availability", [UnguardedAvailability]>;
 def DeprecatedDynamicExceptionSpec

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_deprecated_copy_operation : Warning<
   "definition of implicit copy %select{constructor|assignment operator}1 "
   "for %0 is deprecated because it has a user-declared copy "
   "%select{assignment operator|constructor}1">,
   InGroup<DeprecatedCopy>, DefaultIgnore;
 def warn_deprecated_copy_dtor_operation : Warning<
   "definition of implicit copy %select{constructor|assignment operator}1 "
   "for %0 is deprecated because it has a user-declared destructor">,
   InGroup<DeprecatedCopyDtor>, DefaultIgnore;
+def warn_deprecated_copy_operation_user_provided : Warning<
+  warn_deprecated_copy_operation.Text>,
+  InGroup<DeprecatedCopyUserProvided>, DefaultIgnore;
+def warn_deprecated_copy_dtor_operation_user_provided : Warning<
+  warn_deprecated_copy_dtor_operation.Text>,
+  InGroup<DeprecatedCopyDtorUserProvided>, DefaultIgnore;
 def warn_cxx17_compat_exception_spec_in_signature : Warning<
   "mangled name of %0 will change in C++17 due to non-throwing exception "
   "specification in function signature">, InGroup<CXX17CompatMangling>;
 
 def warn_global_constructor : Warning<
   "declaration requires a global constructor">,
   InGroup<GlobalConstructors>, DefaultIgnore;
 def warn_global_destructor : Warning<

clang/lib/Sema/SemaDeclCXX.cpp

for (auto *I : RD->methods()) {
       if (I->isCopyAssignmentOperator()) {
         UserDeclaredOperation = I;
         break;
       }
     }
     assert(UserDeclaredOperation);
   }
 
-  if (UserDeclaredOperation && UserDeclaredOperation->isUserProvided()) {
-    S.Diag(UserDeclaredOperation->getLocation(),
-           isa<CXXDestructorDecl>(UserDeclaredOperation)
-               ? diag::warn_deprecated_copy_dtor_operation
-               : diag::warn_deprecated_copy_operation)
-        << RD << /*copy assignment*/ !isa<CXXConstructorDecl>(CopyOp);
+  if (UserDeclaredOperation) {
+    bool UDOIsUserProvided = UserDeclaredOperation->isUserProvided();
+    bool UDOIsDestructor = isa<CXXDestructorDecl>(UserDeclaredOperation);
+    bool IsCopyAssignment = !isa<CXXConstructorDecl>(CopyOp);
+    unsigned DiagID =
+        (UDOIsUserProvided && UDOIsDestructor)
+            ? diag::warn_deprecated_copy_dtor_operation_user_provided
+        : (UDOIsUserProvided && !UDOIsDestructor)
+            ? diag::warn_deprecated_copy_operation_user_provided
+        : (!UDOIsUserProvided && UDOIsDestructor)
+            : diag::warn_deprecated_copy_operation;
+    S.Diag(UserDeclaredOperation->getLocation(), DiagID)
+        << RD << /*copy assignment*/ IsCopyAssignment;
   }
 }
 
 void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
                                         CXXMethodDecl *CopyAssignOperator) {
   assert((CopyAssignOperator->isDefaulted() &&
           CopyAssignOperator->isOverloadedOperator() &&
           CopyAssignOperator->getOverloadedOperator() == OO_Equal &&

clang/test/SemaCXX/deprecated-copy-dtor.cpp

+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-dtor -verify
+
+// definitions for std::move
+namespace std {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T &> { typedef T type; };
+template <class T> struct remove_reference<T &&> { typedef T type; };
+
+template <class T> typename remove_reference<T>::type &&move(T &&t);
+} // namespace std
+
+class ITest {
+public:
+  virtual ~ITest() = default; // expected-warning {{definition of implicit copy constructor for 'ITest' is deprecated because it has a user-declared destructor}}
+  virtual void testMethod() = 0;
+};
+
+class TestImpl : public ITest // expected-note {{in implicit copy constructor for 'ITest' first required here}}
+{
+public:
+  virtual void testMethod() override{};
+};
+
+int main() {
+  TestImpl impl1;
+  TestImpl impl2 = std::move(impl1); // expected-note {{in implicit move constructor for 'TestImpl' first required here}}
+  return 0;
+}
 No newline at end of file

clang/test/SemaCXX/deprecated-copy.cpp

+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
 // RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
-// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-dtor -DDEPRECATED_COPY_DTOR -verify
-// RUN: %clang_cc1 -std=c++11 %s -Wextra -verify
 
-#ifdef DEPRECATED_COPY_DTOR
-struct A {
-  int *ptr;
-  ~A() { delete ptr; } // expected-warning {{definition of implicit copy constructor for 'A' is deprecated because it has a user-declared destructor}}
+struct S {
+    int i;
+    S& operator=(const S&) = delete; // expected-warning {{definition of implicit copy constructor for 'S' is deprecated because it has a user-declared copy assignment operator}}
 };
 
-void foo() {
-  A a{};
-  A b = a; // expected-note {{implicit copy constructor for 'A' first required here}}
-}
-#else
-struct B {
-  B &operator=(const B &); // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
-};
-
-void bar() {
-  B b1, b2(b1); // expected-note {{implicit copy constructor for 'B' first required here}}
-}
-#endif
+S test(const S& s) { return S(s); } // expected-note {{implicit copy constructor for 'S' first required here}}

clang/test/SemaCXX/deprecated-dtor-user-provided.cpp

+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-dtor-user-provided -verify
+
+struct A {
+  int *ptr;
+  ~A(); // expected-warning {{definition of implicit copy constructor for 'A' is deprecated because it has a user-declared destructor}}
+};
+
+void foo() {
+  A a{};
+  A b = a; // expected-note {{implicit copy constructor for 'A' first required here}}
+}

clang/test/SemaCXX/deprecated-user-provided.cpp

+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-user-provided -verify
+
+struct A {
+  A &operator=(const A &); // expected-warning {{definition of implicit copy constructor for 'A' is deprecated because it has a user-declared copy assignment operator}}
+};
+
+void foo() {
+  A a1, a2(a1); // expected-note {{implicit copy constructor for 'A' first required here}}
+}

clang/test/SemaCXX/deprecated.cpp

struct Dtor {
     ~Dtor();
     // expected-warning@-1 {{definition of implicit copy constructor for 'Dtor' is deprecated because it has a user-declared destructor}}
     // expected-warning@-2 {{definition of implicit copy assignment operator for 'Dtor' is deprecated because it has a user-declared destructor}}
   };
   Dtor c1, c2(c1); // expected-note {{implicit copy constructor for 'DeprecatedCopy::Dtor' first required here}}
   void g() { c1 = c2; } // expected-note {{implicit copy assignment operator for 'DeprecatedCopy::Dtor' first required here}}
 
   struct DefaultedDtor {
-    ~DefaultedDtor() = default;
-  };
-  DefaultedDtor d1, d2(d1);
-  void h() { d1 = d2; }
+    ~DefaultedDtor() = default; // expected-warning {{definition of implicit copy constructor for 'DefaultedDtor' is deprecated because it has a user-declared destructor}}
+  };                            // expected-warning@-1 {{definition of implicit copy assignment operator for 'DefaultedDtor' is deprecated because it has a user-declared destructor}}
+  DefaultedDtor d1, d2(d1);     // expected-note {{in implicit copy constructor for 'DeprecatedCopy::DefaultedDtor' first required here}}
+  void h() { d1 = d2; }         // expected-note {{in implicit copy assignment operator for 'DeprecatedCopy::DefaultedDtor' first required here}}
 }
 #endif
 
 struct X {
   friend int operator,(X, X);
   void operator[](int);
 };
 void array_index_comma() {