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

login
register
mail settings
Submitter Jason Merrill
Date July 15, 2010, 8:45 p.m.
Message ID <4C3F7363.3020006@redhat.com>
Download mbox | patch
Permalink /patch/59046/
State New
Headers show

Comments

Jason Merrill - July 15, 2010, 8:45 p.m.
On 07/15/2010 04:25 PM, Jason Merrill wrote:
> I'm about to revert that patch.

As promised, here's a completely different approach: rather than rely on 
the incompleteness of the parameter type, we just disqualify any 
constructor taking a different type.  This isn't quite right, but it 
avoids a regression, and a proper fix will need more discussion.

Tested x86_64-pc-linux-gnu, applied to trunk.
IainS - July 15, 2010, 11:17 p.m.
On 15 Jul 2010, at 21:45, Jason Merrill wrote:

> On 07/15/2010 04:25 PM, Jason Merrill wrote:
>> I'm about to revert that patch.
>
> As promised, here's a completely different approach: rather than  
> rely on the incompleteness of the parameter type, we just disqualify  
> any constructor taking a different type.  This isn't quite right,  
> but it avoids a regression, and a proper fix will need more  
> discussion.

thanks, as you say that squashes the ObjC++ regression.

I'm trying to review what's being done in ObjC (on the basis that some  
of it might no longer be correct/optimal).

Iain.

Patch

commit fa69ebd425b755200d7e6bc0248eebda674661ea
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jul 15 16:36:54 2010 -0400

    	PR c++/44909
    	* call.c (add_function_candidate): If we're working on an implicit
    	declaration, don't consider candidates that won't match.
    	* typeck.c (same_type_ignoring_top_level_qualifiers_p): Now a fn.
    	* cp-tree.h (same_type_ignoring_top_level_qualifiers_p): Adjust.
    
    	Revert:
    	* 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.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 1c64149..d2f3a3e 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -1596,6 +1596,27 @@  add_function_candidate (struct z_candidate **candidates,
   else if (!sufficient_parms_p (parmnode))
     viable = 0;
 
+  /* Kludge: When looking for a function from a subobject while generating
+     an implicit copy/move constructor/operator=, don't consider anything
+     that takes (a reference to) a different type.  See c++/44909.  */
+  else if (flags & LOOKUP_SPECULATIVE)
+    {
+      if (DECL_CONSTRUCTOR_P (fn))
+	i = 1;
+      else if (DECL_ASSIGNMENT_OPERATOR_P (fn)
+	       && DECL_OVERLOADED_OPERATOR_P (fn) == NOP_EXPR)
+	i = 2;
+      else
+	i = 0;
+      if (i && len == i)
+	{
+	  parmnode = chain_index (i-1, parmlist);
+	  if (!(same_type_ignoring_top_level_qualifiers_p
+		(non_reference (TREE_VALUE (parmnode)), ctype)))
+	    viable = 0;
+	}
+    }
+
   if (! viable)
     goto out;
 
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index a572af8..437269f 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, int, int);
+static void add_implicitly_declared_members (tree, 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 *, int *, int *);
-static void check_field_decls (tree, tree *, int *, int *, int *, int *);
+static void check_field_decl (tree, tree, int *, int *, int *);
+static void check_field_decls (tree, tree *, 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 *, int *, int *);
+static void check_bases (tree, 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,9 +1249,7 @@  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* cant_have_lazy_ctor,
-	     int* cant_have_lazy_opeq)
+	     int* no_const_asn_ref_p)
 {
   int i;
   int seen_non_virtual_nearly_empty_base_p;
@@ -1290,10 +1288,6 @@  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.  */
@@ -2634,9 +2628,7 @@  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_lazy_ctor,
-				 int cant_have_lazy_opeq)
+				 int cant_have_const_assignment)
 {
   /* Destructor.  */
   if (!CLASSTYPE_DESTRUCTORS (t))
@@ -2690,26 +2682,6 @@  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)
@@ -2858,9 +2830,7 @@  check_field_decl (tree field,
 		  tree t,
 		  int* cant_have_const_ctor,
 		  int* no_const_asn_ref,
-		  int* any_default_members,
-		  int* cant_have_lazy_ctor,
-		  int* cant_have_lazy_opeq)
+		  int* any_default_members)
 {
   tree type = strip_array_types (TREE_TYPE (field));
 
@@ -2877,8 +2847,7 @@  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,
-			    cant_have_lazy_ctor, cant_have_lazy_opeq);
+			    no_const_asn_ref, any_default_members);
     }
   /* Check members with class type for constructors, destructors,
      etc.  */
