[cfe-commits] r132104 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/copy-constructor-error.cpp test/SemaCXX/default-arg-special-member.cpp

[Matt Beaumont-Gay]

On Wed, May 25, 2011 at 18:26, Sean Hunt <scshunt at csclub.uwaterloo.ca> wrote:
> Author: coppro
> Date: Wed May 25 20:26:05 2011
> New Revision: 132104
>
> URL: http://llvm.org/viewvc/llvm-project?rev=132104&view=rev
> Log:
> Implement a new warning for when adding a default argument to a method
> makes it into a special member function. This is very bad and can lead
> to all sorts of nastiness including implicit member functions violating
> the One Definition Rule. This should probably be made ill-formed in a
> later version of the standard, but for now we'll just warn.
>
> Added:
>    cfe/trunk/test/SemaCXX/default-arg-special-member.cpp
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>    cfe/trunk/test/SemaCXX/copy-constructor-error.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=132104&r1=132103&r2=132104&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 25 20:26:05 2011
> @@ -1416,6 +1416,13 @@
>  def err_uninitialized_member_in_ctor : Error<
>   "%select{|implicit default }0constructor for %1 must explicitly initialize "
>   "the %select{reference|const}2 member %3">;
> +def warn_default_arg_makes_ctor_special : Warning<
> +  "addition of default argument on redeclaration makes this constructor a "
> +  "%select{default|copy|move}0 constructor">;

Please put this warning in a DiagGroup so it can be turned off.

> +def note_previous_declaration_special : Note<
> +  // The ERRORs are in hopes that if they occur, they'll get reported.
> +  "previous declaration was %select{*ERROR*|a copy constructor|a move "
> +  "constructor|*ERROR*|*ERROR*|*ERROR*|not a special member function}0">;
>
>  def err_use_of_default_argument_to_function_declared_later : Error<
>   "use of default argument to function %0 that is declared later in class %1">;
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=132104&r1=132103&r2=132104&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed May 25 20:26:05 2011
> @@ -1505,14 +1505,14 @@
>  /// getSpecialMember - get the special member enum for a method.
>  Sema::CXXSpecialMember Sema::getSpecialMember(const CXXMethodDecl *MD) {
>   if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) {
> +    if (Ctor->isDefaultConstructor())
> +      return Sema::CXXDefaultConstructor;
> +
>     if (Ctor->isCopyConstructor())
>       return Sema::CXXCopyConstructor;
> -
> +
>     if (Ctor->isMoveConstructor())
>       return Sema::CXXMoveConstructor;
> -
> -    if (Ctor->isDefaultConstructor())
> -      return Sema::CXXDefaultConstructor;
>   } else if (isa<CXXDestructorDecl>(MD)) {
>     return Sema::CXXDestructor;
>   } else if (MD->isCopyAssignmentOperator()) {
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=132104&r1=132103&r2=132104&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed May 25 20:26:05 2011
> @@ -457,6 +457,15 @@
>              diag::err_param_default_argument_member_template_redecl)
>           << WhichKind
>           << NewParam->getDefaultArgRange();
> +      } else if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(New)) {
> +        CXXSpecialMember NewSM = getSpecialMember(Ctor),
> +                         OldSM = getSpecialMember(cast<CXXConstructorDecl>(Old));
> +        if (NewSM != OldSM) {
> +          Diag(NewParam->getLocation(),diag::warn_default_arg_makes_ctor_special)
> +            << NewParam->getDefaultArgRange() << NewSM;
> +          Diag(Old->getLocation(), diag::note_previous_declaration_special)
> +            << OldSM;
> +        }
>       }
>     }
>   }
>
> Modified: cfe/trunk/test/SemaCXX/copy-constructor-error.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/copy-constructor-error.cpp?rev=132104&r1=132103&r2=132104&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/copy-constructor-error.cpp (original)
> +++ cfe/trunk/test/SemaCXX/copy-constructor-error.cpp Wed May 25 20:26:05 2011
> @@ -13,10 +13,10 @@
>  namespace PR6064 {
>   struct A {
>     A() { }
> -    inline A(A&, int);
> +    inline A(A&, int); // expected-note {{was not a special member function}}
>   };
>
> -  A::A(A&, int = 0) { }
> +  A::A(A&, int = 0) { } // expected-warning {{makes this constructor a copy constructor}}
>
>   void f() {
>     A const a;
>
> Added: cfe/trunk/test/SemaCXX/default-arg-special-member.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/default-arg-special-member.cpp?rev=132104&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/default-arg-special-member.cpp (added)
> +++ cfe/trunk/test/SemaCXX/default-arg-special-member.cpp Wed May 25 20:26:05 2011
> @@ -0,0 +1,11 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +
> +class foo {
> +  foo(foo&, int); // expected-note {{was not a special member function}}
> +  foo(int); // expected-note {{was not a special member function}}
> +  foo(const foo&); // expected-note {{was a copy constructor}}
> +};
> +
> +foo::foo(foo&, int = 0) { } // expected-warning {{makes this constructor a copy constructor}}
> +foo::foo(int = 0) { } // expected-warning {{makes this constructor a default constructor}}
> +foo::foo(const foo& = 0) { } //expected-warning {{makes this constructor a default constructor}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>