[Patch][ObjC][Proposal] NSValue literals

John McCall rjmccall at apple.com
Mon Jun 22 14:20:04 PDT 2015


> On Jun 22, 2015, at 2:16 PM, AlexDenisov <1101.debian at gmail.com> wrote:
>> On 22 Jun 2015, at 22:54, John McCall <rjmccall at apple.com> wrote:
>> 
>>> On Jun 21, 2015, at 2:22 PM, AlexDenisov <1101.debian at gmail.com> 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.



More information about the cfe-commits mailing list