Re: [C++] Fix 3130, 3131

[Jason Merrill <jason_merrill at redhat dot com>]

>>>>> "Nathan" == Nathan Sidwell <nathan@codesourcery.com> writes:

> Initially I tried to just change get_primary_binfo, to see if it could
> find the canonical binfo, when given a choice, but that failed. The only
> way I found to fix it was by allowing a canonical binfo to be inside
> a non-canonical one.

Please explain to me how this makes sense.  How can a canonical binfo be
inside a non-canonical one?

> *************** dfs_accumulate_vtbl_inits (binfo, orig_b
> *** 7578,7587 ****
  
>   	      for (; b; b = BINFO_PRIMARY_BASE_OF (b))
>   		{
>   		  primary = b;
>   		  orig_primary = BINFO_PRIMARY_BASE_OF (orig_primary);
> - 		  if (b == rtti_binfo)
> - 		    break;
>   		}
>   	    }
>   	  else
> --- 7669,7688 ----
  
>   	      for (; b; b = BINFO_PRIMARY_BASE_OF (b))
>   		{
> + 		  tree probe;
> + 
> + 		  /* See if B is still within the hierarchy starting
> + 		     at RTTI_BINFO. */
> + 		  for (probe = b; probe;
> + 		       probe = BINFO_INHERITANCE_CHAIN (probe))
> + 		    if (probe == rtti_binfo)
> + 		      break;
> + 		  
> + 		  if (!probe)
> + 		    break;
> + 		  
>   		  primary = b;
>   		  orig_primary = BINFO_PRIMARY_BASE_OF (orig_primary);
>   		}
>   	    }
>   	  else

Ah, I see.  My assumption that if the offsets matched, the hierarchy also
matched, was wrong.  In Scott's second testcase, indirect primary vbase B
is at offset 4 in D.  In F, it is still at offset 4 from D, but only
because it is primary to E, which is laid out immediately after D.  So that
test for case 1 doesn't work, and the case 1 handling should probably be
folded back into the case 2 & 3 code; this test is case 2, but we're
identifying it as case 1.

Looking at the test for 3145, I notice an oddity whereby the primary base
(C8) of a class (C9) has lost its own primary (C4).  This seems odd to me;
I would have thought the primary base would have had priority, perhaps
simply by treating the primary base as though it had been declared first.
But, of course, it's too late to change this now.  I wish we had had more
implementation experience before freezing the ABI...

Your fix for 3145 seems wrong to me; the bug there is that orig_last
tracking is broken in the fairly common case that the base we're primary to
in T is different from that in BINFO_TYPE (RTTI_BINFO).  Here's my proposed
patch, which does away with orig_last in favor of waiting until we're done
with all the bases before trying to look at someone else's vtable pointer.
It also fixes the extra space in 3145 (which, as you said, does not affect
binary compatibility).  Look reasonable?  I'm going to go ahead and check
it in after testing.

2001-06-15  Jason Merrill  <jason_merrill@redhat.com>

	* class.c (dfs_accumulate_vtbl_inits): Just point to the base we're
	sharing a ctor vtable with.  Merge code for cases 1 and 2.
	(binfo_ctor_vtable): New fn.
	(build_vtt_inits, dfs_build_secondary_vptr_vtt_inits): Use it.

*** class.c.~1~	Thu Jun 14 16:10:32 2001
--- class.c	Fri Jun 15 14:28:04 2001
*************** get_original_base (base_binfo, binfo)
*** 7252,7257 ****
--- 7252,7282 ----
    return NULL;
  }
  
+ /* When building a secondary VTT, BINFO_VTABLE is set to a TREE_LIST with
+    PURPOSE the RTTI_BINFO, VALUE the real vtable pointer for this binfo,
+    and CHAIN the vtable pointer for this binfo after construction is
+    complete.  VALUE can also be another BINFO, in which case we recurse. */
+ 
+ static tree
+ binfo_ctor_vtable (binfo)
+      tree binfo;
+ {
+   tree vt;
+ 
+   while (1)
+     {
+       vt = BINFO_VTABLE (binfo);
+       if (TREE_CODE (vt) == TREE_LIST)
+ 	vt = TREE_VALUE (vt);
+       if (TREE_CODE (vt) == TREE_VEC)
+ 	binfo = vt;
+       else
+ 	break;
+     }
+ 
+   return vt;
+ }
+ 
  /* Recursively build the VTT-initializer for BINFO (which is in the
     hierarchy dominated by T).  INITS points to the end of the initializer
     list to date.  INDEX is the VTT index where the next element will be
*************** build_vtt_inits (binfo, t, inits, index)
*** 7288,7296 ****
      }
  
    /* Add the address of the primary vtable for the complete object.  */
!   init = BINFO_VTABLE (binfo);
!   if (TREE_CODE (init) == TREE_LIST)
!     init = TREE_VALUE (init);
    *inits = build_tree_list (NULL_TREE, init);
    inits = &TREE_CHAIN (*inits);
    if (top_level_p)
