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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Andi via Phabricator via cfe-commits]

Abpostelnicu added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 added a comment.

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 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 added a comment.

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' 
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

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' 
is deprecated because it has a user-declared copy assignment operator 
[-Werror,-Wdeprecated-copy]
void operator=(const ValueArray2& other) = delete;
 ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 added a comment.

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',


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

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

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGabf3ca61e323: [Diagnostics] Restore -Wdeprecated warning 
when user-declared copy assignment… (authored by xbolva00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -83,30 +83,31 @@
 #if __cplusplus >= 201103L
 namespace DeprecatedCopy {
   struct Assign {
-Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-declared copy assignment operator}}
+Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-provided copy assignment operator}}
   };
   Assign a1, a2(a1); // expected-note {{implicit copy constructor for 'DeprecatedCopy::Assign' first required here}}
 
   struct Ctor {
 Ctor();
-Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-declared copy constructor}}
+Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-provided copy constructor}}
   };
   Ctor b1, b2;
   void f() { b1 = b2; } // expected-note {{implicit copy assignment operator for 'DeprecatedCopy::Ctor' first required here}}
 
   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}}
+// expected-warning@-1 {{definition of implicit copy constructor for 'Dtor' is deprecated because it has a user-provided destructor}}
+// expected-warning@-2 {{definition of implicit copy assignment operator for 'Dtor' is deprecated because it has a user-provided 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;
+  DefaultedDtor 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
 
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,23 +1,26 @@
+// 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}}
+A& operator=(const A&) = default; // expected-warning {{definition of implicit copy constructor for 'A' 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 =(const B &); // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+B& operator=(const B&) = delete; // expected-warning {{definition of implicit copy constructor for 'B' 

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

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

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

Use suggested flag names.


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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -83,30 +83,31 @@
 #if __cplusplus >= 201103L
 namespace DeprecatedCopy {
   struct Assign {
-Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-declared copy assignment operator}}
+Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-provided copy assignment operator}}
   };
   Assign a1, a2(a1); // expected-note {{implicit copy constructor for 'DeprecatedCopy::Assign' first required here}}
 
   struct Ctor {
 Ctor();
-Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-declared copy constructor}}
+Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-provided copy constructor}}
   };
   Ctor b1, b2;
   void f() { b1 = b2; } // expected-note {{implicit copy assignment operator for 'DeprecatedCopy::Ctor' first required here}}
 
   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}}
+// expected-warning@-1 {{definition of implicit copy constructor for 'Dtor' is deprecated because it has a user-provided destructor}}
+// expected-warning@-2 {{definition of implicit copy assignment operator for 'Dtor' is deprecated because it has a user-provided 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;
+  DefaultedDtor 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
 
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,23 +1,26 @@
+// 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}}
+A& operator=(const A&) = default; // expected-warning {{definition of implicit copy constructor for 'A' 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 =(const B &); // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+B& operator=(const B&) = delete; // 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}}
+void test() {
+  A 

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

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

xbolva00 added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

Quuxplusone wrote:
> xbolva00 wrote:
> > Quuxplusone wrote:
> > > xbolva00 wrote:
> > > > Quuxplusone wrote:
> > > > > 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.
> > > > Yeah, better names are welcome :)
> > > 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.)
> > 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.
> @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?
I can use with “-with” and create alias for -Wdeprecated-copy-dtor to keep gcc 
compatibility :)


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

https://reviews.llvm.org/D79714

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

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

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

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.




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

xbolva00 wrote:
> Quuxplusone wrote:
> > xbolva00 wrote:
> > > Quuxplusone wrote:
> > > > 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.
> > > Yeah, better names are welcome :)
> > 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.)
> 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.
@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?



Comment at: clang/test/SemaCXX/deprecated-copy-dtor.cpp:2
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-dtor -verify
+

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


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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 added a comment.

Ping @arthur.j.odwyer




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

Quuxplusone wrote:
> xbolva00 wrote:
> > Quuxplusone wrote:
> > > 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.
> > Yeah, better names are welcome :)
> 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.)
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.


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

https://reviews.llvm.org/D79714

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

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

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

xbolva00 updated this revision to Diff 338336.

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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -83,30 +83,31 @@
 #if __cplusplus >= 201103L
 namespace DeprecatedCopy {
   struct Assign {
-Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-declared copy assignment operator}}
+Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-provided copy assignment operator}}
   };
   Assign a1, a2(a1); // expected-note {{implicit copy constructor for 'DeprecatedCopy::Assign' first required here}}
 
   struct Ctor {
 Ctor();
-Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-declared copy constructor}}
+Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-provided copy constructor}}
   };
   Ctor b1, b2;
   void f() { b1 = b2; } // expected-note {{implicit copy assignment operator for 'DeprecatedCopy::Ctor' first required here}}
 
   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}}
