> 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. > 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)). > Please only notify the ASTMutationListener when adding this to a record found > via a typedef, and please add a comment explaining what’s going on here. Done. > Some of your comments in this block still talk about NSString. > > Is there some way we can share some of the code between these cases? Well, it’s obviously a copypasta. I also don’t like this part and want to improve it, but the fix will involve changing of build-ObjCDictionary/ObjCArray-literal methods, which is out of scope of this patch. I’d like to suggest another patch with this particular fix when we finish this one. > This is the right start, but it looks like you didn’t update the ASTWriter. > You’ll need to add a new AST update kind like UPD_CXX_DEDUCED_RETURN_TYPE; > that’s a good example, look at the code for that in both ASTWriter and > ASTReader. It was a bit tricky, but it’s done now. Also, I added a test for this case. re: crash lib/CodeGen/CGObjC.cpp: + const ParmVarDecl *ArgDecl = *BoxingMethod->param_begin() + 1; + QualType ArgTy = ArgDecl->getType(); Something is going wrong here with the ArgTy only after deserialization, so I don’t really know where the bug is - ASTWriter or ASTReader. I played around with a debugger and sixth sense, but didn’t find the root of the problem yet - serialization algorithm still a bit cryptic for me. Also, I tried to use a hardcoded type, e.g.: QualType ArgTy = Context.getPointer(Context.CharTy.withConst()); and it works just fine. It can be an acceptable solution, since we expect the second parameter of valueWithBytes:objCType: to have type ‘const char *’, but in fact it’s just unfair dirty trick and a workaround, that hides the bug… 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
