diff mbox

C++11, implement delegating constructors

Message ID 87aa9z2kco.wl%ville@ville-laptop
State New
Headers show

Commit Message

Ville Voutilainen Sept. 20, 2011, 5:29 p.m. UTC
Tested on Linux/X86-32.

2011-09-20 Ville Voutilainen <ville.voutilainen@gmail.com>
           Implement delegating constructors. Based on an
           original patch by Pedro Lamarao.
           * cp-tree.h (enum cpp0x_warn_str): Add CPP0X_DELEGATING_CTORS.
           * error.c (maybe_warn_cpp0x): Adjust.
           * parser.c (cp_parser_mem_initializer_list): Use it. Diagnose
           multiple initializers if a delegating initializer is present.
           * call.c (build_special_member_call): convert an assert into an if.
           * init.c (perform_target_ctor): New.
           * init.c (emit_mem_initializers): Use it.
           * init.c (expand_member_init): Adjust.
           * init.c (expand_default_init): Adjust.

Comments

Jason Merrill Sept. 20, 2011, 9:05 p.m. UTC | #1
On 09/20/2011 01:29 PM, Ville Voutilainen wrote:
> 2011-09-20 Ville Voutilainen <ville.voutilainen@gmail.com>
>            Implement delegating constructors. Based on an
>            original patch by Pedro Lamarao.

Looks good, just a few minor comments:

> -                   build2 (EQ_EXPR, boolean_type_node,
> -                           current_in_charge_parm, integer_zero_node),
> -                   current_vtt_parm,
> -                   vtt);
> +                    build2 (EQ_EXPR, boolean_type_node,
> +                            current_in_charge_parm, integer_zero_node),
> +                    current_vtt_parm,
> +                    vtt);

Try to avoid reformatting lines that you don't actually change.

> +  if (init == void_type_node)
> +    init = NULL_TREE;

This is changing value-initialization to default-initialization, which 
isn't the same thing.  What happens if you just remove these two lines?

> +  finish_expr_stmt (build_aggr_init (decl, init, 0, tf_warning_or_error));

Let's pass LOOKUP_NORMAL instead of 0 here.

> +                                LOOKUP_NONVIRTUAL|LOOKUP_DESTRUCTOR, 0,

And add LOOKUP_NORMAL| to LOOKUP_NONVIRTUAL|LOOKUP_DESTRUCTOR here.

> +  if (mem_inits
> +      && TYPE_P (TREE_PURPOSE (mem_inits))
> +      && same_type_p (TREE_PURPOSE (mem_inits), current_class_type))
> +    {
> +       gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE);
> +       perform_target_ctor (TREE_VALUE (mem_inits));
> +       return;
> +    }
> +
> +

Let's add a comment pointing out that this is constructor delegation. 
And remove the extra blank line.

> +      if (same_type_p (name, current_class_type))
> +       return name;
> +
>        basetype = TYPE_MAIN_VARIANT (name);
>        name = TYPE_NAME (name);
>      }
>    else if (TREE_CODE (name) == TYPE_DECL)
> -    basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
> +    {
> +      if (same_type_p (TREE_TYPE (name), current_class_type))
> +       return TREE_TYPE (name);
> +      basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
> +    }

Instead of comparing to current_class_type in two places, let's compare 
once in the if (basetype) block.

> +      /* delegating constructor */

Generally comments in GCC should start with a capitalized word and end 
with a period even if they aren't full sentences.

