llvm dev - Jul 2011 - [LLVMdev] type-system-rewrite branch landing tomorrow

Nick Lewycky wrote:
> Jay Foad wrote:
>>> ... and it's in.  Please let me know if you see any problems.
>>
>> My clang self-hosted build (Release+Asserts on Release+Asserts, Linux
>> x86_64) fails with:
>>
>> make[2]: Entering directory `/home/jay/llvm/objdir-self/tools/opt'
>> llvm[2]: Compiling GraphPrinters.cpp for Debug+Asserts build
>> clang:
/home/jay/svn/llvm-project/llvm/trunk/include/llvm/Support/Casting.h:194:
>> typename llvm::cast_retty<To, From>::ret_type llvm::cast(const
Y&)
>> [with X = llvm::FunctionType, Y = llvm::Type*, typename
>> llvm::cast_retty<To, From>::ret_type = llvm::FunctionType*]:
Assertion
>> `isa<X>(Val)&&   "cast<Ty>() argument of
incompatible type!"' failed.
>>
>> This is one of the cases that was giving me grief when I was working
>> on the Clang bits! Here's a small C testcase:
>>
>> $ cat foo.c
>> struct S;
>> extern struct T {
>>     struct S (*p)(void);
>> } t;
>> struct S { int i; };
>> void g(void) {
>>     t.p();
>> }
>
> Here's a subset of that problem. This C program:
>
>     struct S;
>     struct T {
>       struct S *s;
>     } t;
>     struct S { int i; };
>
> produces IR with the definition of both %struct.S and %struct.T. This
> program however:
>
>     struct S;
>     struct T {
>       struct S (*p)(void);
>     } t;
>     struct S { int i; };
>
> outputs a "%struct.T = type { {}* }" with no mention of struct S
(or for
> that matter, the fact it's a function!)
I was thinking we should fix this by emitting a function with a 
reference to abstract %struct.S. Turns out we can't quite do that 
because the definition of the struct changes whether the function 
returns the struct or returns void and takes a struct pointer as an sret 
value, etc. Looks like we just got nailed by the ABI problem.

It looks like we need to have opaque function types? Or we need to defer 
emission of some types until we've finished the translation unit.

Nick

PS. On a related note, in another thread last week I proposed that we 
stop changing the function type for the ABI, and instead add 
ABI-specific notes to the Function. 
http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-June/040643.html
> Nick
>
>> $ clang -cc1 -emit-llvm foo.c
>> clang:
/home/jay/svn/llvm-project/llvm/trunk/include/llvm/Support/Casting.h:194:
>> typename llvm::cast_retty<To, From>::ret_type llvm::cast(const
Y&)
>> [with X = llvm::FunctionType, Y = llvm::Type*, typename
>> llvm::cast_retty<To, From>::ret_type = llvm::FunctionType*]:
Assertion
>> `isa<X>(Val)&&   "cast<Ty>() argument of
incompatible type!"' failed.
>>
>> Jay.
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

> It looks like we need to have opaque function types? Or we need to defer
> emission of some types until we've finished the translation unit.
We were deferring this before, no? We could not produce a function type 
taking an opaque type. Once it go resolved we would need to revisit the 
function type.

Maybe it is just the logic for delaying it that got broken in the 
transition?
> Nick
Cheers,
Rafael

2011/7/9 Rafael Ávila de Espíndola <rafael.espindola at
gmail.com>:
>> It looks like we need to have opaque function types? Or we need to
defer
>> emission of some types until we've finished the translation unit.
>
> We were deferring this before, no? We could not produce a function type
> taking an opaque type. Once it go resolved we would need to revisit the
> function type.
>
> Maybe it is just the logic for delaying it that got broken in the
> transition?
What we were doing before is that if a function type depended on an
incomplete type, the function type itself was made opaque, and
subsequently resolved when the type(s) it depended on were completed.
I haven't looked at the committed version closely, but the draft
patches basically ripped out that code because the approach was
unworkable.

-Eli