[cfe-commits] [PATCH] Merge function types is C.

[Richard Smith]

On Wed, Nov 28, 2012 at 1:52 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Wed, Nov 28, 2012 at 1:31 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Wed, Nov 28, 2012 at 1:25 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On Wed, Nov 28, 2012 at 12:59 PM, Eli Friedman <eli.friedman at gmail.com>
>> > wrote:
>> >>
>> >> On Wed, Nov 28, 2012 at 9:17 AM, Rafael Espíndola
>> >> <rafael.espindola at gmail.com> wrote:
>> >> > Thanks Richard for pointing me some missing cases. The attached patch
>> >> > takes a step back and merge only the return types.
>> >> >
>> >> > Richard suggested avoiding the merging when the new Decl is a k&r
>> >> > definition, but at this point of the code we haven't attached the
>> body
>> >> > yet. It would also be nice if we could handle both definitions and
>> >> > declarations uniformly.
>> >> >
>> >> > I debugged why CodeGen was complaining and the problem is that by
>> >> > merging just the function types we end up with a ParmVarDecl whose
>> >> > type doesn't match the corresponding type in the FunctionProtoType
>> and
>> >> > CodeGen asserts. Two options that would still let us merge the full
>> >> > types is making CodeGen cope with it (produce a llvm cast) or
>> patching
>> >> > the type of the ParmVarDecl too. Do you think we should do it?
>> >>
>> >> Patching the type of the ParmVarDecl would be appropriate.
>> >
>> >
>> > Given:
>> >
>> > int f(int);
>> > int f(a)
>> >   char a;
>> > {
>> >   return sizeof(a);
>> > }
>> >
>> > I would expect 1 to be returned, not 4.
>>
>> Yes, you're right.
>>
>> I was thinking of:
>> int f(int (*)[10]);
>> int f(int (*x)[]) {
>>   return sizeof(*x); // 40
>> }
>>
>> But maybe not relevant to this discussion?
>
>
> I'd expect that to also be ill-formed. The type of the 'x' parameter is
> int(*)[]
>

I think I wasn't very clear here. My interpretation is: The type of the 'x'
variable within the definition of 'f' is int(*)[]. That 'x' is not a
redeclaration of the parameter in the previous 'f' declaration, so its type
doesn't get merged, even though the type of the function does get merged.


> even though the type of the function is int(int(*)[10]), by my
> interpretation (and GCC and EDG agree). It seems to me that CodeGen is just
> wrong to assume that the type of a ParmVarDecl matches the type of the
> corresponding parameter in the function's type, and it'll need to insert
> casts as necessary.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121128/868087e7/attachment.html>