> On Jun 22, 2015, at 2:16 PM, AlexDenisov <[email protected]> wrote:
>> On 22 Jun 2015, at 22:54, John McCall <[email protected]> wrote:
>> 
>>> On Jun 21, 2015, at 2:22 PM, AlexDenisov <[email protected]> wrote:
>>>> Please put the attribute in the correct position (after “struct”) in this 
>>>> example as well.  Examples should consistently emphasize the correct 
>>>> position, just so that people learn it for this and other attributes.
>>> 
>>> Clang complains that attribute cannot be applied to a struct declaration, 
>>> which means that attribute at the end of the type definition is correct.
>>> It is possible to allow using of objc_boxable with a struct declaration, 
>>> but in this case using of a typedef doesn’t make any sense.
>> 
>> This probably means that your entry in Attr.td has the wrong subject list; 
>> it should use the same Subjects line as ObjCBridge.
> 
> Just double-checked it: it has the same subject list.

Weird.  You should try to debug where Sema is emitting this diagnostic, then.

>>>> I don’t know if it’s causing your crash, but it looks like you never 
>>>> zero-initialize these in the Sema constructor.
>>> 
>>> It doesn’t the problem, though it’s a nice catch.
>>> 
>>>> We should probably call this isObjCBoxableRecordType() to eliminate any 
>>>> confusion.
>>> 
>>> Done. Also added test for a union, to be fair.
>>> 
>>>> I don’t understand the purpose of checking for a UnaryOperator here.  Can 
>>>> you not just look at the type?
>>> 
>>> When ObjCBoxedExpr creates with a record (struct/union) then the lvalue 
>>> should be casted to ‘const void *’, which leads to a UnaryOperator here. 
>>> Anyway, I agree that such type-casting looks weird.
>>> Currently, I rewrote it and I still check the canonical type. I can move 
>>> this check into ObjCBoxedExpr, but I need the underlaying type to generate 
>>> correct encoding (@encode(whatnot)).
>> 
>> Hmm.  This sounds like it's unnecessary residue of the old AST pattern, 
>> where you needed the first operand to be a const void* because that was the 
>> type of the parameter.  Just leave the operand as an r-value of the record 
>> type, have IR-gen emit the expression into a temporary, and cast the address 
>> of that temporary to i8*.
> 
> Actually I tried this initially, but clang complains about wrong types, e.g.:
> sending 'NSPoint' (aka 'struct _NSPoint') to parameter of incompatible type 
> 'const void *’
> so I decided to wrap the ValueExpr with a ImplicitCastExpr.

Don’t ask PerformCopyInitialization to initialize the boxing method parameter; 
ask it to initialize a temporary of the record type.

John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to