C++ core issue 1344, PR10618: promote "addition of default argument makes this

a special member" diagnostic from warning to error, and fix the cases where it
produced diagnostics with incorrect wording.

We don't support this as an extension, and we ban it even in C++98 mode. This
breaks too much (for instance, the ABI-specified calling convention for a type
can change if it acquires a copy constructor through the addition of a default
argument).

llvm-svn: 168769

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index f9f9ec7..cae627e 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -43,7 +43,6 @@ def : DiagGroup<"cast-qual">;
 def : DiagGroup<"char-align">;
 def Comment : DiagGroup<"comment">;
 def : DiagGroup<"ctor-dtor-privacy">;
-def DefaultArgSpecialMember : DiagGroup<"default-arg-special-member">;
 def GNUDesignator : DiagGroup<"gnu-designator">;
 
 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a9e2401..8d172ca 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2159,13 +2159,9 @@ def err_uninitialized_member_for_assign : Error<
 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<
+def err_default_arg_makes_ctor_special : Error<
   "addition of default argument on redeclaration makes this constructor a "
-  "%select{default|copy|move}0 constructor">, InGroup<DefaultArgSpecialMember>;
-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">;
+  "%select{default|copy|move}0 constructor">;
 
 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/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index e950f3e..86da9e9 100644 (file)
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -517,19 +517,26 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
              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;
-        }
       }
     }
   }
 
+  // DR1344: If a default argument is added outside a class definition and that
+  // default argument makes the function a special member function, the program
+  // is ill-formed. This can only happen for constructors.
+  if (isa<CXXConstructorDecl>(New) &&
+      New->getMinRequiredArguments() < Old->getMinRequiredArguments()) {
+    CXXSpecialMember NewSM = getSpecialMember(cast<CXXMethodDecl>(New)),
+                     OldSM = getSpecialMember(cast<CXXMethodDecl>(Old));
+    if (NewSM != OldSM) {
+      ParmVarDecl *NewParam = New->getParamDecl(New->getMinRequiredArguments());
+      assert(NewParam->hasDefaultArg());
+      Diag(NewParam->getLocation(), diag::err_default_arg_makes_ctor_special)
+        << NewParam->getDefaultArgRange() << NewSM;
+      Diag(Old->getLocation(), diag::note_previous_declaration);
+    }
+  }
+
   // C++11 [dcl.constexpr]p1: If any declaration of a function or function
   // template has a constexpr specifier then all its declarations shall
   // contain the constexpr specifier.

diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
index dfc1d3d..ad156c8 100644 (file)
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -272,9 +272,8 @@ namespace CtorLookup {
   struct A {
     constexpr A(const A&) {}
     A(A&) {}
-    constexpr A(int); // expected-note {{previous}}
+    constexpr A(int = 0);
   };
-  constexpr A::A(int = 0) {} // expected-warning {{default constructor}}
 
   struct B : A {
     B() = default;

diff --git a/clang/test/SemaCXX/copy-constructor-error.cpp b/clang/test/SemaCXX/copy-constructor-error.cpp
index 64a7d58..6ffed9b 100644 (file)
--- a/clang/test/SemaCXX/copy-constructor-error.cpp
+++ b/clang/test/SemaCXX/copy-constructor-error.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 
 struct S {
    S (S);  // expected-error {{copy constructor must pass its first argument by reference}}
@@ -10,16 +10,50 @@ void g() {
   S a( f() );
 }
 
+class foo {
+  foo(foo&, int); // expected-note {{previous}}
+  foo(int); // expected-note {{previous}}
+  foo(const foo&); // expected-note {{previous}}
+};
+
+foo::foo(foo&, int = 0) { } // expected-error {{makes this constructor a copy constructor}}
+foo::foo(int = 0) { } // expected-error {{makes this constructor a default constructor}}
+foo::foo(const foo& = 0) { } //expected-error {{makes this constructor a default constructor}}
+
 namespace PR6064 {
   struct A {
     A() { }
-    inline A(A&, int); // expected-note {{was not a special member function}}
+    inline A(A&, int); // expected-note {{previous}}
   };
 
-  A::A(A&, int = 0) { } // expected-warning {{makes this constructor a copy constructor}}
+  A::A(A&, int = 0) { } // expected-error {{makes this constructor a copy constructor}}
 
   void f() {
     A const a;
     A b(a);
   }
 }
+
+namespace PR10618 {
+  struct A {
+    A(int, int, int); // expected-note {{previous}}
+  };
+  A::A(int a = 0, // expected-error {{makes this constructor a default constructor}}
+       int b = 0,
+       int c = 0) {}
+
+  struct B {
+    B(int);
+    B(const B&, int); // expected-note {{previous}}
+  };
+  B::B(const B& = B(0), // expected-error {{makes this constructor a default constructor}}
+       int = 0) {
+  }
+
+  struct C {
+    C(const C&, int); // expected-note {{previous}}
+  };
+  C::C(const C&,
+       int = 0) { // expected-error {{makes this constructor a copy constructor}}
+  }
+}

diff --git a/clang/test/SemaCXX/default-arg-special-member.cpp b/clang/test/SemaCXX/default-arg-special-member.cpp
deleted file mode 100644 (file)
index 8402d38..0000000
--- a/clang/test/SemaCXX/default-arg-special-member.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -Wno-default-arg-special-member -Werror -fsyntax-only %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}}