Re: [PATCH] Fix PR62077

[Jason Merrill <jason at redhat dot com>]

On 08/14/2014 12:28 PM, Jason Merrill wrote:
> On 08/14/2014 05:07 AM, Richard Biener wrote:
> > So - can you take over this C++ frontend issue?
>
> OK.

Here's what I'm applying to trunk:


commit 76cce1d62861b6c99e3ecd97bcd607cd242d1efa
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Aug 13 13:11:09 2014 -0400

    	PR bootstrap/62077
    	* tree.c (build_min_array_type, set_array_type_canon): Split out...
    	(build_cplus_array_type): ...from here.  Only call build_array_type
    	for main variants.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 3b53039..c9199f2 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -757,7 +757,40 @@ cplus_array_compare (const void * k1, const void * k2)
    the language-independent type hash table.  */
 static GTY ((param_is (union tree_node))) htab_t cplus_array_htab;
 
-/* Like build_array_type, but handle special C++ semantics.  */
+/* Build an ARRAY_TYPE without laying it out.  */
+
+static tree
+build_min_array_type (tree elt_type, tree index_type)
+{
+  tree t = cxx_make_type (ARRAY_TYPE);
+  TREE_TYPE (t) = elt_type;
+  TYPE_DOMAIN (t) = index_type;
+  return t;
+}
+
+/* Set TYPE_CANONICAL like build_array_type_1, but using
+   build_cplus_array_type.  */
+
+static void
+set_array_type_canon (tree t, tree elt_type, tree index_type)
+{
+  /* Set the canonical type for this new node.  */
+  if (TYPE_STRUCTURAL_EQUALITY_P (elt_type)
+      || (index_type && TYPE_STRUCTURAL_EQUALITY_P (index_type)))
+    SET_TYPE_STRUCTURAL_EQUALITY (t);
+  else if (TYPE_CANONICAL (elt_type) != elt_type
+	   || (index_type && TYPE_CANONICAL (index_type) != index_type))
+    TYPE_CANONICAL (t)
+      = build_cplus_array_type (TYPE_CANONICAL (elt_type),
+				index_type
+				? TYPE_CANONICAL (index_type) : index_type);
+  else
+    TYPE_CANONICAL (t) = t;
+}
+
+/* Like build_array_type, but handle special C++ semantics: an array of a
+   variant element type is a variant of the array of the main variant of
+   the element type.  */
 
 tree
 build_cplus_array_type (tree elt_type, tree index_type)
@@ -767,10 +800,19 @@ build_cplus_array_type (tree elt_type, tree index_type)
   if (elt_type == error_mark_node || index_type == error_mark_node)
     return error_mark_node;
 
-  if (processing_template_decl
-      && (dependent_type_p (elt_type)
-	  || (index_type && !TREE_CONSTANT (TYPE_MAX_VALUE (index_type)))))
+  bool dependent
+    = (processing_template_decl
+       && (dependent_type_p (elt_type)
+	   || (index_type && !TREE_CONSTANT (TYPE_MAX_VALUE (index_type)))));
+
+  if (elt_type != TYPE_MAIN_VARIANT (elt_type))
+    /* Start with an array of the TYPE_MAIN_VARIANT.  */
+    t = build_cplus_array_type (TYPE_MAIN_VARIANT (elt_type),
+				index_type);
+  else if (dependent)
     {
+      /* Since type_hash_canon calls layout_type, we need to use our own
+	 hash table.  */
       void **e;
       cplus_array_info cai;
       hashval_t hash;
@@ -792,82 +834,33 @@ build_cplus_array_type (tree elt_type, tree index_type)
       else
 	{
 	  /* Build a new array type.  */
-	  t = cxx_make_type (ARRAY_TYPE);
-	  TREE_TYPE (t) = elt_type;
-	  TYPE_DOMAIN (t) = index_type;
+	  t = build_min_array_type (elt_type, index_type);
 
 	  /* Store it in the hash table. */
 	  *e = t;
 
 	  /* Set the canonical type for this new node.  */
-	  if (TYPE_STRUCTURAL_EQUALITY_P (elt_type)
-	      || (index_type && TYPE_STRUCTURAL_EQUALITY_P (index_type)))
-	    SET_TYPE_STRUCTURAL_EQUALITY (t);
-	  else if (TYPE_CANONICAL (elt_type) != elt_type
-		   || (index_type 
-		       && TYPE_CANONICAL (index_type) != index_type))
-	    TYPE_CANONICAL (t)
-		= build_cplus_array_type 
-		   (TYPE_CANONICAL (elt_type),
-		    index_type ? TYPE_CANONICAL (index_type) : index_type);
-	  else
-	    TYPE_CANONICAL (t) = t;
+	  set_array_type_canon (t, elt_type, index_type);
 	}
     }
   else
     {
-      if (!TYPE_STRUCTURAL_EQUALITY_P (elt_type)
-	  && !(index_type && TYPE_STRUCTURAL_EQUALITY_P (index_type))
-	  && (TYPE_CANONICAL (elt_type) != elt_type
-	      || (index_type && TYPE_CANONICAL (index_type) != index_type)))
-	/* Make sure that the canonical type is on the appropriate
-	   variants list.  */
-	build_cplus_array_type
-	  (TYPE_CANONICAL (elt_type),
-	   index_type ? TYPE_CANONICAL (index_type) : index_type);
       t = build_array_type (elt_type, index_type);
     }
 