> +         error ("seeing initializer for member %<%D%>; "
> +                "previous target constructor for %T must be sole initializer",

Let's word this "mem-initializer for %qD follows constructor delegation"

> +             error ("target constructor for %T must be sole initializer; "
> +                    "saw previous initializer for member %<%D%>",

and "constructor delegation follows mem-initializer for %qD"

We don't need to name the type again, I think.

Jason
diff mbox

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c707d66..c08b48c 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7048,19 +7048,20 @@  build_special_member_call (tree instance, tree name, VEC(tree,gc) **args,
       vtt = DECL_CHAIN (CLASSTYPE_VTABLES (current_class_type));
       vtt = decay_conversion (vtt);
       vtt = build3 (COND_EXPR, TREE_TYPE (vtt),
-		    build2 (EQ_EXPR, boolean_type_node,
-			    current_in_charge_parm, integer_zero_node),
-		    current_vtt_parm,
-		    vtt);
-      gcc_assert (BINFO_SUBVTT_INDEX (binfo));
-      sub_vtt = fold_build_pointer_plus (vtt, BINFO_SUBVTT_INDEX (binfo));
-
+                    build2 (EQ_EXPR, boolean_type_node,
+                            current_in_charge_parm, integer_zero_node),
+                    current_vtt_parm,
+                    vtt);
+      if (BINFO_SUBVTT_INDEX (binfo))
+        sub_vtt = fold_build_pointer_plus (vtt, BINFO_SUBVTT_INDEX (binfo));
+      else
+        sub_vtt = vtt;
       if (args == NULL)
-	{
-	  allocated = make_tree_vector ();
-	  args = &allocated;
-	}
-
+        {
+          allocated = make_tree_vector ();
+          args = &allocated;
+        }
+      
       VEC_safe_insert (tree, gc, *args, 0, sub_vtt);
     }
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ae4cd07..31c600e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -394,7 +394,9 @@  typedef enum cpp0x_warn_str
   /* inline namespaces */
   CPP0X_INLINE_NAMESPACES,
   /* override controls, override/final */
-  CPP0X_OVERRIDE_CONTROLS
+  CPP0X_OVERRIDE_CONTROLS,
+  /* delegating constructors */
+  CPP0X_DELEGATING_CTORS
 } cpp0x_warn_str;
   
 /* The various kinds of operation used by composite_pointer_type. */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 598ddf1..82f3175 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3236,6 +3236,11 @@  maybe_warn_cpp0x (cpp0x_warn_str str)
 		 "override controls (override/final) "
 		 "only available with -std=c++0x or -std=gnu++0x");
         break;
+      case CPP0X_DELEGATING_CTORS:
+	pedwarn (input_location, 0,
+		 "delegating constructors "
+		 "only available with -std=c++0x or -std=gnu++0x");
+        break;
       default:
 	gcc_unreachable();
       }
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index ff1884b..b70dd17 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -482,6 +482,30 @@  build_value_init_noctor (tree type, tsubst_flags_t complain)
   return build_zero_init (type, NULL_TREE, /*static_storage_p=*/false);
 }
 
+/* Initialize current class with INIT, a TREE_LIST of
+   arguments for a target constructor. If TREE_LIST is void_type_node,
+   an empty initializer list was given.  */
+
+static void
+perform_target_ctor (tree init)
+{
+  tree decl = current_class_ref;
+  tree type = current_class_type;
+
+  if (init == void_type_node)
+    init = NULL_TREE;
+  finish_expr_stmt (build_aggr_init (decl, init, 0, tf_warning_or_error));
+  if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
+    {
+      tree expr = build_delete (type, decl, sfk_complete_destructor,
+                                LOOKUP_NONVIRTUAL|LOOKUP_DESTRUCTOR, 0,
+                                tf_warning_or_error);
+      if (expr != error_mark_node)
+	finish_eh_cleanup (expr);
+    }
+}
+
+
 /* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
    arguments.  If TREE_LIST is void_type_node, an empty initializer
    list was given; if NULL_TREE no initializer was given.  */
@@ -924,6 +948,16 @@  emit_mem_initializers (tree mem_inits)
   if (!COMPLETE_TYPE_P (current_class_type))
     return;
 
+  if (mem_inits
+      && TYPE_P (TREE_PURPOSE (mem_inits))
+      && same_type_p (TREE_PURPOSE (mem_inits), current_class_type))
+    {
+	gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE);
+	perform_target_ctor (TREE_VALUE (mem_inits));
+	return;
+    }
+
+
   if (DECL_DEFAULTED_FN (current_function_decl))
     flags |= LOOKUP_DEFAULTED;
 
@@ -1239,11 +1273,18 @@  expand_member_init (tree name)
     }
   else if (TYPE_P (name))
     {
+      if (same_type_p (name, current_class_type))
+	return name;
+
       basetype = TYPE_MAIN_VARIANT (name);
       name = TYPE_NAME (name);
     }
   else if (TREE_CODE (name) == TYPE_DECL)
-    basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
+    {
+      if (same_type_p (TREE_TYPE (name), current_class_type))
+	return TREE_TYPE (name);
+      basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
+    }
   else
     basetype = NULL_TREE;
 
