diff mbox

C++ PATCH for c++/44909 (confusion with circular lazy declaration)

Message ID 4C3CE8DF.7010504@redhat.com
State New
Headers show

Commit Message

Jason Merrill July 13, 2010, 10:29 p.m. UTC
In this testcase, trying to lazily declare the Ray default constructor 
means lazily declaring the array copy constructor, which means seeing 
which Vector constructor will be used for the copy, which means seeing 
if there's a conversion from Vector to Ray, which means lazily declaring 
the Ray copy constructor, which means failing to lazily declare the 
array copy constructor.

This patch avoids this issue by immediately declaring the constructors 
for array; at that point, Ray isn't a complete type, so there is no 
conversion from Vector to Ray, so the cycle is broken.

Tested x86_64-pc-linux-gnu, applied to trunk.

Comments

Iain Sandoe July 15, 2010, 4:38 p.m. UTC | #1
On 13 Jul 2010, at 23:29, Jason Merrill wrote:

> In this testcase, trying to lazily declare the Ray default  
> constructor means lazily declaring the array copy constructor, which  
> means seeing which Vector constructor will be used for the copy,  
> which means seeing if there's a conversion from Vector to Ray, which  
> means lazily declaring the Ray copy constructor, which means failing  
> to lazily declare the array copy constructor.
>
> This patch avoids this issue by immediately declaring the  
> constructors for array; at that point, Ray isn't a complete type, so  
> there is no conversion from Vector to Ray, so the cycle is broken.
>
> Tested x86_64-pc-linux-gnu, applied to trunk.
> commit a2a65b8045e79a850558fe2e9157e224abf5f924
> Author: Jason Merrill <jason@redhat.com>
> Date:   Tue Jul 13 15:18:59 2010 -0400
>
>    	PR c++/44909
>    	* cp-tree.h (struct lang_type_class): Add has_user_opeq.
>    	(TYPE_HAS_USER_OPEQ): New.
>    	* decl.c (grok_special_member_properties): Set it.
>    	* class.c (add_implicitly_declared_members): Don't lazily declare
>    	constructors/operator= if a base or member has a user-declared  
> one.
>    	(check_bases_and_members, check_bases): Adjust.
>    	(check_field_decls, check_field_decl): Adjust.
>    	* method.c (synthesized_method_walk): Initialize check_vdtor.

this causes two ObjC++ tests to regress.
FAIL: obj-c++.dg/cxx-ivars-{2,3}.mm -fnext-runtime (internal compiler  
error)

The ICE is because objc-act.c [895:918]  assumes that the length of  
the TYPE_NEXT_VARIANT() list will not change across a call to  
finish_struct();

Looking at 162148 [working] and 162149 [fail]
is seems to me that, in 162149,  an identical copy of the first type  
in the list is being pre-pended to the list (or the initial entry is  
duplicated, I guess).

At the moment, I can't be sure whether:

(a) that is a valid consequence of the intention of the change (and  
thus ObjC needs to be amended to use a different strategy for what  
it's doing)

(b) there is some other explanation.


cheers,
Iain
Jason Merrill July 15, 2010, 8:25 p.m. UTC | #2
I'm about to revert that patch.

Jason
diff mbox

Patch

commit a2a65b8045e79a850558fe2e9157e224abf5f924
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jul 13 15:18:59 2010 -0400

    	PR c++/44909
    	* cp-tree.h (struct lang_type_class): Add has_user_opeq.
    	(TYPE_HAS_USER_OPEQ): New.
    	* decl.c (grok_special_member_properties): Set it.
    	* class.c (add_implicitly_declared_members): Don't lazily declare
    	constructors/operator= if a base or member has a user-declared one.
    	(check_bases_and_members, check_bases): Adjust.
    	(check_field_decls, check_field_decl): Adjust.
    	* method.c (synthesized_method_walk): Initialize check_vdtor.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index dfb2cd9..ed7367c 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -130,7 +130,7 @@  static void finish_struct_methods (tree);
 static void maybe_warn_about_overly_private_class (tree);
 static int method_name_cmp (const void *, const void *);
 static int resort_method_name_cmp (const void *, const void *);
-static void add_implicitly_declared_members (tree, int, int);
+static void add_implicitly_declared_members (tree, int, int, int, int);
 static tree fixed_type_or_null (tree, int *, int *);
 static tree build_simple_base_path (tree expr, tree binfo);
 static tree build_vtbl_ref_1 (tree, tree);
@@ -139,13 +139,13 @@  static void build_vtbl_initializer (tree, tree, tree, tree, int *,
 static int count_fields (tree);
 static int add_fields_to_record_type (tree, struct sorted_fields_type*, int);
 static bool check_bitfield_decl (tree);
-static void check_field_decl (tree, tree, int *, int *, int *);
-static void check_field_decls (tree, tree *, int *, int *);
+static void check_field_decl (tree, tree, int *, int *, int *, int *, int *);
+static void check_field_decls (tree, tree *, int *, int *, int *, int *);
 static tree *build_base_field (record_layout_info, tree, splay_tree, tree *);
 static void build_base_fields (record_layout_info, splay_tree, tree *);
 static void check_methods (tree);
 static void remove_zero_width_bit_fields (tree);
-static void check_bases (tree, int *, int *);
+static void check_bases (tree, int *, int *, int *, int *);
 static void check_bases_and_members (tree);
 static tree create_vtable_ptr (tree, tree *);
 static void include_empty_classes (record_layout_info);
@@ -1249,7 +1249,9 @@  handle_using_decl (tree using_decl, tree t)
 static void
 check_bases (tree t,
 	     int* cant_have_const_ctor_p,
-	     int* no_const_asn_ref_p)
+	     int* no_const_asn_ref_p,
+	     int* cant_have_lazy_ctor,
+	     int* cant_have_lazy_opeq)
 {
   int i;
   int seen_non_virtual_nearly_empty_base_p;
@@ -1288,6 +1290,10 @@  check_bases (tree t,
       if (TYPE_HAS_COPY_ASSIGN (basetype)
 	  && !TYPE_HAS_CONST_COPY_ASSIGN (basetype))
 	*no_const_asn_ref_p = 1;
+      if (TYPE_HAS_USER_CONSTRUCTOR (basetype))
+	*cant_have_lazy_ctor = 1;
+      if (TYPE_HAS_USER_OPEQ (basetype))
+	*cant_have_lazy_opeq = 1;
 
       if (BINFO_VIRTUAL_P (base_binfo))
 	/* A virtual base does not effect nearly emptiness.  */
@@ -2628,7 +2634,9 @@  maybe_add_class_template_decl_list (tree type, tree t, int friend_p)
 static void
 add_implicitly_declared_members (tree t,
 				 int cant_have_const_cctor,
-				 int cant_have_const_assignment)
+				 int cant_have_const_assignment,
+				 int cant_have_lazy_ctor,
+				 int cant_have_lazy_opeq)
 {
   /* Destructor.  */
   if (!CLASSTYPE_DESTRUCTORS (t))
@@ -2682,6 +2690,26 @@  add_implicitly_declared_members (tree t,
 	CLASSTYPE_LAZY_MOVE_ASSIGN (t) = 1;
     }
 
+  /* If a base or member type has a user-declared constructor or operator=,
+     we need to declare ours now to avoid issues with circular lazy
+     declarations (cpp0x/implicit6.C).  */
+  if (cant_have_lazy_ctor)
+    {
+      if (CLASSTYPE_LAZY_DEFAULT_CTOR (t))
+	lazily_declare_fn (sfk_constructor, t);
+      if (CLASSTYPE_LAZY_COPY_CTOR (t))
+	lazily_declare_fn (sfk_copy_constructor, t);
+      if (CLASSTYPE_LAZY_MOVE_CTOR (t))
+	lazily_declare_fn (sfk_move_constructor, t);
+    }
+  if (cant_have_lazy_opeq)
+    {
+      if (CLASSTYPE_LAZY_COPY_ASSIGN (t))
+	lazily_declare_fn (sfk_copy_assignment, t);
+      if (CLASSTYPE_LAZY_MOVE_ASSIGN (t))
+	lazily_declare_fn (sfk_move_assignment, t);
+    }
+
   /* We can't be lazy about declaring functions that might override
      a virtual function from a base class.  */
   if (TYPE_POLYMORPHIC_P (t)
@@ -2830,7 +2858,9 @@  check_field_decl (tree field,
 		  tree t,
 		  int* cant_have_const_ctor,
 		  int* no_const_asn_ref,
-		  int* any_default_members)
+		  int* any_default_members,
+		  int* cant_have_lazy_ctor,
+		  int* cant_have_lazy_opeq)
 {
   tree type = strip_array_types (TREE_TYPE (field));
 
@@ -2847,7 +2877,8 @@  check_field_decl (tree field,
       for (fields = TYPE_FIELDS (type); fields; fields = TREE_CHAIN (fields))
 	if (TREE_CODE (fields) == FIELD_DECL && !DECL_C_BIT_FIELD (field))
 	  check_field_decl (fields, t, cant_have_const_ctor,
-			    no_const_asn_ref, any_default_members);
+			    no_const_asn_ref, any_default_members,
+			    cant_have_lazy_ctor, cant_have_lazy_opeq);
     }
   /* Check members with class type for constructors, destructors,
      etc.  */
@@ -2893,6 +2924,11 @@  check_field_decl (tree field,
       if (TYPE_HAS_COPY_ASSIGN (type)
 	  && !TYPE_HAS_CONST_COPY_ASSIGN (type))
 	*no_const_asn_ref = 1;
+
+      if (TYPE_HAS_USER_CONSTRUCTOR (type))
+	*cant_have_lazy_ctor = 1;
+      if (TYPE_HAS_USER_OPEQ (type))
+	*cant_have_lazy_opeq = 1;
     }
   if (DECL_INITIAL (field) != NULL_TREE)
     {
@@ -2932,7 +2968,9 @@  check_field_decl (tree field,
 static void
 check_field_decls (tree t, tree *access_decls,
 		   int *cant_have_const_ctor_p,
-		   int *no_const_asn_ref_p)
+		   int *no_const_asn_ref_p,
+		   int *cant_have_lazy_ctor_p,
+		   int *cant_have_lazy_opeq_p)
 {
   tree *field;
   tree *next;
@@ -3124,7 +3162,9 @@  check_field_decls (tree t, tree *access_decls,
 	check_field_decl (x, t,
 			  cant_have_const_ctor_p,
 			  no_const_asn_ref_p,
-			  &any_default_members);
+			  &any_default_members,
+			  cant_have_lazy_ctor_p,
+			  cant_have_lazy_opeq_p);
 
       /* If any field is const, the structure type is pseudo-const.  */
       if (CP_TYPE_CONST_P (type))
@@ -4447,6 +4487,8 @@  check_bases_and_members (tree t)
   /* Nonzero if the implicitly generated assignment operator
      should take a non-const reference argument.  */
   int no_const_asn_ref;
+  int cant_have_lazy_ctor = 0;
+  int cant_have_lazy_opeq = 0;
   tree access_decls;
   bool saved_complex_asn_ref;
   bool saved_nontrivial_dtor;
@@ -4459,7 +4501,8 @@  check_bases_and_members (tree t)
 
   /* Check all the base-classes.  */
   check_bases (t, &cant_have_const_ctor,
-	       &no_const_asn_ref);
+	       &no_const_asn_ref, &cant_have_lazy_ctor,
+	       &cant_have_lazy_opeq);
 
   /* Check all the method declarations.  */
   check_methods (t);
@@ -4476,7 +4519,9 @@  check_bases_and_members (tree t)
      being set appropriately.  */
   check_field_decls (t, &access_decls,
 		     &cant_have_const_ctor,
-		     &no_const_asn_ref);
+		     &no_const_asn_ref,
+		     &cant_have_lazy_ctor,
+		     &cant_have_lazy_opeq);
 
   /* A nearly-empty class has to be vptr-containing; a nearly empty
      class contains just a vptr.  */
@@ -4548,7 +4593,9 @@  check_bases_and_members (tree t)
   /* Synthesize any needed methods.  */
   add_implicitly_declared_members (t,
 				   cant_have_const_ctor,
-				   no_const_asn_ref);
+				   no_const_asn_ref,
+				   cant_have_lazy_ctor,
+				   cant_have_lazy_opeq);
 
   /* Check defaulted declarations here so we have cant_have_const_ctor
      and don't need to worry about clones.  */
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index cf128dc..8b076d3 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1326,6 +1326,7 @@  struct GTY(()) lang_type_class {
   unsigned lazy_move_assign : 1;
   unsigned has_complex_move_ctor : 1;
   unsigned has_complex_move_assign : 1;
+  unsigned has_user_opeq : 1;
 
   /* When adding a flag here, consider whether or not it ought to
      apply to a template instance if it applies to the template.  If
@@ -1334,7 +1335,7 @@  struct GTY(()) lang_type_class {
   /* There are some bits left to fill out a 32-bit word.  Keep track
      of this by updating the size of this bitfield whenever you add or
      remove a flag.  */
-  unsigned dummy : 4;
+  unsigned dummy : 3;
 
   tree primary_base;
   VEC(tree_pair_s,gc) *vcall_indices;
@@ -3142,6 +3143,10 @@  more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
    user-declared constructor.  */
 #define TYPE_HAS_USER_CONSTRUCTOR(NODE) (TYPE_LANG_FLAG_1 (NODE))
 
+/* ...or a user-declared operator=.  */
+#define TYPE_HAS_USER_OPEQ(NODE) \
+  (LANG_TYPE_CLASS_CHECK (NODE)->has_user_opeq)
+
 /* When appearing in an INDIRECT_REF, it means that the tree structure
    underneath is actually a call to a constructor.  This is needed
    when the constructor must initialize local storage (which can
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 541f77e..1491720 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10293,6 +10293,8 @@  grok_special_member_properties (tree decl)
 
       int assop = copy_fn_p (decl);
 
+      if (!DECL_ARTIFICIAL (decl))
+	TYPE_HAS_USER_OPEQ (class_type) = 1;
       if (assop)
 	{
 	  TYPE_HAS_COPY_ASSIGN (class_type) = 1;
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index ad41e9a..b09064b 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1004,6 +1004,7 @@  synthesized_method_walk (tree ctype, special_function_kind sfk, bool const_p,
 #endif
 
   assign_p = false;
+  check_vdtor = false;
   switch (sfk)
     {
     case sfk_move_assignment:
diff --git a/gcc/testsuite/g++.dg/cpp0x/implicit6.C b/gcc/testsuite/g++.dg/cpp0x/implicit6.C
new file mode 100644
index 0000000..e517333
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/implicit6.C
@@ -0,0 +1,25 @@ 
+// Circular implicit declarations were causing errors
+// { dg-options -std=c++0x }
+
+struct Ray;
+
+struct Vector
+{
+  virtual void f();		// make non-trivially-copyable
+  Vector();
+  Vector(const Ray &) ;
+};
+
+struct array
+{
+  Vector v;
+};
+
+struct Ray
+{
+  array a;
+  operator Vector();
+};
+
+extern Ray r1;
+Ray r2=r1;
diff --git a/gcc/testsuite/g++.dg/parse/error28.C b/gcc/testsuite/g++.dg/parse/error28.C
index 7e235a1..a0b1e7f 100644
--- a/gcc/testsuite/g++.dg/parse/error28.C
+++ b/gcc/testsuite/g++.dg/parse/error28.C
@@ -3,7 +3,7 @@ 
 
 struct virt { virt () {} virt (int i) {} };
 struct der : public virtual virt { // { dg-message "8:der::der" }
-  der (int i) : virt(i) {} // { dg-message "3:candidates are: der" }
+  der (int i) : virt(i) {} // { dg-message "3:der::der" }
 };
 struct top : public der { 
   top () {} // { dg-bogus "der\\(const" }