Re: patch to fix tentative struct type collision

[Ian Lance Taylor <iant at google dot com>]

Fariborz Jahanian <fjahanian@apple.com> writes:

> > To me this looks like a middle-end type system problem.  My
> > understanding is that in C two structs are compatible if they have the
> > same fields.  In C++ they are compatible if they have the same name.
> > type_hash_eq currently uses the C semantics.
> 
> In this case, same fields are NULL. Is 'struct Foo *' compatible with
> 'struct Bar *' in C?

No, but that's not the right question.  The question is whether struct
Foo and struct Bar are compatible when neither is defined.  The answer
is that the question is meaningless, since you can't use them anyhow.


> > I think that frontends, other than perhaps the C frontend, simply can
> > not currently call build_type_attribute_variant with a RECORD_TYPE
> > from the frontend.  The middle-end type system is too imprecise to
> > support that.  As far as I can see, the C++ frontend never does that.
> 
> It cannot because of shortcoming in type_hash_eq. Maybe we should fix
> type_hash_eq instead of disallowing using build_type_attribute_variant.

When you say "fix," you are making an assumption.  You are assuming
that the type system in the middle-end is the type system that you
want in the frontend.  Although the frontend type system and the
middle-end type system look very similar, since they use the same
structures, they are different by definition: different frontends
require different type systems.  The middle-end type system is
currently, unfortunately, undefined.  The use of the word "fix" here
is incorrect in the absence of a defined middle-end type system.  When
you say "fix," I read "change."  And it makes me nervous to change
working code, in the absence of a specification that we want to
follow, in the absence of a test case, for the support of a relatively
obscure language, when there are already straightforward working
approaches which are used by existing frontends.


> > In any case, the patch as it stands looks clearly incorrect to me.  It
> > does not make sense to only check TYPE_NAME when there are no fields.
> > If we check it at all, we need to check it in all cases.  And the
> 
> Checking for TYPE_NAME is one case. What are the other cases?

Your proposed patch only checked TYPE_NAME when TYPE_FIELDS was NULL.
The other case would be to check TYPE_NAME when TYPE_FIELDS is not
NULL.

Ian