> On 22 Jun 2015, at 23:20, John McCall <[email protected]> wrote: > >> 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.
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.
objc_boxable.patch
Description: Binary data
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
