Re: [C PATCH 6/6] c23: construct composite type for tagged types

[Joseph Myers <joseph@codesourcery.com>]

On Sat, 26 Aug 2023, Martin Uecker via Gcc-patches wrote:

> @@ -501,9 +510,61 @@ composite_type (tree t1, tree t2)
>  	return build_type_attribute_variant (t1, attributes);
>        }
>  
> -    case ENUMERAL_TYPE:
>      case RECORD_TYPE:
>      case UNION_TYPE:
> +      if (flag_isoc2x && !comptypes_same_p (t1, t2))
> +	{
> +	  gcc_checking_assert (COMPLETE_TYPE_P (t1) && COMPLETE_TYPE_P (t2));
> +	  gcc_checking_assert (comptypes (t1, t2));
> +
> +	  /* If a composite type for these two types is already under
> +	     construction, return it.  */
> +
> +	  for (struct composite_cache *c = cache; c != NULL; c = c->next)
> +	    if (c->t1 == t1 && c->t2 == t2)
> +	       return c->composite;
> +
> +	  /* Otherwise, create a new type node and link it into the cache.  */
> +
> +	  tree n = make_node (code1);
> +	  struct composite_cache cache2 = { t1, t2, n, cache };
> +	  cache = &cache2;
> +
> +	  tree f1 = TYPE_FIELDS (t1);
> +	  tree f2 = TYPE_FIELDS (t2);
> +	  tree fields = NULL_TREE;
> +
> +	  for (tree a = f1, b = f2; a && b;
> +	       a = DECL_CHAIN (a), b = DECL_CHAIN (b))
> +	    {
> +	      tree ta = TREE_TYPE (a);
> +	      tree tb = TREE_TYPE (b);
> +
> +	      gcc_assert (DECL_NAME (a) == DECL_NAME (b));
> +	      gcc_assert (comptypes (ta, tb));
> +
> +	      tree f = build_decl (input_location, FIELD_DECL, DECL_NAME (a),
> +				   composite_type_internal (ta, tb, cache));
> +
> +	      DECL_FIELD_CONTEXT (f) = n;
> +	      DECL_CHAIN (f) = fields;

There is a lot more per-field setup done in grokdeclarator, grokfield and 
finish_struct when a struct or union is defined.  I'm concerned that just 
calling build_decl here and then missing most of the per-field setup done 
elsewhere will not get the composite type set up correctly, especially in 
cases such as bit-fields and packed structures.

Note that the test you have of bit-fields (c2x-tag-composite-3.c) probably 
doesn't exercise this code, because the two types are the same (defined in 
the same scope, so it would be an error if they weren't the same) and so 
the comptypes_same_p check should short-circuit this code.  You need to 
test such issues in cases where the types are genuinely not the same - and 
for bit-fields, that includes ensuring you cover code paths that depend on 
each of DECL_BIT_FIELD, DECL_C_BIT_FIELD, DECL_BIT_FIELD_TYPE, to make 
sure that all of those are correct.

-- 
Joseph S. Myers
joseph@codesourcery.com