+// expected-warning@-1 {{definition of implicit copy constructor for 'Dtor' is deprecated because it has a user-provided destructor}}
+// expected-warning@-2 {{definition of implicit copy assignment operator for 'Dtor' is deprecated because it has a user-provided 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;
+  DefaultedDtor 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
 
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,23 +1,18 @@
+// 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}}
+A& operator=(const A&) = default; // expected-warning {{definition of implicit copy constructor for 'A' 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 =(const B &); // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+B& operator=(const B&) = delete; // 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}}
+void foo() {
+  A a1;
+  A a2(a1); // expected-note {{implicit copy constructor for 'A' 

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

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

xbolva00 updated this revision to Diff 338334.
xbolva00 marked 5 inline comments as done.
xbolva00 added a comment.

Updated, addressed review feedback.


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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -83,21 +83,21 @@
 #if __cplusplus >= 201103L
 namespace DeprecatedCopy {
   struct Assign {
-Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-declared copy assignment operator}}
+Assign =(const Assign&); // expected-warning {{definition of implicit copy constructor for 'Assign' is deprecated because it has a user-provided copy assignment operator}}
   };
   Assign a1, a2(a1); // expected-note {{implicit copy constructor for 'DeprecatedCopy::Assign' first required here}}
 
   struct Ctor {
 Ctor();
-Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-declared copy constructor}}
+Ctor(const Ctor&); // expected-warning {{definition of implicit copy assignment operator for 'Ctor' is deprecated because it has a user-provided copy constructor}}
   };
   Ctor b1, b2;
   void f() { b1 = b2; } // expected-note {{implicit copy assignment operator for 'DeprecatedCopy::Ctor' first required here}}
 
   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}}
+// expected-warning@-1 {{definition of implicit copy constructor for 'Dtor' is deprecated because it has a user-provided destructor}}
+// expected-warning@-2 {{definition of implicit copy assignment operator for 'Dtor' is deprecated because it has a user-provided 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}}
@@ -105,7 +105,8 @@
   struct DefaultedDtor {
 ~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}}
+  DefaultedDtor d1;
+  DefaultedDtor 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
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,9 +1,18 @@
 // RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
 // RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
 
-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}}
+struct A {
+A& operator=(const A&) = default; // expected-warning {{definition of implicit copy constructor for 'A' is deprecated because it has a user-declared copy assignment operator}}
 };
 
-S test(const S& s) { return S(s); } // expected-note {{implicit copy constructor for 'S' first required here}}
\ No newline at end of file
+struct B {
+B& operator=(const B&) = delete; // expected-warning {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+};
+
+void foo() {
+  A a1;
+  A a2(a1); // expected-note {{implicit copy constructor for 'A' first required here}}
+
+  B b1;
+  B b2(b1); // expected-note {{implicit copy constructor for 'B' first required here}}
+}
Index: clang/test/SemaCXX/deprecated-copy-user-provided-dtor.cpp

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

xbolva00 wrote:
> Quuxplusone wrote:
> > 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.
> Yeah, better names are welcome :)
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.)



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13948
+S.Diag(UserDeclaredOperation->getLocation(), DiagID)
+<< RD << /*copy assignment*/ IsCopyAssignment;
   }

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



Comment at: clang/test/SemaCXX/deprecated-copy.cpp:7
+#ifdef NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif

Quuxplusone wrote:
> xbolva00 wrote:
> > xbolva00 wrote:
> > > Quuxplusone wrote:
> > > > 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?
> > > ifdef is here
> > > 
> > > and ifNdef is below :)
> > and DEPRECATED_COPY_DTOR is in own "code block"
> 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.
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"?



Comment at: clang/test/SemaCXX/deprecated-dtor-user-provided.cpp:6
+  int *ptr;
+  ~A(); // expected-warning {{definition of implicit copy constructor for 'A' 
is deprecated because it has a user-declared destructor}}
+};

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?)



Comment at: clang/test/SemaCXX/deprecated.cpp:108
+  };// 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 

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

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

xbolva00 updated this revision to Diff 280694.

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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -103,10 +103,10 @@
   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
 
