[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Re-re-reading 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0619r4.html#3.2, the 
recommendation is "We will consider again for C++23". So I think this should be 
enabled with -std=c++20 at the earliest, since that's where the idea is from. 
But even there it's uncertain if it's going to happen. So I'd say put it in 
-pedantic, and possibly enable it by default with -std=c++20.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

No other compiler ? GCC 9 introduced it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Ah, Hans points out that the paper says that it's been deprecated in C++11, so 
this should either be on by default (and mention "C++11" somewhere), or it 
should be in -pedantic given that no other compiler diagnoses this and given 
that it fires heavily on existing code (including LLVM).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Why do we emit this before C++20? It's only deprecated in C++20 onwards, no? 
Shouldn't this be default-enabled based off language version instead of being 
in -Wextra? No other language deprecation warning so far (register etc) has 
worked like this warning does.

(Also, this being in -Wextra has confused users for us: We added 
-Wno-deprecated-copy somewhere in our central build flags, and then some target 
added -Wextra later on and was confused that the -Wno-deprecated-copy flag 
didn't have an effect – but that's a minor point.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0619r4.html#3.2

So C++20 or C++23.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D70342#1758832 , @xbolva00 wrote:

> Add copy ctor/op= manually.


Does the warning mean that the implicitly defined copy ctor is not going to 
work in some later version of the language?

I'm just trying to understand what it's warning about. If it's pointing out 
bugs in our code, we should fix them obviously.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Add copy ctor/op= manually.

If you have a lot of warnings, you can use -Wno-deprecated-copy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

We're now getting a ton of warnings about this in Chromium. 
(https://crbug.com/1028110)

And I'm not really sure what the warning wants us to do?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9353c5dd0664: [Diagnostics] Put deprecated copy 
warnings into -Wdeprecated-copy (authored by xbolva00).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/deprecated-copy.cpp


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// 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}}
+};
+
+void foo() {
+  A a{};
+  A b = a; // expected-note {{implicit copy constructor for 'A' first required 
here}}
+}
+#else
+struct B {
+  B =(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
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -12434,9 +12434,10 @@
 
   if (UserDeclaredOperation) {
 S.Diag(UserDeclaredOperation->getLocation(),
- diag::warn_deprecated_copy_operation)
-  << RD << /*copy assignment*/!isa(CopyOp)
-  << /*destructor*/isa(UserDeclaredOperation);
+   isa(UserDeclaredOperation)
+   ? diag::warn_deprecated_copy_dtor_operation
+   : diag::warn_deprecated_copy_operation)
+<< RD << /*copy assignment*/ !isa(CopyOp);
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -551,9 +551,13 @@
   "use using declarations instead">;
 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 "
-  "%select{copy %select{assignment operator|constructor}1|destructor}2">,
-  InGroup, DefaultIgnore;
+  "for %0 is deprecated because it has a user-declared copy "
+  "%select{assignment operator|constructor}1">,
+  InGroup, 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, 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;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -128,6 +128,8 @@
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
+def DeprecatedCopy : DiagGroup<"deprecated-copy">;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
@@ -147,6 +149,8 @@
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAttributes,
   DeprecatedCommaSubscript,
+  DeprecatedCopy,
+  DeprecatedCopyDtor,
   DeprecatedDeclarations,
   DeprecatedDynamicExceptionSpec,
   DeprecatedIncrementBool,
@@ -812,6 +816,7 @@
   ]>;
 
 def Extra : DiagGroup<"extra", [
+DeprecatedCopy,
 MissingFieldInitializers,
 IgnoredQualifiers,
 InitializerOverrides,


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+// RUN: 

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me - can handle anything else from @rsmith in post-commit review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 229660.
xbolva00 added a comment.

Newline fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/deprecated-copy.cpp


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// 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}}
+};
+
+void foo() {
+  A a{};
+  A b = a; // expected-note {{implicit copy constructor for 'A' first required 
here}}
+}
+#else
+struct B {
+  B =(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
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -12434,9 +12434,10 @@
 
   if (UserDeclaredOperation) {
 S.Diag(UserDeclaredOperation->getLocation(),
- diag::warn_deprecated_copy_operation)
-  << RD << /*copy assignment*/!isa(CopyOp)
-  << /*destructor*/isa(UserDeclaredOperation);
+   isa(UserDeclaredOperation)
+   ? diag::warn_deprecated_copy_dtor_operation
+   : diag::warn_deprecated_copy_operation)
+<< RD << /*copy assignment*/ !isa(CopyOp);
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -551,9 +551,13 @@
   "use using declarations instead">;
 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 "
-  "%select{copy %select{assignment operator|constructor}1|destructor}2">,
-  InGroup, DefaultIgnore;
+  "for %0 is deprecated because it has a user-declared copy "
+  "%select{assignment operator|constructor}1">,
+  InGroup, 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, 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;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -128,6 +128,8 @@
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
+def DeprecatedCopy : DiagGroup<"deprecated-copy">;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
@@ -147,6 +149,8 @@
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAttributes,
   DeprecatedCommaSubscript,
+  DeprecatedCopy,
+  DeprecatedCopyDtor,
   DeprecatedDeclarations,
   DeprecatedDynamicExceptionSpec,
   DeprecatedIncrementBool,
@@ -812,6 +816,7 @@
   ]>;
 
 def Extra : DiagGroup<"extra", [
+DeprecatedCopy,
 MissingFieldInitializers,
 IgnoredQualifiers,
 InitializerOverrides,


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// 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 

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D70342#1748555 , @rsmith wrote:

> Generally OK, but I'm concerned about including this in `-Wextra`. In 
> particular, the deprecation of copy/move operations when a destructor is 
> explicitly declared has experimentally been found to have a high 
> false-positive rate. Does GCC include this in `-Wextra`? (Its documentation 
> doesn't seem to say so.)


Yes, right. Fixed. New revision should be OK.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 229658.
xbolva00 added a comment.

Added -Wdeprecated-copy-dtor (not part of -Wextra). Now we should be fully 
compatible with GCC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/deprecated-copy.cpp


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// 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}}
+};
+
+void foo() {
+  A a{};
+  A b = a; // expected-note {{implicit copy constructor for 'A' first required 
here}}
+}
+#else
+struct B {
+  B =(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
\ No newline at end of file
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -12434,9 +12434,10 @@
 
   if (UserDeclaredOperation) {
 S.Diag(UserDeclaredOperation->getLocation(),
- diag::warn_deprecated_copy_operation)
-  << RD << /*copy assignment*/!isa(CopyOp)
-  << /*destructor*/isa(UserDeclaredOperation);
+   isa(UserDeclaredOperation)
+   ? diag::warn_deprecated_copy_dtor_operation
+   : diag::warn_deprecated_copy_operation)
+<< RD << /*copy assignment*/ !isa(CopyOp);
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -551,9 +551,13 @@
   "use using declarations instead">;
 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 "
-  "%select{copy %select{assignment operator|constructor}1|destructor}2">,
-  InGroup, DefaultIgnore;
+  "for %0 is deprecated because it has a user-declared copy "
+  "%select{assignment operator|constructor}1">,
+  InGroup, 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, 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;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -128,6 +128,8 @@
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
+def DeprecatedCopy : DiagGroup<"deprecated-copy">;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
@@ -147,6 +149,8 @@
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAttributes,
   DeprecatedCommaSubscript,
+  DeprecatedCopy,
+  DeprecatedCopyDtor,
   DeprecatedDeclarations,
   DeprecatedDynamicExceptionSpec,
   DeprecatedIncrementBool,
@@ -812,6 +816,7 @@
   ]>;
 
 def Extra : DiagGroup<"extra", [
+DeprecatedCopy,
 MissingFieldInitializers,
 IgnoredQualifiers,
 InitializerOverrides,


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+// RUN: 

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Generally OK, but I'm concerned about including this in `-Wextra`. In 
particular, the deprecation of copy/move operations when a destructor is 
explicitly declared has experimentally been found to have a high false-positive 
rate. Does GCC include this in `-Wextra`? (Its documentation doesn't seem to 
say so.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70342/new/

https://reviews.llvm.org/D70342



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70342: [Diagnostics] Put "deprecated copy" warnings into -Wdeprecated-copy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
xbolva00 added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC 9 added -Wdeprecated-copy (as part of -Wextra). This diagnostic is already 
implemented in Clang too, just hidden under -Wdeprecated (not on by default).
This patch adds -Wdeprecated-copy and makes it compatible with GCC 9+.
This diagnostic is heavily tested in deprecated.cpp, so I added simple tests 
just to check we warn when new flag/-Wextra is enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70342

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/deprecated-copy.cpp


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++14 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++17 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++2a %s -Wdeprecated-copy -verify
+
+// RUN: %clang_cc1 -std=c++11 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++14 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++17 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++2a %s -Wextra -verify
+
+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 B {
+  B =(const B &); // expected-warning {{definition of implicit copy 
constructor for 'B' 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}}
+  B b1, b2(b1); // expected-note {{implicit copy constructor for 'B' first 
required here}}
+}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -553,7 +553,7 @@
   "definition of implicit copy %select{constructor|assignment operator}1 "
   "for %0 is deprecated because it has a user-declared "
   "%select{copy %select{assignment operator|constructor}1|destructor}2">,
