diff mbox

[C++] Fix TYPE_CANONICAL on TEMPLATE_TYPE_PARM created by rewrite_template_parm (PR c++/80309)

Message ID 20170405184803.GX17461@tucnak
State New
Headers show

Commit Message

Jakub Jelinek April 5, 2017, 6:48 p.m. UTC
Hi!

The testcase in the PR (GC related, so not easily reduceable) ICEs
because two TEMPLATE_TYPE_PARMs that comptypes returns true on have
different TYPE_CANONICAL (each has itself).
The problem is that comptypes->structural_comptypes->comp_template_parms_position
compares not just idx and level, but also the
TEMPLATE_PARM_PARAMETER_PACK bit, and rewrite_template_parm first
computes the TYPE_CANONICAL (with the bit still unset) and then copies
over the TEMPLATE_PARM_PARAMETER_PACK bit.  This means that it can for
TYPE_CANONICAL comparison compare unequal when it will compare equal later
or maybe vice versa.

Fixed thusly (the rewrite_template_parm changes), additionally while
debugging I've noticed an inefficiency, vec_safe_grow_cleared is certainly
more efficient than vec_safe_push of NULL in a loop - e.g. the latter
might reallocate multiple times and really can't beat one growing and
one memset, and two minor formatting nits.

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

2017-04-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/80309
	* pt.c (canonical_type_parameter): Use vec_safe_grow_cleared instead
	of a loop doing vec_safe_push of NULL.  Formatting fixes.
	(rewrite_template_parm): Copy TEMPLATE_PARM_PARAMETER_PACK from oldidx
	to newidx before calling canonical_type_parameter on newtype.


	Jakub

Comments

Jason Merrill April 5, 2017, 6:59 p.m. UTC | #1
OK.

On Wed, Apr 5, 2017 at 2:48 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The testcase in the PR (GC related, so not easily reduceable) ICEs
> because two TEMPLATE_TYPE_PARMs that comptypes returns true on have
> different TYPE_CANONICAL (each has itself).
> The problem is that comptypes->structural_comptypes->comp_template_parms_position
> compares not just idx and level, but also the
> TEMPLATE_PARM_PARAMETER_PACK bit, and rewrite_template_parm first
> computes the TYPE_CANONICAL (with the bit still unset) and then copies
> over the TEMPLATE_PARM_PARAMETER_PACK bit.  This means that it can for
> TYPE_CANONICAL comparison compare unequal when it will compare equal later
> or maybe vice versa.
>
> Fixed thusly (the rewrite_template_parm changes), additionally while
> debugging I've noticed an inefficiency, vec_safe_grow_cleared is certainly
> more efficient than vec_safe_push of NULL in a loop - e.g. the latter
> might reallocate multiple times and really can't beat one growing and
> one memset, and two minor formatting nits.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-04-05  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/80309
>         * pt.c (canonical_type_parameter): Use vec_safe_grow_cleared instead
>         of a loop doing vec_safe_push of NULL.  Formatting fixes.
>         (rewrite_template_parm): Copy TEMPLATE_PARM_PARAMETER_PACK from oldidx
>         to newidx before calling canonical_type_parameter on newtype.
>
> --- gcc/cp/pt.c.jj      2017-04-04 07:32:53.000000000 +0200
> +++ gcc/cp/pt.c 2017-04-05 13:43:18.883690743 +0200
> @@ -3997,10 +3997,10 @@ canonical_type_parameter (tree type)
>    tree list;
>    int idx = TEMPLATE_TYPE_IDX (type);
>    if (!canonical_template_parms)
> -    vec_alloc (canonical_template_parms, idx+1);
> +    vec_alloc (canonical_template_parms, idx + 1);
>
> -  while (canonical_template_parms->length () <= (unsigned)idx)
> -    vec_safe_push (canonical_template_parms, NULL_TREE);
> +  if (canonical_template_parms->length () <= (unsigned) idx)
> +    vec_safe_grow_cleared (canonical_template_parms, idx + 1);
>
>    list = (*canonical_template_parms)[idx];
>    while (list && !comptypes (type, TREE_VALUE (list), COMPARE_STRUCTURAL))
> @@ -4011,8 +4011,7 @@ canonical_type_parameter (tree type)
>    else
>      {
>        (*canonical_template_parms)[idx]
> -               = tree_cons (NULL_TREE, type,
> -                            (*canonical_template_parms)[idx]);
> +       = tree_cons (NULL_TREE, type, (*canonical_template_parms)[idx]);
>        return type;
>      }
>  }
> @@ -24955,6 +24954,8 @@ rewrite_template_parm (tree olddecl, uns
>        newidx = TEMPLATE_TYPE_PARM_INDEX (newtype)
>         = build_template_parm_index (index, level, level,
>                                      newdecl, newtype);
> +      TEMPLATE_PARM_PARAMETER_PACK (newidx)
> +       = TEMPLATE_PARM_PARAMETER_PACK (oldidx);
>        TYPE_STUB_DECL (newtype) = TYPE_NAME (newtype) = newdecl;
>        TYPE_CANONICAL (newtype) = canonical_type_parameter (newtype);
>
> @@ -25002,11 +25003,11 @@ rewrite_template_parm (tree olddecl, uns
>        SET_DECL_TEMPLATE_PARM_P (newconst);
>        newidx = build_template_parm_index (index, level, level,
>                                           newconst, newtype);
> +      TEMPLATE_PARM_PARAMETER_PACK (newidx)
> +       = TEMPLATE_PARM_PARAMETER_PACK (oldidx);
>        DECL_INITIAL (newdecl) = DECL_INITIAL (newconst) = newidx;
>      }
>
> -  TEMPLATE_PARM_PARAMETER_PACK (newidx)
> -    = TEMPLATE_PARM_PARAMETER_PACK (oldidx);
>    return newdecl;
>  }
>
>
>         Jakub
diff mbox

