From 9ae60d54e4454192384933f3020968ea5c8c3540 Mon Sep 17 00:00:00 2001 From: Sean Hunt Date: Thu, 26 May 2011 01:26:05 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@132104 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 7 +++++++ lib/Sema/SemaDecl.cpp | 12 ++++++------ lib/Sema/SemaDeclCXX.cpp | 9 +++++++++ test/SemaCXX/copy-constructor-error.cpp | 4 ++-- test/SemaCXX/default-arg-special-member.cpp | 11 +++++++++++ 5 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 test/SemaCXX/default-arg-special-member.cpp diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index f36f3475c1..5e1f3f5818 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1416,6 +1416,13 @@ def note_first_required_here : Note< 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">; +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">; diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index dae7ba20b1..77bd5b79ae 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -1505,14 +1505,14 @@ struct GNUCompatibleParamWarning { /// getSpecialMember - get the special member enum for a method. Sema::CXXSpecialMember Sema::getSpecialMember(const CXXMethodDecl *MD) { if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) { - if (Ctor->isCopyConstructor()) - return Sema::CXXCopyConstructor; - - if (Ctor->isMoveConstructor()) - return Sema::CXXMoveConstructor; - if (Ctor->isDefaultConstructor()) return Sema::CXXDefaultConstructor; + + if (Ctor->isCopyConstructor()) + return Sema::CXXCopyConstructor; + + if (Ctor->isMoveConstructor()) + return Sema::CXXMoveConstructor; } else if (isa(MD)) { return Sema::CXXDestructor; } else if (MD->isCopyAssignmentOperator()) { diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 6870f3c46b..2fd4cf58ca 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -457,6 +457,15 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old) { diag::err_param_default_argument_member_template_redecl) << WhichKind << NewParam->getDefaultArgRange(); + } else if (CXXConstructorDecl *Ctor = dyn_cast(New)) { + CXXSpecialMember NewSM = getSpecialMember(Ctor), + OldSM = getSpecialMember(cast(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; + } } } } diff --git a/test/SemaCXX/copy-constructor-error.cpp b/test/SemaCXX/copy-constructor-error.cpp index 9809bfc84b..64a7d58e19 100644 --- a/test/SemaCXX/copy-constructor-error.cpp +++ b/test/SemaCXX/copy-constructor-error.cpp @@ -13,10 +13,10 @@ void g() { 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; diff --git a/test/SemaCXX/default-arg-special-member.cpp b/test/SemaCXX/default-arg-special-member.cpp new file mode 100644 index 0000000000..aadbbeb2fc --- /dev/null +++ b/test/SemaCXX/default-arg-special-member.cpp @@ -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}}