-  InGroup, DefaultIgnore;
+  InGroup, 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;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -128,6 +128,7 @@
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedCommaSubscript : DiagGroup<"deprecated-comma-subscript">;
+def DeprecatedCopy : DiagGroup<"deprecated-copy">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
 def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
@@ -147,6 +148,7 @@
 // FIXME: Why is DeprecatedImplementations not in this group?
 def Deprecated : DiagGroup<"deprecated", [DeprecatedAttributes,
   DeprecatedCommaSubscript,
+  DeprecatedCopy,
   DeprecatedDeclarations,
   DeprecatedDynamicExceptionSpec,
   DeprecatedIncrementBool,
@@ -812,6 +814,7 @@
   ]>;
 
 def Extra : DiagGroup<"extra", [
+DeprecatedCopy,
 MissingFieldInitializers,
 IgnoredQualifiers,
 InitializerOverrides,


Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++14 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++17 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++2a %s -Wdeprecated-copy -verify
+
+// RUN: %clang_cc1 -std=c++11 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++14 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++17 %s -Wextra -verify
+// RUN: %clang_cc1 -std=c++2a %s -Wextra -verify
+
+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 B {
+  B =(const B &); // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+};
+
+void foo() {
+  A a{};
+  A b = a;  // expected-note {{implicit