Patch

--- gcc/cp/pt.c.jj	2017-04-04 07:32:53.000000000 +0200
+++ gcc/cp/pt.c	2017-04-05 13:43:18.883690743 +0200
@@ -3997,10 +3997,10 @@  canonical_type_parameter (tree type)
   tree list;
   int idx = TEMPLATE_TYPE_IDX (type);
   if (!canonical_template_parms)
-    vec_alloc (canonical_template_parms, idx+1);
+    vec_alloc (canonical_template_parms, idx + 1);
 
-  while (canonical_template_parms->length () <= (unsigned)idx)
-    vec_safe_push (canonical_template_parms, NULL_TREE);
+  if (canonical_template_parms->length () <= (unsigned) idx)
+    vec_safe_grow_cleared (canonical_template_parms, idx + 1);
 
   list = (*canonical_template_parms)[idx];
   while (list && !comptypes (type, TREE_VALUE (list), COMPARE_STRUCTURAL))
@@ -4011,8 +4011,7 @@  canonical_type_parameter (tree type)
   else
     {
       (*canonical_template_parms)[idx]
-		= tree_cons (NULL_TREE, type,
-			     (*canonical_template_parms)[idx]);
+	= tree_cons (NULL_TREE, type, (*canonical_template_parms)[idx]);
       return type;
     }
 }
@@ -24955,6 +24954,8 @@  rewrite_template_parm (tree olddecl, uns
       newidx = TEMPLATE_TYPE_PARM_INDEX (newtype)
 	= build_template_parm_index (index, level, level,
 				     newdecl, newtype);
+      TEMPLATE_PARM_PARAMETER_PACK (newidx)
+	= TEMPLATE_PARM_PARAMETER_PACK (oldidx);
       TYPE_STUB_DECL (newtype) = TYPE_NAME (newtype) = newdecl;
       TYPE_CANONICAL (newtype) = canonical_type_parameter (newtype);
 
@@ -25002,11 +25003,11 @@  rewrite_template_parm (tree olddecl, uns
       SET_DECL_TEMPLATE_PARM_P (newconst);
       newidx = build_template_parm_index (index, level, level,
 					  newconst, newtype);
+      TEMPLATE_PARM_PARAMETER_PACK (newidx)
+	= TEMPLATE_PARM_PARAMETER_PACK (oldidx);
       DECL_INITIAL (newdecl) = DECL_INITIAL (newconst) = newidx;
     }
 
-  TEMPLATE_PARM_PARAMETER_PACK (newidx)
-    = TEMPLATE_PARM_PARAMETER_PACK (oldidx);
   return newdecl;
 }