Re: PATCH RFA: Support Plan 9 extensions in gcc

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Sun, 26 Sep 2010, Ian Lance Taylor wrote:

> Joseph Myers asked specifically about standardization; I very much doubt
> the Plan 9 folks care one way or another about whether this
> functionality is added to the standard.  However, it seems to me
> unlikely that any contradictory functionality would be added.

It is also the case that we now know what form of anonymous structures and 
unions is in C1X and I implemented support for that in GCC.

However, one case of ambiguity that this patch does not detect appears to 
conflict with the standard semantics.

typedef struct { int a; } s1;
struct s2 { s1; int s1; };
int f(struct s2 *p) { return p->s1; }

In C1X, p->s1 is an unambiguous reference to the int field.  With 
-fplan9-extensions, there is ambiguity; this is not detected as such, but 
instead you get an error:

t1.c:3:23: error: incompatible types when returning type 's1' but 'int' was expected

Logically this option should perhaps cause such a case to be detected with 
an error for duplicate fields (and so be documented as causing some code 
to be rejected that is otherwise accepted).

The duplicate field errors for anonymous structures and unions post-date 
your previous patch.  I don't actually see how the other cases of 
ambiguity that your patch tries to detect can arise without an error being 
given earlier for duplicate field names - they all involve multiple fields 
with the same anonymous structure or union type, which would mean all the 
fields within that type are duplicated.  If you ignore that issue, I don't 
think the ambiguity detection will behave as intended in all cases.

> +/* Return whether STRUCT_TYPE has an anonymous field with type TYPE.
> +   If there are more than one such field, this returns NULL and sets

(false, not NULL)

> +   *AMBIGUOUS_P.  This is used to implement -fplan9-extensions.  */
> +
> +static bool
> +find_anonymous_field_with_type (tree struct_type, tree type,
> +				bool *ambiguous_p)
> +{
> +  tree field;
> +  bool found;
> +
> +  gcc_assert (TREE_CODE (struct_type) == RECORD_TYPE
> +	      || TREE_CODE (struct_type) == UNION_TYPE);
> +  found = false;
> +  for (field = TYPE_FIELDS (struct_type);
> +       field != NULL_TREE;
> +       field = TREE_CHAIN (field))
> +    {
> +      if (DECL_NAME (field) == NULL
> +	  && comptypes (type, TYPE_MAIN_VARIANT (TREE_TYPE (field))))
> +	{
> +	  if (found)
> +	    {
> +	      *ambiguous_p = true;
> +	      return false;
> +	    }
> +	  found = true;
> +	}
> +      else if (DECL_NAME (field) == NULL
> +	       && (TREE_CODE (TREE_TYPE (field)) == RECORD_TYPE
> +		   || TREE_CODE (TREE_TYPE (field)) == UNION_TYPE)
> +	       && find_anonymous_field_with_type (TREE_TYPE (field), type,
> +						  ambiguous_p))
> +	{
> +	  if (found)
> +	    {
> +	      *ambiguous_p = true;
> +	      return false;
> +	    }
> +	  found = true;
> +	}
> +    }
> +  return found;
> +}

The inner call to find_anonymous_field_with_type could detect ambiguity 
and return false, with *ambiguous_p set to true.  The loop inside the 
outer find_anonymous_field_with_type would then continue and might find 
yet another field with the given type, so returning true but with 
*ambiguous_p set to true as well.  This at least isn't in accordance with 
the documented semantics of this function; I think you want something 
similar to the

> +      else if (*ambiguous_p)
> +	return NULL_TREE;

in convert_to_anonymous_field.

-- 
Joseph S. Myers
joseph@codesourcery.com