--- 7313,7319 ----
      }
  
    /* Add the address of the primary vtable for the complete object.  */
!   init = binfo_ctor_vtable (binfo);
    *inits = build_tree_list (NULL_TREE, init);
    inits = &TREE_CHAIN (*inits);
    if (top_level_p)
*************** dfs_build_secondary_vptr_vtt_inits (binf
*** 7427,7435 ****
        while (BINFO_PRIMARY_BASE_OF (binfo))
          binfo = BINFO_PRIMARY_BASE_OF (binfo);
      }
!   init = BINFO_VTABLE (binfo);
!   if (TREE_CODE (init) == TREE_LIST)
!     init = TREE_VALUE (init);
    TREE_VALUE (l) = tree_cons (NULL_TREE, init, TREE_VALUE (l));
  
    return NULL_TREE;
--- 7450,7456 ----
        while (BINFO_PRIMARY_BASE_OF (binfo))
          binfo = BINFO_PRIMARY_BASE_OF (binfo);
      }
!   init = binfo_ctor_vtable (binfo);
    TREE_VALUE (l) = tree_cons (NULL_TREE, init, TREE_VALUE (l));
  
    return NULL_TREE;
*************** dfs_accumulate_vtbl_inits (binfo, orig_b
*** 7621,7762 ****
    tree vtbl = NULL_TREE;
    int ctor_vtbl_p = !same_type_p (BINFO_TYPE (rtti_binfo), t);
  
!   if (ctor_vtbl_p)
      {
!       tree primary = binfo;
!       tree orig_primary = orig_binfo;
!       
!       if (TREE_VIA_VIRTUAL (orig_binfo) && BINFO_PRIMARY_P (orig_binfo))
! 	{
! 	  /* In the hierarchy of BINFO_TYPE (RTTI_BINFO), this is a
!              primary virtual base.  If it is not the same primary in
!              the hierarchy of T, we'll need to generate a ctor vtable
!              for it, to place at its location in T.  If it is the same
!              primary, we still need a VTT entry for the vtable, but it
!              should point to the ctor vtable for the base it is a
!              primary for within the sub-hierarchy of RTTI_BINFO.
  	      
! 	     There are three possible cases:
  	      
!              1) We are in the same place.
! 	     2) We are a primary base within a lost primary virtual base of
! 	     RTTI_BINFO.
! 	     3) We are not primary to anything else in RTTI_BINFO.  */
  	  
! 	  if (tree_int_cst_equal (BINFO_OFFSET (orig_binfo),
! 				  size_diffop (BINFO_OFFSET (binfo),
! 					       BINFO_OFFSET (rtti_binfo))))
! 	    {
! 	      /* Case 1: We're in the same place relative to
! 	     	 RTTI_BINFO as we were in the complete type, so we are
! 	     	 primary either to RTTI_BINFO or one of its secondary
! 	     	 bases.  */
! 	      
! 	      /* Walk down our until we either find the last
! 	     	 primary base or rtti_binfo.  */
! 	      tree b = BINFO_PRIMARY_BASE_OF (binfo);
! 
! 	      for (; b; b = BINFO_PRIMARY_BASE_OF (b))
! 		{
! 		  tree probe;
! 
! 		  /* See if B is still within the hierarchy starting
! 		     at RTTI_BINFO. */
! 		  for (probe = b; probe;
! 		       probe = BINFO_INHERITANCE_CHAIN (probe))
! 		    if (probe == rtti_binfo)
! 		      break;
! 		  
! 		  if (!probe)
! 		    break;
! 		  
! 		  primary = b;
! 		  orig_primary = BINFO_PRIMARY_BASE_OF (orig_primary);
! 		}
! 	    }
! 	  else
! 	    {
! 	      /* Case 2 or 3: We're not in the same place.  We might
! 	         still be primary to something within a lost primary
! 	         virtual base of RTTI_BINFO.  */
! 	      tree b;
! 	      tree last = binfo;
! 	      tree orig_last = orig_binfo;
! 
! 	      /* First, look through the bases we are primary to for a
! 	     	 virtual base.  */
! 	      for (b = BINFO_PRIMARY_BASE_OF (binfo);
! 		   b;
! 		   b = BINFO_PRIMARY_BASE_OF (b))
! 		{
! 		  if (!TREE_VIA_VIRTUAL (b))
! 		    {
! 		      /* See if B is still within the hierarchy starting
! 			 at RTTI_BINFO. */
! 		      tree probe;
! 
! 		      for (probe = b; probe;
! 			   probe = BINFO_INHERITANCE_CHAIN (probe))
! 			if (probe == rtti_binfo)
! 			  break;
! 		      if (!probe)
! 			{
! 			  b = NULL_TREE;
! 			  break;
! 			}
! 		    }
! 		  
! 		  last = b;
! 		  if (orig_last)
! 		    orig_last = BINFO_PRIMARY_BASE_OF (orig_last);
! 		  if (TREE_VIA_VIRTUAL (b))
! 		    break;
! 		}
! 	      /* If we run out of primary links, keep looking down our
! 	     	 inheritance chain; we might be an indirect primary of
! 	     	 a virtual base.  */
! 	      if (b == NULL_TREE)
! 		for (b = last; b; b = BINFO_INHERITANCE_CHAIN (b))
! 		  if (TREE_VIA_VIRTUAL (b))
! 		    break;
! 
! 	      /* If we found a virtual base B and it is a base of
! 	     	 RTTI_BINFO, we share our vtable with LAST, i.e. the
! 	     	 derived-most base within B of which we are a primary.
! 	     	 Otherwise, we get our own.  */
! 	      if (b && binfo_for_vbase (BINFO_TYPE (b),
! 					BINFO_TYPE (rtti_binfo)))
! 		{
! 		  my_friendly_assert (orig_last, 20010611);
! 		  primary = last;
! 		  orig_primary = orig_last;
! 		}
! 	    }
! 	}
!       
!       vtbl = BINFO_VTABLE (primary);
!       if (vtbl && TREE_CODE (vtbl) == TREE_LIST
! 	  && TREE_PURPOSE (vtbl) == rtti_binfo)
! 	{
! 	  vtbl = TREE_VALUE (vtbl);
! 	  if (primary == binfo)
! 	    /* We created this vtable because we met its primary base
! 	       earlier in the inheritance graph walk of
! 	       RTTI_BINFO.  */
! 	    return inits;
! 	}
!       else if (primary != binfo)
  	{
! 	  /* We're the primary of some binfo that we've not yet
! 	     met in the inheritance graph walk of RTTI_BINFO. We
! 	     must create that vtable now. */
! 	  inits = dfs_accumulate_vtbl_inits (primary, orig_primary,
! 					     rtti_binfo, t, l);
! 	  vtbl = BINFO_VTABLE (primary);
! 	  vtbl = TREE_VALUE (vtbl);
  	}
!       else
! 	vtbl = NULL;
      }
    else if (!BINFO_NEW_VTABLE_MARKED (orig_binfo, BINFO_TYPE (rtti_binfo)))
      return inits;
--- 7642,7699 ----
    tree vtbl = NULL_TREE;
    int ctor_vtbl_p = !same_type_p (BINFO_TYPE (rtti_binfo), t);
  
!   if (ctor_vtbl_p
!       && TREE_VIA_VIRTUAL (orig_binfo) && BINFO_PRIMARY_P (orig_binfo))
      {
!       /* In the hierarchy of BINFO_TYPE (RTTI_BINFO), this is a
! 	 primary virtual base.  If it is not the same primary in
! 	 the hierarchy of T, we'll need to generate a ctor vtable
! 	 for it, to place at its location in T.  If it is the same
! 	 primary, we still need a VTT entry for the vtable, but it
! 	 should point to the ctor vtable for the base it is a
! 	 primary for within the sub-hierarchy of RTTI_BINFO.
  	      
! 	 There are three possible cases:
  	      
! 	 1) We are in the same place.
! 	 2) We are a primary base within a lost primary virtual base of
! 	 RTTI_BINFO.
! 	 3) We are not primary to anything else in RTTI_BINFO.  */
  	  
!       tree b = BINFO_PRIMARY_BASE_OF (binfo);
!       tree last = NULL_TREE;
!       tree primary = NULL_TREE;
! 
!       /* First, look through the bases we are primary to for RTTI_BINFO
! 	 or a virtual base.  */
!       for (; b; b = BINFO_PRIMARY_BASE_OF (b))
  	{
! 	  last = b;
! 	  if (TREE_VIA_VIRTUAL (b) || b == rtti_binfo)
! 	    break;
  	}
!       /* If we run out of primary links, keep looking down our
! 	 inheritance chain; we might be an indirect primary.  */
!       if (b == NULL_TREE)
! 	for (b = last; b; b = BINFO_INHERITANCE_CHAIN (b))
! 	  if (TREE_VIA_VIRTUAL (b) || b == rtti_binfo)
! 	    break;
! 
!       /* If we found RTTI_BINFO, this is case 1.  If we found a virtual
! 	 base B and it is a base of RTTI_BINFO, this is case 2.  In
! 	 either case, we share our vtable with LAST, i.e. the
! 	 derived-most base within B of which we are a primary.  */
!       if (b == rtti_binfo
! 	  || (b && binfo_for_vbase (BINFO_TYPE (b),
! 				    BINFO_TYPE (rtti_binfo))))
! 	primary = last;
!       /* Otherwise, this is case 3 and we get our own.  */
! 
!       if (primary)
! 	/* We're the primary of some binfo that we may not have
! 	   met in the inheritance graph walk of RTTI_BINFO.  Just
! 	   point to it.  */
! 	vtbl = primary;
      }
    else if (!BINFO_NEW_VTABLE_MARKED (orig_binfo, BINFO_TYPE (rtti_binfo)))
      return inits;