C++ PATCH for core issue 1331 (const mismatch with defaulted copy constructor)

Message ID 20180612190245.GP28085@redhat.com
State New
Headers show
Series
  • C++ PATCH for core issue 1331 (const mismatch with defaulted copy constructor)
Related show

Commit Message

Marek Polacek June 12, 2018, 7:02 p.m.
As [class.copy.ctor] says, the type of an implicitly declared copy constructor
will be either

  X::X(const X&)

if each potentially constructed subobject of a class type M (or array thereof)
has a copy constructor whose first parameter is of type const M& or const
volatile M&.  Otherwise, the implicitly-declared copy constructor will
have the form

  X::X(X&)

Similarly for implicitly-declared copy assignment operator: it will be either

  X& operator=(const X&)

or

  X& operator=(X&)

Previously, the declared type of an explicitly defaulted function had to be the
same as if it had been implicitly declared, otherwise the code was ill-formed.
But Core Issue 1331 [1] changes this in such a way that it be defined as deleted
instead, with two exceptions for an assignment operator with a mismatched return
type, and an assignment operator with a parameter type that's not a reference.

The change itself seems trivial (unless I'm missing something), so most of my
effort went into writing testcases for various scenarios I could think of.
We already give errors for cases that are still considered ill-formed.

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0641r2.html

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-06-12  Marek Polacek  <polacek@redhat.com>

	Core issue 1331 - const mismatch with defaulted copy constructor
	* class.c (check_bases_and_members): When checking a defaulted
	function, mark it as deleted rather than giving an error.

	* g++.dg/cpp0x/defaulted15.C (struct F): Remove dg-error.
	* g++.dg/cpp0x/defaulted52.C: New test.
	* g++.dg/cpp0x/defaulted53.C: New test.
	* g++.dg/cpp0x/defaulted54.C: New test.
	* g++.dg/cpp0x/defaulted55.C: New test.
	* g++.dg/cpp0x/defaulted56.C: New test.
	* g++.dg/cpp0x/defaulted57.C: New test.
	* g++.dg/cpp0x/defaulted58.C: New test.
	* g++.dg/cpp0x/defaulted59.C: New test.
	* g++.dg/cpp0x/defaulted60.C: New test.

Comments

Jason Merrill June 12, 2018, 8:14 p.m. | #1
OK.

On Tue, Jun 12, 2018 at 3:02 PM, Marek Polacek <polacek@redhat.com> wrote:
> As [class.copy.ctor] says, the type of an implicitly declared copy constructor
> will be either
>
>   X::X(const X&)
>
> if each potentially constructed subobject of a class type M (or array thereof)
> has a copy constructor whose first parameter is of type const M& or const
> volatile M&.  Otherwise, the implicitly-declared copy constructor will
> have the form
>
>   X::X(X&)
>
> Similarly for implicitly-declared copy assignment operator: it will be either
>
>   X& operator=(const X&)
>
> or
>
>   X& operator=(X&)
>
> Previously, the declared type of an explicitly defaulted function had to be the
> same as if it had been implicitly declared, otherwise the code was ill-formed.
> But Core Issue 1331 [1] changes this in such a way that it be defined as deleted
> instead, with two exceptions for an assignment operator with a mismatched return
> type, and an assignment operator with a parameter type that's not a reference.
>
> The change itself seems trivial (unless I'm missing something), so most of my
> effort went into writing testcases for various scenarios I could think of.
> We already give errors for cases that are still considered ill-formed.
>
> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0641r2.html
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-06-12  Marek Polacek  <polacek@redhat.com>
>
>         Core issue 1331 - const mismatch with defaulted copy constructor
>         * class.c (check_bases_and_members): When checking a defaulted
>         function, mark it as deleted rather than giving an error.
>
>         * g++.dg/cpp0x/defaulted15.C (struct F): Remove dg-error.
>         * g++.dg/cpp0x/defaulted52.C: New test.
>         * g++.dg/cpp0x/defaulted53.C: New test.
>         * g++.dg/cpp0x/defaulted54.C: New test.
>         * g++.dg/cpp0x/defaulted55.C: New test.
>         * g++.dg/cpp0x/defaulted56.C: New test.
>         * g++.dg/cpp0x/defaulted57.C: New test.
>         * g++.dg/cpp0x/defaulted58.C: New test.
>         * g++.dg/cpp0x/defaulted59.C: New test.
>         * g++.dg/cpp0x/defaulted60.C: New test.
>
> diff --git gcc/cp/class.c gcc/cp/class.c
> index fbf39035e18..b6e78c6377d 100644
> --- gcc/cp/class.c
> +++ gcc/cp/class.c
> @@ -5660,9 +5660,9 @@ check_bases_and_members (tree t)
>
>             if (fn_const_p && !imp_const_p)
>               /* If the function is defaulted outside the class, we just
> -                give the synthesis error.  */
> -             error ("%q+D declared to take const reference, but implicit "
> -                    "declaration would take non-const", fn);
> +                give the synthesis error.  Core Issue #1331 says this is
> +                no longer ill-formed, it is defined as deleted instead.  */
> +             DECL_DELETED_FN (fn) = true;
>           }
>         defaulted_late_check (fn);
>        }
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted15.C gcc/testsuite/g++.dg/cpp0x/defaulted15.C
> index fabcc23a150..1e0b3545840 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted15.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted15.C
> @@ -48,8 +48,7 @@ struct F
>
>  struct G: public F
>  {
> -  // Can't be const because F copy ctor isn't.
> -  G(const G&) = default;       // { dg-error "const" }
> +  G(const G&) = default;
>  };
>
>  struct H
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted52.C gcc/testsuite/g++.dg/cpp0x/defaulted52.C
> index e69de29bb2d..c617230b493 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted52.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted52.C
> @@ -0,0 +1,20 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M();
> +  // So that W wouldn't have had "const W&" copy ctor if it were
> +  // implicitly declared.
> +  M(M&);
> +};
> +
> +template<typename T> struct W
> +{
> +  W();
> +  // This should now compile and be =deleted.
> +  W(const W&) = default;
> +  T t;
> +};
> +
> +W<M> w;
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted53.C gcc/testsuite/g++.dg/cpp0x/defaulted53.C
> index e69de29bb2d..8147e7e2ad1 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted53.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted53.C
> @@ -0,0 +1,35 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M& operator=(M&);
> +};
> +
> +struct R
> +{
> +  R& operator=(R&) = default;
> +  M m;
> +};
> +
> +struct S
> +{
> +  S& operator=(const S&) = default;
> +  M m;
> +};
> +
> +struct T
> +{
> +  // If F is an assignment operator, and the return type of T1
> +  // differs from the return type of T2 the program is ill-formed.
> +  T operator=(T&) = default; // { dg-error "defaulted" }
> +  M m;
> +};
> +
> +struct U
> +{
> +  // If F is an assignment operator, and T1's parameter type is
> +  // not a reference, the program is ill-formed.
> +  U& operator=(U) = default; // { dg-error "defaulted" }
> +  M m;
> +};
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted54.C gcc/testsuite/g++.dg/cpp0x/defaulted54.C
> index e69de29bb2d..f8ddc4e47ce 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted54.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted54.C
> @@ -0,0 +1,18 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M();
> +  M(M&);
> +};
> +
> +template<typename T> struct W
> +{
> +  W();
> +  W(const W&) = default; // { dg-error "binding" }
> +  T t;
> +};
> +
> +W<M> w;
> +W<M> w2 = w; // { dg-error "use of deleted function" }
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted55.C gcc/testsuite/g++.dg/cpp0x/defaulted55.C
> index e69de29bb2d..04cfc172000 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted55.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted55.C
> @@ -0,0 +1,19 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M();
> +  M(M&);
> +};
> +
> +template<typename T> struct W
> +{
> +  W();
> +  W(W&) = default;
> +  // T1 and T2 may have differing ref-qualifiers (copy assign op).
> +  constexpr W& operator=(const W&) && = default; // { dg-error "defaulted" "" { target c++11_down } }
> +  T t;
> +};
> +
> +W<M> w;
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted56.C gcc/testsuite/g++.dg/cpp0x/defaulted56.C
> index e69de29bb2d..e7ce12c5566 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted56.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted56.C
> @@ -0,0 +1,25 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +// If T2 (what would be the implicit declaration) has a parameter of
> +// type const C&, the corresponding parameter of T1 may be of type C&.
> +
> +struct S
> +{
> +  constexpr S(S &) = default;
> +};
> +
> +struct T
> +{
> +  constexpr T(volatile T &) = default; // { dg-error "defaulted" }
> +};
> +
> +struct U
> +{
> +  constexpr U(const volatile U &) = default; // { dg-error "defaulted" }
> +};
> +
> +struct V
> +{
> +  constexpr V(const V &) = default;
> +};
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted57.C gcc/testsuite/g++.dg/cpp0x/defaulted57.C
> index e69de29bb2d..37fb7dd6e1d 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted57.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted57.C
> @@ -0,0 +1,25 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +// If T2 (what would be the implicit declaration) has a parameter of
> +// type const C&, the corresponding parameter of T1 may be of type C&.
> +
> +struct S
> +{
> +  S& operator=(S &) = default;
> +};
> +
> +struct T
> +{
> +  T& operator=(volatile T &) = default; // { dg-error "defaulted" }
> +};
> +
> +struct U
> +{
> +  U& operator=(const volatile U &) = default; // { dg-error "defaulted" }
> +};
> +
> +struct V
> +{
> +  V& operator=(const V &) = default;
> +};
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted58.C gcc/testsuite/g++.dg/cpp0x/defaulted58.C
> index e69de29bb2d..920a4ad0c6d 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted58.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted58.C
> @@ -0,0 +1,22 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M() = default;
> +  M& operator=(M&);
> +};
> +
> +template<typename T> struct W
> +{
> +  W() = default;
> +  W& operator=(const W&) = default; // { dg-error "binding" }
> +  T t;
> +};
> +
> +int
> +main ()
> +{
> +  W<M> w1, w2;
> +  w1 = w2; // { dg-error "use of deleted function" }
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted59.C gcc/testsuite/g++.dg/cpp0x/defaulted59.C
> index e69de29bb2d..4f871d7f5b1 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted59.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted59.C
> @@ -0,0 +1,12 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M(M&) = default;
> +};
> +
> +struct W : public M
> +{
> +  W(const W&) = default;
> +};
> diff --git gcc/testsuite/g++.dg/cpp0x/defaulted60.C gcc/testsuite/g++.dg/cpp0x/defaulted60.C
> index e69de29bb2d..ad025236cce 100644
> --- gcc/testsuite/g++.dg/cpp0x/defaulted60.C
> +++ gcc/testsuite/g++.dg/cpp0x/defaulted60.C
> @@ -0,0 +1,18 @@
> +// Core Issue #1331 (const mismatch with defaulted copy constructor)
> +// { dg-do compile { target c++11 } }
> +
> +struct M
> +{
> +  M(M&);
> +};
> +
> +struct W
> +{
> +  W();
> +  W(const W&);
> +  M m;
> +};
> +
> +// Not first declaration.
> +W::W(const W&) = default; // { dg-error "binding" }
> +W w;