Index: clang/test/SemaCXX/deprecated-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-user-provided.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-user-provided -verify
+
+struct A {
+  A =(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}}
+}
Index: clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
@@ -0,0 +1,12 @@
+// 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}}
+}
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,23 +1,9 @@
+// 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 =(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}}
\ No newline at end of file
Index: clang/test/SemaCXX/deprecated-copy-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy-dtor.cpp
@@ -0,0 +1,29 @@
+// 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  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+
+template  typename remove_reference::type &(T &);
+} // namespace std
+
+class ITest {
+public:
+  virtual ~ITest() = default; // 

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

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

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

More tests


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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -103,10 +103,10 @@
   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
 
Index: clang/test/SemaCXX/deprecated-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-user-provided.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-user-provided -verify
+
+struct A {
+  A =(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}}
+}
Index: clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
@@ -0,0 +1,12 @@
+// 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}}
+}
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,23 +1,9 @@
+// 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 =(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}}
\ No newline at end of file
Index: clang/test/SemaCXX/deprecated-copy-dtor.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-copy-dtor.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+
+struct S {
+int i;
+~A() = delete; // expected-warning {{definition of implicit copy constructor for 'S' is deprecated because it has a user-declared copy assignment operator}}
+};
+
+S test(const S& s) { return S(s); } // expected-note {{implicit copy constructor for 'S' first required here}}
\ No 

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

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

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

Updated. Thanks for review.


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

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -103,10 +103,10 @@
   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
 
Index: clang/test/SemaCXX/deprecated-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-user-provided.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy-user-provided -verify
+
+struct A {
+  A =(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}}
+}
Index: clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/deprecated-dtor-user-provided.cpp
@@ -0,0 +1,12 @@
+// 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}}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13932,12 +13932,20 @@
 assert(UserDeclaredOperation);
   }
 
-  if (UserDeclaredOperation && UserDeclaredOperation->isUserProvided()) {
-S.Diag(UserDeclaredOperation->getLocation(),
-   isa(UserDeclaredOperation)
-   ? diag::warn_deprecated_copy_dtor_operation
-   : diag::warn_deprecated_copy_operation)
-<< RD << /*copy assignment*/ !isa(CopyOp);
+  if (UserDeclaredOperation) {
+bool UDOIsUserProvided = UserDeclaredOperation->isUserProvided();
+bool UDOIsDestructor = isa(UserDeclaredOperation);
+bool IsCopyAssignment = !isa(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;
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -564,6 +564,12 @@
   "definition of implicit copy %select{constructor|assignment operator}1 "
   "for %0 is deprecated because it has a user-declared destructor">,
   InGroup, DefaultIgnore;
+def warn_deprecated_copy_operation_user_provided : Warning<
+  warn_deprecated_copy_operation.Text>,
+  InGroup, DefaultIgnore;
+def warn_deprecated_copy_dtor_operation_user_provided : Warning<
+  warn_deprecated_copy_dtor_operation.Text>,
+  InGroup, DefaultIgnore;
 def warn_cxx17_compat_exception_spec_in_signature : Warning<
   "mangled name of %0 will change in C++17 due to non-throwing 

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/test/SemaCXX/deprecated-copy.cpp:7
+#ifdef NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif

xbolva00 wrote:
> xbolva00 wrote:
> > Quuxplusone wrote:
> > > 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?
> > ifdef is here
> > 
> > and ifNdef is below :)
> and DEPRECATED_COPY_DTOR is in own "code block"
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714



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

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

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

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: clang/test/SemaCXX/deprecated-copy.cpp:7
+#ifdef NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif

xbolva00 wrote:
> Quuxplusone wrote:
> > 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?
> ifdef is here
> 
> and ifNdef is below :)
and DEPRECATED_COPY_DTOR is in own "code block"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714



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

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

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

xbolva00 marked 4 inline comments as done.
xbolva00 added a comment.

Thanks for initial comments.




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

Quuxplusone wrote:
> 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.
Yeah, better names are welcome :)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:568
+def warn_deprecated_copy_dtor_operation_user_provided : Warning<
+  warn_deprecated_copy_dtor_operation.Text>,
+  InGroup, DefaultIgnore;

Quuxplusone wrote:
> 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.
> 
I think this is established way how to duplicated warning text strings.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13864
+S.Diag(UserDeclaredOperation->getLocation(), DiagID)
 << RD << /*copy assignment*/ !isa(CopyOp);
   }

Quuxplusone wrote:
> I wonder if this would be easier to read as
> 
> ```
> if (UserDeclaredOperation) {
> bool UDOIsUserProvided = UserDeclaredOperation->isUserProvided();
> bool UDOIsDestructor = isa(UserDeclaredOperation);
> bool IsCopyAssignment = !isa(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;
> ```
I have been thinking about similar form, so +1.



