This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: patch to fix tentative struct type collision



On Dec 14, 2006, at 7:44 AM, Ian Lance Taylor wrote:


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.

No we cannot. But type_hash_eq is a low level utility routine which is supposed to correctly handle any correct type tree presented to it, regardless of where it came from.




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.

Yes, it makes me nervous too. But, we are not in stage 3? I sense what you are saying is better to avoid making an intrusive change to the middle-end if we can bypass it in the front-end. OK then. But I think we should acknowledge that there is a hole in correctly handling all type trees in this routine.




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.

There is no need to do this. When they are not NULL, rest of code checks that they match or not. I think this part is OK.


Thanks for your response. I will change the middle end in this case.

- Fariborz


Ian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]