Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-10-04 Thread Stephan Bergmann via cfe-commits

On 04/10/2018 01:14, Richard Smith wrote:
On Tue, 2 Oct 2018 at 01:18, Stephan Bergmann via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:


On 28/09/2018 03:16, Richard Smith via cfe-commits wrote:
 > Author: rsmith
 > Date: Thu Sep 27 18:16:43 2018
 > New Revision: 343285
 >
 > URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
 > Log:
 > [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
 > render the function deleted instead of rendering the program
ill-formed.
 >
 > This change also adds an enabled-by-default warning for the case
where
 > an explicitly-defaulted special member function of a non-template
class
 > is implicitly deleted by the type checking rules. (This fires
either due
 > to this language change or due to pre-C++20 reasons for the
member being
 > implicitly deleted). I've tested this on a large codebase and
found only
 > bugs (where the program means something that's clearly different from
 > what the programmer intended), so this is enabled by default, but we
 > should revisit this if there are problems with this being enabled by
 > default.

Two questions on the new -Wdefaulted-function-deleted:

1  Do you have examples of actual bugs found by that?


What do you mean by "actual bugs"? Every instance I've seen it flag is a 
case where the code means something other than what the programmer 
intended. (Eg, every case fixed by https://reviews.llvm.org/rL343369 is 
a bug in that sense.)


You started to talk about "bugs" here :)  What I had a hard time 
figuring out is what such a case of =default would look like where the 
programmer's intend obviously had been that the function wouldn't be 
implicitly deleted.


However, it's rare that it'll flag something that leads to the runtime 
behavior of the program being different from the intent (usually you 
instead get a later compile error in cases where that would happen). It 
is possible, though -- in particular, it will flag cases where a move 
operation that you really wanted to result in a move actually results in 
a copy, and the program is silently accepted and is less efficient than 
you had expected.


Thanks, that clarifies what you meant with "bugs".


I'm running it on
the LibreOffice code base now and it feels like lots of just noise. 
For

example, I recently added explicitly defaulted decls of the four
copy/move functions to lots of classes with a virtual dtor (where the
dtor needs to be user-declared e.g. because it doesn't override any
virtual base class dtor or because it shall not be defined inline), to
silence GCC's new -Wdeprecated-copy.  If such a class then for example
has a const non-static data member, Clang's
-Wdefaulted-function-deleted
now kicks in for the assignment ops.  I'm not sure whether it's better
to have those functions explicitly deleted (which then needs revisiting
when some details of the class change), or just explicitly defaulted
and
the compiler figure out whether its deleted or not (but which now
causes
Clang to bark), or simply not user-declared at all (but which now
causes
GCC to bark).


When revising (say) a non-copyable class results in it becoming 
copyable, do you want that change to happen with no notice being given 
to the programmer? I at least don't think that it's obvious that you do.


Having gone through all the warnings in the LibreOffice code base now, 
it wasn't that many individual warnings in the end (just blown up by 
being reported over and over again in the same include files), and the 
vast majority was involving classes that already exhibit pathological 
behavior anyway, like deriving from a base that has non-deleted copy 
ctor but deleted copy assignment op, due to somewhat broken overall 
design.  (Plus some cases where members had recently been made const, 
implicitly deleting the assignment ops.)  So I'm rather neutral now on 
whether that warning is on by default.