Comment at: clang/test/SemaCXX/deprecated-copy.cpp:7
+#ifdef NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif

Quuxplusone wrote:
> 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?
ifdef is here

and ifNdef is below :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714



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

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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158
+def DeprecatedCopy : DiagGroup<"deprecated-copy", 
[DeprecatedCopyUserProvided]>;
+def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", 
[DeprecatedCopyDtorUserProvided]>;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;

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.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:568
+def warn_deprecated_copy_dtor_operation_user_provided : Warning<
+  warn_deprecated_copy_dtor_operation.Text>,
+  InGroup, DefaultIgnore;

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.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:13864
+S.Diag(UserDeclaredOperation->getLocation(), DiagID)
 << RD << /*copy assignment*/ !isa(CopyOp);
   }

I wonder if this would be easier to read as

```
if (UserDeclaredOperation) {
bool UDOIsUserProvided = UserDeclaredOperation->isUserProvided();
bool UDOIsDestructor = isa(UserDeclaredOperation);
bool IsCopyAssignment = !isa(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;
```



Comment at: clang/test/SemaCXX/deprecated-copy.cpp:7
+#ifdef NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79714



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

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

[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.

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 options to on/off this behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79714

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

Index: clang/test/SemaCXX/deprecated.cpp
===
--- clang/test/SemaCXX/deprecated.cpp
+++ clang/test/SemaCXX/deprecated.cpp
@@ -103,10 +103,10 @@
   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
 
Index: clang/test/SemaCXX/deprecated-copy.cpp
===
--- clang/test/SemaCXX/deprecated-copy.cpp
+++ clang/test/SemaCXX/deprecated-copy.cpp
@@ -1,7 +1,12 @@
 // RUN: %clang_cc1 -std=c++11 %s -Wdeprecated-copy -verify
+// RUN: %clang_cc1 -std=c++11 %s -Wno-deprecated-copy-user-provided -DNO_USER_PROVIDED -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 NO_USER_PROVIDED
+// expected-no-diagnostics
+#endif
+
 #ifdef DEPRECATED_COPY_DTOR
 struct A {
   int *ptr;
@@ -14,10 +19,30 @@
 }
 #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}}
+  B =(const B &);
+#ifndef NO_USER_PROVIDED
+// expected-warning@-2 {{definition of implicit copy constructor for 'B' is deprecated because it has a user-declared copy assignment operator}}
+#endif
 };
 
 void bar() {
-  B b1, b2(b1); // expected-note {{implicit copy constructor for 'B' first required here}}
+  B b1, b2(b1);
+#ifndef NO_USER_PROVIDED
+// expected-note@-2 {{implicit copy constructor for 'B' first required here}}
+#endif
 }
+
+struct S {
+  int i;
+  S =(const S &) = delete;
+#ifndef NO_USER_PROVIDED
+// expected-warning@-2 {{definition of implicit copy constructor for 'S' is deprecated because it has a user-declared copy assignment operator}}
+#endif
+};
+
+S test(const S ) { return S(s); }
+#ifndef NO_USER_PROVIDED
+// expected-note@-2  {{in implicit copy constructor for 'S' first required here}}
+#endif
+
 #endif
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13850,11 +13850,17 @@
 assert(UserDeclaredOperation);
   }
 
-  if (UserDeclaredOperation && UserDeclaredOperation->isUserProvided()) {
-S.Diag(UserDeclaredOperation->getLocation(),
-   isa(UserDeclaredOperation)
-   ? diag::warn_deprecated_copy_dtor_operation
-   : diag::warn_deprecated_copy_operation)
+  if (UserDeclaredOperation) {
+unsigned DiagID;
+if (UserDeclaredOperation->isUserProvided())
+  DiagID = isa(UserDeclaredOperation)
+   ? diag::warn_deprecated_copy_dtor_operation_user_provided
+   : diag::warn_deprecated_copy_operation_user_provided;
+else
+  DiagID = isa(UserDeclaredOperation)
+   ? diag::warn_deprecated_copy_dtor_operation
+   : diag::warn_deprecated_copy_operation;
+S.Diag(UserDeclaredOperation->getLocation(), DiagID)
 << RD << /*copy assignment*/ !isa(CopyOp);
   }
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -561,6 +561,12 @@
   "definition of implicit copy %select{constructor|assignment operator}1 "
   "for %0 is deprecated because it has a user-declared destructor">,
   InGroup, DefaultIgnore;
+def warn_deprecated_copy_operation_user_provided