Patch

diff --git gcc/cp/class.c gcc/cp/class.c
index fbf39035e18..b6e78c6377d 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -5660,9 +5660,9 @@  check_bases_and_members (tree t)
 
 	    if (fn_const_p && !imp_const_p)
 	      /* If the function is defaulted outside the class, we just
-		 give the synthesis error.  */
-	      error ("%q+D declared to take const reference, but implicit "
-		     "declaration would take non-const", fn);
+		 give the synthesis error.  Core Issue #1331 says this is
+		 no longer ill-formed, it is defined as deleted instead.  */
+	      DECL_DELETED_FN (fn) = true;
 	  }
 	defaulted_late_check (fn);
       }
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted15.C gcc/testsuite/g++.dg/cpp0x/defaulted15.C
index fabcc23a150..1e0b3545840 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted15.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted15.C
@@ -48,8 +48,7 @@  struct F
 
 struct G: public F
 {
-  // Can't be const because F copy ctor isn't.
-  G(const G&) = default;	// { dg-error "const" }
+  G(const G&) = default;
 };
 
 struct H
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted52.C gcc/testsuite/g++.dg/cpp0x/defaulted52.C
index e69de29bb2d..c617230b493 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted52.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted52.C
@@ -0,0 +1,20 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M();
+  // So that W wouldn't have had "const W&" copy ctor if it were
+  // implicitly declared.
+  M(M&);
+};
+
+template<typename T> struct W
+{
+  W();
+  // This should now compile and be =deleted.
+  W(const W&) = default;
+  T t;
+};
+
+W<M> w;
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted53.C gcc/testsuite/g++.dg/cpp0x/defaulted53.C
index e69de29bb2d..8147e7e2ad1 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted53.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted53.C
@@ -0,0 +1,35 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M& operator=(M&);
+};
+
+struct R
+{
+  R& operator=(R&) = default;
+  M m;
+};
+
+struct S
+{
+  S& operator=(const S&) = default;
+  M m;
+};
+
+struct T
+{
+  // If F is an assignment operator, and the return type of T1
+  // differs from the return type of T2 the program is ill-formed.
+  T operator=(T&) = default; // { dg-error "defaulted" }
+  M m;
+};
+
+struct U
+{
+  // If F is an assignment operator, and T1's parameter type is
+  // not a reference, the program is ill-formed.
+  U& operator=(U) = default; // { dg-error "defaulted" }
+  M m;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted54.C gcc/testsuite/g++.dg/cpp0x/defaulted54.C
index e69de29bb2d..f8ddc4e47ce 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted54.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted54.C
@@ -0,0 +1,18 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M();
+  M(M&);
+};
+
+template<typename T> struct W
+{
+  W();
+  W(const W&) = default; // { dg-error "binding" }
+  T t;
+};
+
+W<M> w;
+W<M> w2 = w; // { dg-error "use of deleted function" }
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted55.C gcc/testsuite/g++.dg/cpp0x/defaulted55.C
index e69de29bb2d..04cfc172000 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted55.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted55.C
@@ -0,0 +1,19 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M();
+  M(M&);
+};
+
+template<typename T> struct W
+{
+  W();
+  W(W&) = default;
+  // T1 and T2 may have differing ref-qualifiers (copy assign op).
+  constexpr W& operator=(const W&) && = default; // { dg-error "defaulted" "" { target c++11_down } }
+  T t;
+};
+
+W<M> w;
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted56.C gcc/testsuite/g++.dg/cpp0x/defaulted56.C
index e69de29bb2d..e7ce12c5566 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted56.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted56.C
@@ -0,0 +1,25 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+// If T2 (what would be the implicit declaration) has a parameter of
+// type const C&, the corresponding parameter of T1 may be of type C&.
+
+struct S
+{
+  constexpr S(S &) = default;
+};
+
+struct T
+{
+  constexpr T(volatile T &) = default; // { dg-error "defaulted" }
+};
+
+struct U
+{
+  constexpr U(const volatile U &) = default; // { dg-error "defaulted" }
+};
+
+struct V
+{
+  constexpr V(const V &) = default;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted57.C gcc/testsuite/g++.dg/cpp0x/defaulted57.C
index e69de29bb2d..37fb7dd6e1d 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted57.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted57.C
@@ -0,0 +1,25 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+// If T2 (what would be the implicit declaration) has a parameter of
+// type const C&, the corresponding parameter of T1 may be of type C&.
+
+struct S
+{
+  S& operator=(S &) = default;
+};
+
+struct T
+{
+  T& operator=(volatile T &) = default; // { dg-error "defaulted" }
+};
+
+struct U
+{
+  U& operator=(const volatile U &) = default; // { dg-error "defaulted" }
+};
+
+struct V
+{
+  V& operator=(const V &) = default;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted58.C gcc/testsuite/g++.dg/cpp0x/defaulted58.C
index e69de29bb2d..920a4ad0c6d 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted58.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted58.C
@@ -0,0 +1,22 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M() = default;
+  M& operator=(M&);
+};
+
+template<typename T> struct W
+{
+  W() = default;
+  W& operator=(const W&) = default; // { dg-error "binding" }
+  T t;
+};
+
+int
+main ()
+{
+  W<M> w1, w2;
+  w1 = w2; // { dg-error "use of deleted function" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted59.C gcc/testsuite/g++.dg/cpp0x/defaulted59.C
index e69de29bb2d..4f871d7f5b1 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted59.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted59.C
@@ -0,0 +1,12 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M(M&) = default;
+};
+
+struct W : public M
+{
+  W(const W&) = default;
+};
diff --git gcc/testsuite/g++.dg/cpp0x/defaulted60.C gcc/testsuite/g++.dg/cpp0x/defaulted60.C
index e69de29bb2d..ad025236cce 100644
--- gcc/testsuite/g++.dg/cpp0x/defaulted60.C
+++ gcc/testsuite/g++.dg/cpp0x/defaulted60.C
@@ -0,0 +1,18 @@ 
+// Core Issue #1331 (const mismatch with defaulted copy constructor)
+// { dg-do compile { target c++11 } }
+
+struct M
+{
+  M(M&);
+};
+
+struct W
+{
+  W();
+  W(const W&);
+  M m;
+};
+
+// Not first declaration.
+W::W(const W&) = default; // { dg-error "binding" }
+W w;