I'm fine with turning this warning off by default (maybe moving it to 
-Wextra or similar) if there's a feeling that it's more noisy than it is 
useful.

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


Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-10-03 Thread Richard Smith via cfe-commits
On Tue, 2 Oct 2018 at 01:18, Stephan Bergmann via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On 28/09/2018 03:16, Richard Smith via cfe-commits wrote:
> > Author: rsmith
> > Date: Thu Sep 27 18:16:43 2018
> > New Revision: 343285
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
> > Log:
> > [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
> > render the function deleted instead of rendering the program ill-formed.
> >
> > This change also adds an enabled-by-default warning for the case where
> > an explicitly-defaulted special member function of a non-template class
> > is implicitly deleted by the type checking rules. (This fires either due
> > to this language change or due to pre-C++20 reasons for the member being
> > implicitly deleted). I've tested this on a large codebase and found only
> > bugs (where the program means something that's clearly different from
> > what the programmer intended), so this is enabled by default, but we
> > should revisit this if there are problems with this being enabled by
> > default.
>
> Two questions on the new -Wdefaulted-function-deleted:
>
> 1  Do you have examples of actual bugs found by that?


What do you mean by "actual bugs"? Every instance I've seen it flag is a
case where the code means something other than what the programmer
intended. (Eg, every case fixed by https://reviews.llvm.org/rL343369 is a
bug in that sense.)
However, it's rare that it'll flag something that leads to the runtime
behavior of the program being different from the intent (usually you
instead get a later compile error in cases where that would happen). It is
possible, though -- in particular, it will flag cases where a move
operation that you really wanted to result in a move actually results in a
copy, and the program is silently accepted and is less efficient than you
had expected.


> I'm running it on
> the LibreOffice code base now and it feels like lots of just noise.  For
> example, I recently added explicitly defaulted decls of the four
> copy/move functions to lots of classes with a virtual dtor (where the
> dtor needs to be user-declared e.g. because it doesn't override any
> virtual base class dtor or because it shall not be defined inline), to
> silence GCC's new -Wdeprecated-copy.  If such a class then for example
> has a const non-static data member, Clang's -Wdefaulted-function-deleted
> now kicks in for the assignment ops.  I'm not sure whether it's better
> to have those functions explicitly deleted (which then needs revisiting
> when some details of the class change), or just explicitly defaulted and
> the compiler figure out whether its deleted or not (but which now causes
> Clang to bark), or simply not user-declared at all (but which now causes
> GCC to bark).
>

When revising (say) a non-copyable class results in it becoming copyable,
do you want that change to happen with no notice being given to the
programmer? I at least don't think that it's obvious that you do.

I'm fine with turning this warning off by default (maybe moving it to
-Wextra or similar) if there's a feeling that it's more noisy than it is
useful.