@@ -2930,11 +2899,6 @@  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)
     {
@@ -2974,9 +2938,7 @@  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 *cant_have_lazy_ctor_p,
-		   int *cant_have_lazy_opeq_p)
+		   int *no_const_asn_ref_p)
 {
   tree *field;
   tree *next;
@@ -3168,9 +3130,7 @@  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,
-			  cant_have_lazy_ctor_p,
-			  cant_have_lazy_opeq_p);
+			  &any_default_members);
 
       /* If any field is const, the structure type is pseudo-const.  */
       if (CP_TYPE_CONST_P (type))
@@ -4493,8 +4453,6 @@  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;
@@ -4507,8 +4465,7 @@  check_bases_and_members (tree t)
 
   /* Check all the base-classes.  */
   check_bases (t, &cant_have_const_ctor,
-	       &no_const_asn_ref, &cant_have_lazy_ctor,
-	       &cant_have_lazy_opeq);
+	       &no_const_asn_ref);
 
   /* Check all the method declarations.  */
   check_methods (t);
@@ -4525,9 +4482,7 @@  check_bases_and_members (tree t)
      being set appropriately.  */
   check_field_decls (t, &access_decls,
 		     &cant_have_const_ctor,
-		     &no_const_asn_ref,
-		     &cant_have_lazy_ctor,
-		     &cant_have_lazy_opeq);
+		     &no_const_asn_ref);
 
   /* A nearly-empty class has to be vptr-containing; a nearly empty
      class contains just a vptr.  */
@@ -4599,9 +4554,7 @@  check_bases_and_members (tree t)
   /* Synthesize any needed methods.  */
   add_implicitly_declared_members (t,
 				   cant_have_const_ctor,
-				   no_const_asn_ref,
-				   cant_have_lazy_ctor,
-				   cant_have_lazy_opeq);
+				   no_const_asn_ref);
 
   /* 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 8b076d3..b40fa89 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -289,11 +289,6 @@  typedef struct ptrmem_cst * ptrmem_cst_t;
 #define same_type_p(TYPE1, TYPE2) \
   comptypes ((TYPE1), (TYPE2), COMPARE_STRICT)
 
-/* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
-   top-level qualifiers.  */
-#define same_type_ignoring_top_level_qualifiers_p(TYPE1, TYPE2) \
-  same_type_p (TYPE_MAIN_VARIANT (TYPE1), TYPE_MAIN_VARIANT (TYPE2))
-
 /* Nonzero if we are presently building a statement tree, rather
    than expanding each statement as we encounter it.  */
 #define building_stmt_tree()  (cur_stmt_list != NULL_TREE)
@@ -1326,7 +1321,6 @@  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
@@ -1335,7 +1329,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 : 3;
+  unsigned dummy : 4;
 
   tree primary_base;
   VEC(tree_pair_s,gc) *vcall_indices;
@@ -3143,10 +3137,6 @@  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
@@ -5434,6 +5424,7 @@  extern int type_unknown_p			(const_tree);
 enum { ce_derived, ce_normal, ce_exact };
 extern bool comp_except_specs			(const_tree, const_tree, int);
 extern bool comptypes				(tree, tree, int);