-  /* Push these needs up so that initialization takes place
-     more easily.  */
-  bool needs_ctor
-    = TYPE_NEEDS_CONSTRUCTING (TYPE_MAIN_VARIANT (elt_type));
-  TYPE_NEEDS_CONSTRUCTING (t) = needs_ctor;
-  bool needs_dtor
-    = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TYPE_MAIN_VARIANT (elt_type));
-  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t) = needs_dtor;
-
-  /* We want TYPE_MAIN_VARIANT of an array to strip cv-quals from the
-     element type as well, so fix it up if needed.  */
+  /* Now check whether we already have this array variant.  */
   if (elt_type != TYPE_MAIN_VARIANT (elt_type))
     {
-      tree m = build_cplus_array_type (TYPE_MAIN_VARIANT (elt_type),
-				       index_type);
-
-      if (TYPE_MAIN_VARIANT (t) != m)
+      tree m = t;
+      for (t = m; t; t = TYPE_NEXT_VARIANT (t))
+	if (TREE_TYPE (t) == elt_type)
+	  break;
+      if (!t)
 	{
-	  if (COMPLETE_TYPE_P (TREE_TYPE (t)) && !COMPLETE_TYPE_P (m))
-	    {
-	      /* m was built before the element type was complete, so we
-		 also need to copy the layout info from t.  We might
-	         end up doing this multiple times if t is an array of
-	         unknown bound.  */
-	      tree size = TYPE_SIZE (t);
-	      tree size_unit = TYPE_SIZE_UNIT (t);
-	      unsigned int align = TYPE_ALIGN (t);
-	      unsigned int user_align = TYPE_USER_ALIGN (t);
-	      enum machine_mode mode = TYPE_MODE (t);
-	      for (tree var = m; var; var = TYPE_NEXT_VARIANT (var))
-		{
-		  TYPE_SIZE (var) = size;
-		  TYPE_SIZE_UNIT (var) = size_unit;
-		  TYPE_ALIGN (var) = align;
-		  TYPE_USER_ALIGN (var) = user_align;
-		  SET_TYPE_MODE (var, mode);
-		  TYPE_NEEDS_CONSTRUCTING (var) = needs_ctor;
-		  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (var) = needs_dtor;
-		}
-	    }
+	  t = build_min_array_type (elt_type, index_type);
+	  set_array_type_canon (t, elt_type, index_type);
+	  if (!dependent)
+	    layout_type (t);
 
 	  TYPE_MAIN_VARIANT (t) = m;
 	  TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (m);
@@ -879,6 +872,27 @@ build_cplus_array_type (tree elt_type, tree index_type)
   if (TYPE_SIZE (t) && EXPR_P (TYPE_SIZE (t)))
     TREE_NO_WARNING (TYPE_SIZE (t)) = 1;
 
+  /* Push these needs up to the ARRAY_TYPE so that initialization takes
+     place more easily.  */
+  bool needs_ctor = (TYPE_NEEDS_CONSTRUCTING (t)
+		     = TYPE_NEEDS_CONSTRUCTING (elt_type));
+  bool needs_dtor = (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t)
+		     = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (elt_type));
+
+  if (!dependent && t == TYPE_MAIN_VARIANT (t)
+      && !COMPLETE_TYPE_P (t) && COMPLETE_TYPE_P (elt_type))
+    {
+      /* The element type has been completed since the last time we saw
+	 this array type; update the layout and 'tor flags for any variants
+	 that need it.  */
+      layout_type (t);
+      for (tree v = TYPE_NEXT_VARIANT (t); v; v = TYPE_NEXT_VARIANT (v))
+	{
+	  TYPE_NEEDS_CONSTRUCTING (v) = needs_ctor;
+	  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (v) = needs_dtor;
+	}
+    }
+
   return t;
 }
 
diff --git a/gcc/tree.c b/gcc/tree.c
index ec65fa8..2e4ce8d 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -6793,10 +6793,7 @@ type_hash_canon (unsigned int hashcode, tree type)
   if (*loc)
     {
       tree t1 = ((type_hash *) *loc)->type;
-      /* ???  We'd like to assert here that the hashtable only contains
-         main variants but the C++ frontend breaks this by modifying
-	 types already in the hashtable in build_cplus_array_type.  */
-      /* gcc_assert (TYPE_MAIN_VARIANT (t1) == t1); */
+      gcc_assert (TYPE_MAIN_VARIANT (t1) == t1);
       if (GATHER_STATISTICS)
 	{
 	  tree_code_counts[(int) TREE_CODE (type)]--;