[Patch][ObjC][Proposal] NSValue literals

[AlexDenisov]

> On 22 Jun 2015, at 23:20, John McCall <rjmccall at apple.com> wrote:
> 
>> 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.

Yes, it was my bad. I applied attribute to a wrong declaration, now it’s fixed.
Also, I think we can easily drop support of typedef, because it works only if attribute applied after typedef, e.g.:
typedef old new __attribute(objc_boxable);

which is not a preferred option.

> 
>>>>> 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.

Also done with this, though I’m not sure if I did it in a correct way.

> 
> John.

Please, take a look at the new version.

Best regards,
Alex.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: objc_boxable.patch
Type: application/octet-stream
Size: 73732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150625/21e9e265/attachment.obj>
-------------- next part --------------


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150625/21e9e265/attachment.sig>