+extern bool same_type_ignoring_top_level_qualifiers_p (tree, tree);
 extern bool compparms				(const_tree, const_tree);
 extern int comp_cv_qualification		(const_tree, const_tree);
 extern int comp_cv_qual_signature		(tree, tree);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 1491720..541f77e 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10293,8 +10293,6 @@  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/typeck.c b/gcc/cp/typeck.c
index d5e43de..bbc72bf 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1503,6 +1503,18 @@  comptypes (tree t1, tree t2, int strict)
     return structural_comptypes (t1, t2, strict);
 }
 
+/* Returns nonzero iff TYPE1 and TYPE2 are the same type, ignoring
+   top-level qualifiers.  */
+
+bool
+same_type_ignoring_top_level_qualifiers_p (tree type1, tree type2)
+{
+  if (type1 == error_mark_node || type2 == error_mark_node)
+    return false;
+
+  return same_type_p (TYPE_MAIN_VARIANT (type1), TYPE_MAIN_VARIANT (type2));
+}
+
 /* Returns 1 if TYPE1 is at least as qualified as TYPE2.  */
 
 bool
diff --git a/gcc/testsuite/g++.dg/cpp0x/implicit6.C b/gcc/testsuite/g++.dg/cpp0x/implicit6.C
index e517333..c790296 100644
--- a/gcc/testsuite/g++.dg/cpp0x/implicit6.C
+++ b/gcc/testsuite/g++.dg/cpp0x/implicit6.C
@@ -6,7 +6,6 @@  struct Ray;
 struct Vector
 {
   virtual void f();		// make non-trivially-copyable
-  Vector();
   Vector(const Ray &) ;
 };
 
@@ -18,7 +17,6 @@  struct array
 struct Ray
 {
   array a;
-  operator Vector();
 };
 
 extern Ray r1;
diff --git a/gcc/testsuite/g++.dg/cpp0x/implicit7.C b/gcc/testsuite/g++.dg/cpp0x/implicit7.C
new file mode 100644
index 0000000..f29e500
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/implicit7.C
@@ -0,0 +1,37 @@ 
+// PR c++/44909
+// { dg-options -std=c++0x }
+// Declaring A<D<E>>'s copy ctor means choosing a ctor to initialize D<E>,
+// which means choosing a ctor for C<B<E>>, which meant considering
+// C(const B<E>&) which means choosing a ctor for B<E>, which means choosing
+// a ctor for A<D<E>>.  Cycle.
+
+template<typename T>
+struct A
+{
+  T t;
+};
+
+template <typename T>
+struct B
+{
+  typename T::U u;
+};
+
+template <typename T>
+struct C
+{
+  C(const T&);
+};
+
+template <typename T>
+struct D
+{
+  C<B<T> > v;
+};
+
+struct E {
+  typedef A<D<E> > U;
+};
+
+extern A<D<E> > a;
+A<D<E> > a2(a);
diff --git a/gcc/testsuite/g++.dg/cpp0x/implicit8.C b/gcc/testsuite/g++.dg/cpp0x/implicit8.C
new file mode 100644
index 0000000..2f3feba
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/implicit8.C
@@ -0,0 +1,34 @@ 
+// The hack for PR c++/44909 breaks this testcase.  We need feedback
+// from the C++ committee to know how to proceed.
+// { dg-options -std=c++0x }
+// { dg-prune-output "implicitly deleted" }
+// { dg-prune-output "cannot bind" }
+// { dg-prune-output "initializing argument" }
+
+struct A
+{
+  A();
+  A(A&);
+};
+
+struct B;
+struct BP
+{
+  BP(const B&);
+};
+
+struct B
+{
+  B();
+  B(B&&);
+  B(const BP&);
+};
+
+// If B(B&&) suppresses the B copy constructor, then copying the B
+// subobject of C should use B(const BP&).  But we ignore that constructor
+// in order to break the cycle in 44909.  Perhaps the move ctor shouldn't
+// suppress the copy ctor?
+struct C: A, B { };
+
+C c;
+C c2(c);			// { dg-bogus "deleted" "" { xfail *-*-* } }