@@ -1494,13 +1535,34 @@  expand_default_init (tree binfo, tree true_exp, tree exp, tree init, int flags,
   else
     parms = make_tree_vector_single (init);
 
-  if (true_exp == exp)
-    ctor_name = complete_ctor_identifier;
+  if (exp == current_class_ref && current_function_decl
+      && DECL_HAS_IN_CHARGE_PARM_P (current_function_decl))
+    {
+      /* delegating constructor */
+      tree complete;
+      tree base;
+      complete = build_special_member_call (exp, complete_ctor_identifier, 
+                                        &parms, binfo, flags,
+                                        complain);
+      base = build_special_member_call (exp, base_ctor_identifier, 
+                                        &parms, binfo, flags,
+                                        complain);
+      rval = build3 (COND_EXPR, TREE_TYPE (complete),
+		    build2 (EQ_EXPR, boolean_type_node,
+			    current_in_charge_parm, integer_zero_node),
+		    base,
+		    complete);
+    }
   else
-    ctor_name = base_ctor_identifier;
+    {
+      if (true_exp == exp)
+        ctor_name = complete_ctor_identifier;
+      else
+        ctor_name = base_ctor_identifier;
+      rval = build_special_member_call (exp, ctor_name, &parms, binfo, flags,
+                                        complain);
+  }
 
-  rval = build_special_member_call (exp, ctor_name, &parms, binfo, flags,
-                                    complain);
 
   if (parms != NULL)
     release_tree_vector (parms);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 03f75fc..02feb36 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10665,6 +10665,8 @@  static void
 cp_parser_mem_initializer_list (cp_parser* parser)
 {
   tree mem_initializer_list = NULL_TREE;
+  tree target_ctor = error_mark_node;
+
   cp_token *token = cp_lexer_peek_token (parser->lexer);
 
   /* Let the semantic analysis code know that we are starting the
@@ -10702,6 +10704,31 @@  cp_parser_mem_initializer_list (cp_parser* parser)
           if (mem_initializer != error_mark_node)
             mem_initializer = make_pack_expansion (mem_initializer);
         }
+      if (target_ctor != error_mark_node
+	  && mem_initializer != error_mark_node)
+	{
+	  error ("seeing initializer for member %<%D%>; "
+		 "previous target constructor for %T must be sole initializer",
+		 TREE_PURPOSE (mem_initializer),
+		 TREE_PURPOSE (target_ctor));
+	  mem_initializer = error_mark_node;
+	}
+      /* Look for a target constructor. */
+      if (mem_initializer != error_mark_node
+	  && TYPE_P (TREE_PURPOSE (mem_initializer))
+	  && same_type_p (TREE_PURPOSE (mem_initializer), current_class_type))
+	{
+          maybe_warn_cpp0x (CPP0X_DELEGATING_CTORS);
+	  if (mem_initializer_list)
+	    {
+	      error ("target constructor for %T must be sole initializer; "
+		     "saw previous initializer for member %<%D%>",
+		     TREE_PURPOSE (mem_initializer),
+		     TREE_PURPOSE (mem_initializer_list));
+	      mem_initializer = error_mark_node;
+	    }
+	  target_ctor = mem_initializer;
+	}
       /* Add it to the list, unless it was erroneous.  */
       if (mem_initializer != error_mark_node)
 	{
diff --git a/gcc/testsuite/g++.dg/cpp0x/dc1.C b/gcc/testsuite/g++.dg/cpp0x/dc1.C
new file mode 100644
index 0000000..ba2e4f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/dc1.C
@@ -0,0 +1,43 @@ 
+// { dg-do compile }
+// { dg-options --std=c++0x }
+
+struct B {
+	int i;
+	B (int _i) : i(_i) { }
+	~B () { i = 0; }
+};
+
+struct A : public B {
+	A () : B(-1) { }
+	A (int i) : A() { }
+	A (double b) : A(static_cast<int>(b)) { }
+	A (double b, double b2) : A(b2) { }
+	~A () { }
+};
+
+void f_A () { A a(2.0, 3.0); }
+
+struct C {
+	C () { }
+	virtual ~C() { }
+	virtual int f () = 0;
+};
+
+struct D : public C {
+	int i;
+	D (int _i) : C(), i(_i) { }
+	D () : D(-1) { }
+	virtual ~D() { }
+	virtual int f () { }
+};
+
+void f_D () { C* c = new D(); }
+
+template <typename T>
+struct E {
+	T t;
+	E () : E(T()) { }
+	E (T _t) : t(_t) { }
+};
+
+void f_E () { E<int> e; }
diff --git a/gcc/testsuite/g++.dg/cpp0x/dc2.C b/gcc/testsuite/g++.dg/cpp0x/dc2.C
new file mode 100644
index 0000000..aba5cea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/dc2.C
@@ -0,0 +1,23 @@ 
+// { dg-do compile }
+// { dg-options --std=c++0x }
+
+struct A {
+	int i, j;
+	A () : A(0), j(0) { } // { dg-error "" "only initializer" }
+	A (int _i) : i(_i) { }
+};
+
+struct B {
+	int i, j;
+	B () : i(0), B(0) { } // { dg-error "" "only initializer" }
+	B (int _j) : j(_j) { }
+
+};
+
+struct C {};
+
+struct D : public C {
+	D () : C() { }
+	D (float) : D(), C() { } // { dg-error "" "only initializer" }
+	D (float, float): C(), D() { } // { dg-error "" "only initializer" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp0x/dc3.C b/gcc/testsuite/g++.dg/cpp0x/dc3.C
new file mode 100644
index 0000000..b411c99
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/dc3.C
@@ -0,0 +1,63 @@ 
+// { dg-do compile }
+// { dg-options --std=c++0x }
+
+struct X {};
+
+struct B {
+	int i;
+	B (int _i) : i(_i) { }
+	~B () { i = 0; }
+};
+
+template <typename T>
+struct A : public B {
+	A () : B(-1) { }
+	~A () { }
+};
+
+template <typename T>
+struct A<T*> : public B {
+	A () : B(-1) { }
+	A (int i) : A() { }
+	A (double b) : A(static_cast<int>(b)) { }
+	A (double b, double b2) : A(b2) { }
+	~A () { }
+};
+
+void f_A () { A<X*> a(2.0, 3.0); }
+
+struct C {
+	C () { }
+	virtual ~C() { }
+	virtual int f () = 0;
+};
+
+template <typename T>
+struct D : public C {
+	int i;
+	D (int _i) : C(), i(_i) { }
+};
+
+template <>
+struct D<X> : public C {
+	int i;
+	D (int _i) : C(), i(_i) { }
+	D () : D(-1) { }
+	virtual ~D() { }
+	virtual int f () { }
+};
+
+void f_D () { D<X>* d = new D<X>(); }
+
+template <typename T>
+struct E {
+};
+
+template <>
+struct E<int> {
+	int i;
+	E () : E(0) { }
+	E (int _i) : i(_i) { }
+};
+
+void f_E () { E<int> e; }
diff --git a/gcc/testsuite/g++.dg/cpp0x/dc4.C b/gcc/testsuite/g++.dg/cpp0x/dc4.C
new file mode 100644
index 0000000..634b549
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/dc4.C
@@ -0,0 +1,7 @@ 
+// { dg-do compile }
+// { dg-options "--std=c++98" }
+
+struct X {
+  X() {}
+  X(int) : X() {} // { dg-warning "delegating constructors" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp0x/dc5.C b/gcc/testsuite/g++.dg/cpp0x/dc5.C
new file mode 100644
index 0000000..0052b32
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/dc5.C
@@ -0,0 +1,28 @@ 
+// { dg-do run }
+// { dg-options "--std=c++0x" }
+
+#include <cassert>
+
+int count = 0;
+struct VB
+{
+  VB() {++count;}
+};
+
+struct B : virtual VB
+{
+  B() : B(42) {}
+  B(int)  {}
+};
+
+struct D : B
+{
+  D() {}
+  D(int) : D() {}
+};
+
+int main()
+{
+  D d{42};
+  assert(count == 1);
+}
diff --git a/gcc/testsuite/g++.dg/template/meminit1.C b/gcc/testsuite/g++.dg/template/meminit1.C
index b1c4d42..fc31828 100644
--- a/gcc/testsuite/g++.dg/template/meminit1.C
+++ b/gcc/testsuite/g++.dg/template/meminit1.C
@@ -2,7 +2,7 @@ 
 template <class T >
 struct S
 {
-  S() : S() {} // { dg-error "base" }
+  S() : S() {}
 };
 
-S<int> s; // { dg-message "required" }
+S<int> s; // { dg-warning "delegating constructors" }