2  With
>
> > $ cat test.cc
> > struct S1 { S1 & operator =(S1 const &) = delete; };
> > struct S2 {
> > S1 m;
> > S2(S2 const &);
> > };
> > struct S3: S2 {
> > S3 & operator =(S3 const &) = default;
> > S3 & operator =(S3 &&) = default;
> > };
> >
> > $ clang++ -fsyntax-only test.cc
> > test.cc:7:10: warning: explicitly defaulted copy assignment operator is
> implicitly deleted [-Wdefaulted-function-deleted]
> > S3 & operator =(S3 const &) = default;
> >  ^
> > test.cc:6:12: note: copy assignment operator of 'S3' is implicitly
> deleted because base class 'S2' has a deleted copy assignment operator
> > struct S3: S2 {
> >^
> > test.cc:3:8: note: copy assignment operator of 'S2' is implicitly
> deleted because field 'm' has a deleted copy assignment operator
> > S1 m;
> >^
> > test.cc:1:18: note: 'operator=' has been explicitly marked deleted here
> > struct S1 { S1 & operator =(S1 const &) = delete; };
> >  ^
> > test.cc:8:10: warning: explicitly defaulted move assignment operator is
> implicitly deleted [-Wdefaulted-function-deleted]
> > S3 & operator =(S3 &&) = default;
> >  ^
> > test.cc:6:12: note: move assignment operator of 'S3' is implicitly
> deleted because base class 'S2' has a deleted move assignment operator
> > struct S3: S2 {
> >^
> > test.cc:3:8: note: copy assignment operator of 'S2' is implicitly
> deleted because field 'm' has a deleted copy assignment operator
> > S1 m;
> >^
> > test.cc:1:18: note: 'operator=' has been explicitly marked deleted here
> > struct S1 { S1 & operator =(S1 const &) = delete; };
> >  ^
> > 2 warnings generated.
>
> the final two notes for the second warning are somewhat confusing I
> think, as there's a mismatch from the preceding note's "'S2' 

Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-10-02 Thread Hans Wennborg via cfe-commits
On Tue, Oct 2, 2018 at 12:02 PM, Hans Wennborg  wrote:
> I'm confused about what implicitly deleted means. In this code for example:
>
> struct S {
>   S(const S&) = default;
>   S& operator=(const S&) = default;
>   const int x;
> };
>
> void f(S ) {
>   S t = s;
> }
>
> Clang will warn that operator= is implicitly deleted.
>
> But it seems the code still works fine, whereas if operator= were
> actually deleted it would not compile. So what does implicitly deleted
> mean then?

Oh, I see, my example is wrong. The assignment operator really is
deleted and it uses the copy constructor instead.

>
>
> On Fri, Sep 28, 2018 at 3:16 AM, Richard Smith via cfe-commits
>  wrote:
>> Author: rsmith
>> Date: Thu Sep 27 18:16:43 2018
>> New Revision: 343285
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
>> Log:
>> [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
>> render the function deleted instead of rendering the program ill-formed.
>>
>> This change also adds an enabled-by-default warning for the case where
>> an explicitly-defaulted special member function of a non-template class
>> is implicitly deleted by the type checking rules. (This fires either due
>> to this language change or due to pre-C++20 reasons for the member being
>> implicitly deleted). I've tested this on a large codebase and found only
>> bugs (where the program means something that's clearly different from
>> what the programmer intended), so this is enabled by default, but we
>> should revisit this if there are problems with this being enabled by
>> default.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp
>> cfe/trunk/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
>> cfe/trunk/test/CXX/drs/dr6xx.cpp
>> cfe/trunk/test/CXX/special/class.copy/p12-0x.cpp
>> cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp
>> cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
>> cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
>> cfe/trunk/test/SemaCUDA/implicit-member-target.cu
>> cfe/trunk/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
>> cfe/trunk/test/SemaCXX/cxx17-compat.cpp
>> cfe/trunk/test/SemaCXX/dr1301.cpp
>> cfe/trunk/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
>> cfe/trunk/test/SemaTemplate/exception-spec-crash.cpp
>> cfe/trunk/www/cxx_status.html
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343285=343284=343285=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 27 18:16:43 
>> 2018
>> @@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_sp
>>  def err_incorrect_defaulted_constexpr : Error<
>>"defaulted definition of %sub{select_special_member_kind}0 "
>>"is not constexpr">;
>> +def warn_defaulted_method_deleted : Warning<
>> +  "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
>> +  "deleted">, InGroup>;
>>  def err_out_of_line_default_deletes : Error<
>>"defaulting this %sub{select_special_member_kind}0 "
>>"would delete it after its first declaration">;
>> +def note_deleted_type_mismatch : Note<
>> +  "function is implicitly deleted because its declared type does not match "
>> +  "the type of an implicit %sub{select_special_member_kind}0">;
>> +def warn_cxx17_compat_defaulted_method_type_mismatch : Warning<
>> +  "explicitly defaulting this %sub{select_special_member_kind}0 with a type 
>> "
>> +  "different from the implicit type is incompatible with C++ standards 
>> before "
>> +  "C++2a">, InGroup, DefaultIgnore;
>>  def warn_vbase_moved_multiple_times : Warning<
>>"defaulted move assignment operator of %0 will move assign virtual base "
>>"class %1 multiple times">, InGroup>;
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=343285=343284=343285=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 27 18:16:43 2018
>> @@ -6451,20 +6451,29 @@ void Sema::CheckExplicitlyDefaultedSpeci
>>//copy operation can take a non-const reference) as an implicit
>>//declaration, and
>>// -- not have default arguments.
>> +  // C++2a changes the second bullet to instead delete the function if it's
>> +  // defaulted on its first declaration, unless it's "an assignment 
>> operator,
>> +  // and its return type differs or its parameter type is not a reference".
>> +  bool DeleteOnTypeMismatch = getLangOpts().CPlusPlus2a && First;
>> +  

Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-10-02 Thread Hans Wennborg via cfe-commits
I'm confused about what implicitly deleted means. In this code for example:

struct S {
  S(const S&) = default;
  S& operator=(const S&) = default;
  const int x;
};

void f(S ) {
  S t = s;
}

Clang will warn that operator= is implicitly deleted.

But it seems the code still works fine, whereas if operator= were
actually deleted it would not compile. So what does implicitly deleted
mean then?


On Fri, Sep 28, 2018 at 3:16 AM, Richard Smith via cfe-commits
 wrote:
> Author: rsmith
> Date: Thu Sep 27 18:16:43 2018
> New Revision: 343285
>
> URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
> Log:
> [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
> render the function deleted instead of rendering the program ill-formed.
>
> This change also adds an enabled-by-default warning for the case where
> an explicitly-defaulted special member function of a non-template class
> is implicitly deleted by the type checking rules. (This fires either due
> to this language change or due to pre-C++20 reasons for the member being
> implicitly deleted). I've tested this on a large codebase and found only
> bugs (where the program means something that's clearly different from
> what the programmer intended), so this is enabled by default, but we
> should revisit this if there are problems with this being enabled by
> default.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp
> cfe/trunk/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
> cfe/trunk/test/CXX/drs/dr6xx.cpp
> cfe/trunk/test/CXX/special/class.copy/p12-0x.cpp
> cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp
> cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
> cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
> cfe/trunk/test/SemaCUDA/implicit-member-target.cu
> cfe/trunk/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
> cfe/trunk/test/SemaCXX/cxx17-compat.cpp
> cfe/trunk/test/SemaCXX/dr1301.cpp
> cfe/trunk/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
> cfe/trunk/test/SemaTemplate/exception-spec-crash.cpp
> cfe/trunk/www/cxx_status.html
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343285=343284=343285=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 27 18:16:43 
> 2018
> @@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_sp
>  def err_incorrect_defaulted_constexpr : Error<
>"defaulted definition of %sub{select_special_member_kind}0 "
>"is not constexpr">;
> +def warn_defaulted_method_deleted : Warning<
> +  "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
> +  "deleted">, InGroup>;
>  def err_out_of_line_default_deletes : Error<
>"defaulting this %sub{select_special_member_kind}0 "
>"would delete it after its first declaration">;
> +def note_deleted_type_mismatch : Note<
> +  "function is implicitly deleted because its declared type does not match "
> +  "the type of an implicit %sub{select_special_member_kind}0">;
> +def warn_cxx17_compat_defaulted_method_type_mismatch : Warning<
> +  "explicitly defaulting this %sub{select_special_member_kind}0 with a type "
> +  "different from the implicit type is incompatible with C++ standards 
> before "
> +  "C++2a">, InGroup, DefaultIgnore;
>  def warn_vbase_moved_multiple_times : Warning<
>"defaulted move assignment operator of %0 will move assign virtual base "
>"class %1 multiple times">, InGroup>;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=343285=343284=343285=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 27 18:16:43 2018
> @@ -6451,20 +6451,29 @@ void Sema::CheckExplicitlyDefaultedSpeci
>//copy operation can take a non-const reference) as an implicit
>//declaration, and
>// -- not have default arguments.
> +  // C++2a changes the second bullet to instead delete the function if it's
> +  // defaulted on its first declaration, unless it's "an assignment operator,
> +  // and its return type differs or its parameter type is not a reference".
> +  bool DeleteOnTypeMismatch = getLangOpts().CPlusPlus2a && First;
> +  bool ShouldDeleteForTypeMismatch = false;
>unsigned ExpectedParams = 1;
>if (CSM == CXXDefaultConstructor || CSM == CXXDestructor)
>  ExpectedParams = 0;
>if (MD->getNumParams() != ExpectedParams) {
> -// This also checks for default arguments: a copy or move constructor 
> 

Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-10-02 Thread Stephan Bergmann via cfe-commits

On 28/09/2018 03:16, Richard Smith via cfe-commits wrote:

Author: rsmith
Date: Thu Sep 27 18:16:43 2018
New Revision: 343285

URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
Log:
[cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
render the function deleted instead of rendering the program ill-formed.

This change also adds an enabled-by-default warning for the case where
an explicitly-defaulted special member function of a non-template class
is implicitly deleted by the type checking rules. (This fires either due
to this language change or due to pre-C++20 reasons for the member being
implicitly deleted). I've tested this on a large codebase and found only
bugs (where the program means something that's clearly different from
what the programmer intended), so this is enabled by default, but we
should revisit this if there are problems with this being enabled by
default.


Two questions on the new -Wdefaulted-function-deleted:

1  Do you have examples of actual bugs found by that?  I'm running it on 
the LibreOffice code base now and it feels like lots of just noise.  For 
example, I recently added explicitly defaulted decls of the four 
copy/move functions to lots of classes with a virtual dtor (where the 
dtor needs to be user-declared e.g. because it doesn't override any 
virtual base class dtor or because it shall not be defined inline), to 
silence GCC's new -Wdeprecated-copy.  If such a class then for example 
has a const non-static data member, Clang's -Wdefaulted-function-deleted 
now kicks in for the assignment ops.  I'm not sure whether it's better 
to have those functions explicitly deleted (which then needs revisiting 
when some details of the class change), or just explicitly defaulted and 
the compiler figure out whether its deleted or not (but which now causes 
Clang to bark), or simply not user-declared at all (but which now causes 
GCC to bark).


2  With


$ cat test.cc
struct S1 { S1 & operator =(S1 const &) = delete; };
struct S2 {
S1 m;
S2(S2 const &);
};
struct S3: S2 {
S3 & operator =(S3 const &) = default;
S3 & operator =(S3 &&) = default;
};

$ clang++ -fsyntax-only test.cc
test.cc:7:10: warning: explicitly defaulted copy assignment operator is 
implicitly deleted [-Wdefaulted-function-deleted]
S3 & operator =(S3 const &) = default;
 ^
test.cc:6:12: note: copy assignment operator of 'S3' is implicitly deleted 
because base class 'S2' has a deleted copy assignment operator
struct S3: S2 {
   ^
test.cc:3:8: note: copy assignment operator of 'S2' is implicitly deleted 
because field 'm' has a deleted copy assignment operator
S1 m;
   ^
test.cc:1:18: note: 'operator=' has been explicitly marked deleted here
struct S1 { S1 & operator =(S1 const &) = delete; };
 ^
test.cc:8:10: warning: explicitly defaulted move assignment operator is 
implicitly deleted [-Wdefaulted-function-deleted]
S3 & operator =(S3 &&) = default;
 ^
test.cc:6:12: note: move assignment operator of 'S3' is implicitly deleted 
because base class 'S2' has a deleted move assignment operator
struct S3: S2 {
   ^
test.cc:3:8: note: copy assignment operator of 'S2' is implicitly deleted 
because field 'm' has a deleted copy assignment operator
S1 m;
   ^
test.cc:1:18: note: 'operator=' has been explicitly marked deleted here
struct S1 { S1 & operator =(S1 const &) = delete; };
 ^
2 warnings generated.


the final two notes for the second warning are somewhat confusing I 
think, as there's a mismatch from the preceding note's "'S2' has a 
deleted move assignment operator" to the second-last note's "copy 
assignment operator of 'S2' is implicitly deleted".  Not sure though 
whether this is an issue specific to this new 
-Wdefaulted-function-deleted, or a general issue with generic code 
producing such "deleted because" notes.

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


Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-09-28 Thread Vitaly Buka via cfe-commits
https://reviews.llvm.org/rL343369

On Fri, Sep 28, 2018 at 4:51 PM Galina Kistanova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hello Richard,
>
> This commit broke couple of our builders:
>
> http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/12924
> http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/10475
> . . .
> FAILED:
> lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o
>
> /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/install/stage1/bin/clang++
> -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> -Ilib/Transforms/Instrumentation
> -I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation
> -Iinclude
> -I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/include
> -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time
> -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor
> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color
> -ffunction-sections -fdata-sections -O3-UNDEBUG  -fno-exceptions
> -fno-rtti -MD -MT
> lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o
> -MF
> lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o.d
> -o
> lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o
> -c
> /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
> /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:231:24:
> error: explicitly defaulted move assignment operator is implicitly deleted
> [-Werror,-Wdefaulted-function-deleted]
>   TransformedFunction& operator=(TransformedFunction&&) = default;
>^
> /home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:234:23:
> note: move assignment operator of 'TransformedFunction' is implicitly
> deleted because field 'OriginalType' is of const-qualified type
> 'llvm::FunctionType *const'
>   FunctionType* const OriginalType;
>   ^
> 1 error generated.
>
> Please have a look?
>
> Thanks
>
> Galina
>
> On Thu, Sep 27, 2018 at 6:18 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Thu Sep 27 18:16:43 2018
>> New Revision: 343285
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
>> Log:
>> [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
>> render the function deleted instead of rendering the program ill-formed.
>>
>> This change also adds an enabled-by-default warning for the case where
>> an explicitly-defaulted special member function of a non-template class
>> is implicitly deleted by the type checking rules. (This fires either due
>> to this language change or due to pre-C++20 reasons for the member being
>> implicitly deleted). I've tested this on a large codebase and found only
>> bugs (where the program means something that's clearly different from
>> what the programmer intended), so this is enabled by default, but we
>> should revisit this if there are problems with this being enabled by
>> default.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp
>> cfe/trunk/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
>> cfe/trunk/test/CXX/drs/dr6xx.cpp
>> cfe/trunk/test/CXX/special/class.copy/p12-0x.cpp
>> cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp
>> cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
>> cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
>> cfe/trunk/test/SemaCUDA/implicit-member-target.cu
>> cfe/trunk/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
>> cfe/trunk/test/SemaCXX/cxx17-compat.cpp
>> cfe/trunk/test/SemaCXX/dr1301.cpp
>> cfe/trunk/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
>> cfe/trunk/test/SemaTemplate/exception-spec-crash.cpp
>> cfe/trunk/www/cxx_status.html
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343285=343284=343285=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 27
>> 18:16:43 2018
>> @@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_sp
>>  def err_incorrect_defaulted_constexpr : 

Re: r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-09-28 Thread Galina Kistanova via cfe-commits
Hello Richard,

This commit broke couple of our builders:

http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/12924
http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/10475
. . .
FAILED:
lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o

/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/install/stage1/bin/clang++
-DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
-Ilib/Transforms/Instrumentation
-I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation
-Iinclude
-I/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/include
-fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time
-Werror=unguarded-availability-new -std=c++11 -Wall -Wextra
-Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor
-Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color
-ffunction-sections -fdata-sections -O3-UNDEBUG  -fno-exceptions
-fno-rtti -MD -MT
lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o
-MF
lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o.d
-o
lib/Transforms/Instrumentation/CMakeFiles/LLVMInstrumentation.dir/DataFlowSanitizer.cpp.o
-c
/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:231:24:
error: explicitly defaulted move assignment operator is implicitly deleted
[-Werror,-Wdefaulted-function-deleted]
  TransformedFunction& operator=(TransformedFunction&&) = default;
   ^
/home/buildslave/ps4-buildslave1/clang-with-thin-lto-ubuntu/llvm.src/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:234:23:
note: move assignment operator of 'TransformedFunction' is implicitly
deleted because field 'OriginalType' is of const-qualified type
'llvm::FunctionType *const'
  FunctionType* const OriginalType;
  ^
1 error generated.

Please have a look?

Thanks

Galina

On Thu, Sep 27, 2018 at 6:18 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu Sep 27 18:16:43 2018
> New Revision: 343285
>
> URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
> Log:
> [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
> render the function deleted instead of rendering the program ill-formed.
>
> This change also adds an enabled-by-default warning for the case where
> an explicitly-defaulted special member function of a non-template class
> is implicitly deleted by the type checking rules. (This fires either due
> to this language change or due to pre-C++20 reasons for the member being
> implicitly deleted). I've tested this on a large codebase and found only
> bugs (where the program means something that's clearly different from
> what the programmer intended), so this is enabled by default, but we
> should revisit this if there are problems with this being enabled by
> default.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp
> cfe/trunk/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
> cfe/trunk/test/CXX/drs/dr6xx.cpp
> cfe/trunk/test/CXX/special/class.copy/p12-0x.cpp
> cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp
> cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
> cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
> cfe/trunk/test/SemaCUDA/implicit-member-target.cu
> cfe/trunk/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
> cfe/trunk/test/SemaCXX/cxx17-compat.cpp
> cfe/trunk/test/SemaCXX/dr1301.cpp
> cfe/trunk/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
> cfe/trunk/test/SemaTemplate/exception-spec-crash.cpp
> cfe/trunk/www/cxx_status.html
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343285=343284=343285=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 27
> 18:16:43 2018
> @@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_sp
>  def err_incorrect_defaulted_constexpr : Error<
>"defaulted definition of %sub{select_special_member_kind}0 "
>"is not constexpr">;
> +def warn_defaulted_method_deleted : Warning<
> +  "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
> +  "deleted">, InGroup>;
>  def 

r343285 - [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only

2018-09-27 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Sep 27 18:16:43 2018
New Revision: 343285

URL: http://llvm.org/viewvc/llvm-project?rev=343285=rev
Log:
[cxx2a] P0641R2: (Some) type mismatches on defaulted functions only
render the function deleted instead of rendering the program ill-formed.

This change also adds an enabled-by-default warning for the case where
an explicitly-defaulted special member function of a non-template class
is implicitly deleted by the type checking rules. (This fires either due
to this language change or due to pre-C++20 reasons for the member being
implicitly deleted). I've tested this on a large codebase and found only
bugs (where the program means something that's clearly different from
what the programmer intended), so this is enabled by default, but we
should revisit this if there are problems with this being enabled by
default.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp
cfe/trunk/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p1.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/test/CXX/special/class.copy/p12-0x.cpp
cfe/trunk/test/CXX/special/class.copy/p23-cxx11.cpp
cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp
cfe/trunk/test/SemaCUDA/implicit-member-target.cu
cfe/trunk/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
cfe/trunk/test/SemaCXX/cxx17-compat.cpp
cfe/trunk/test/SemaCXX/dr1301.cpp
cfe/trunk/test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
cfe/trunk/test/SemaTemplate/exception-spec-crash.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=343285=343284=343285=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 27 18:16:43 
2018
@@ -7760,9 +7760,19 @@ def err_incorrect_defaulted_exception_sp
 def err_incorrect_defaulted_constexpr : Error<
   "defaulted definition of %sub{select_special_member_kind}0 "
   "is not constexpr">;
+def warn_defaulted_method_deleted : Warning<
+  "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
+  "deleted">, InGroup>;
 def err_out_of_line_default_deletes : Error<
   "defaulting this %sub{select_special_member_kind}0 "
   "would delete it after its first declaration">;
+def note_deleted_type_mismatch : Note<
+  "function is implicitly deleted because its declared type does not match "
+  "the type of an implicit %sub{select_special_member_kind}0">;
+def warn_cxx17_compat_defaulted_method_type_mismatch : Warning<
+  "explicitly defaulting this %sub{select_special_member_kind}0 with a type "
+  "different from the implicit type is incompatible with C++ standards before "
+  "C++2a">, InGroup, DefaultIgnore;
 def warn_vbase_moved_multiple_times : Warning<
   "defaulted move assignment operator of %0 will move assign virtual base "
   "class %1 multiple times">, InGroup>;

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=343285=343284=343285=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 27 18:16:43 2018
@@ -6451,20 +6451,29 @@ void Sema::CheckExplicitlyDefaultedSpeci
   //copy operation can take a non-const reference) as an implicit
   //declaration, and
   // -- not have default arguments.
+  // C++2a changes the second bullet to instead delete the function if it's
+  // defaulted on its first declaration, unless it's "an assignment operator,
+  // and its return type differs or its parameter type is not a reference".
+  bool DeleteOnTypeMismatch = getLangOpts().CPlusPlus2a && First;
+  bool ShouldDeleteForTypeMismatch = false;
   unsigned ExpectedParams = 1;
   if (CSM == CXXDefaultConstructor || CSM == CXXDestructor)
 ExpectedParams = 0;
   if (MD->getNumParams() != ExpectedParams) {
-// This also checks for default arguments: a copy or move constructor with 
a
+// This checks for default arguments: a copy or move constructor with a
 // default argument is classified as a default constructor, and assignment
 // operations and destructors can't have default arguments.
 Diag(MD->getLocation(), diag::err_defaulted_special_member_params)
   << CSM << MD->getSourceRange();
 HadError = true;
   } else if (MD->isVariadic()) {
-Diag(MD->getLocation(), diag::err_defaulted_special_member_variadic)
-  << CSM << MD->getSourceRange();
-HadError = true;
+if (DeleteOnTypeMismatch)
+  ShouldDeleteForTypeMismatch = true;
+else {
+