rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:1054 + "pointer to __strong expected"); + EmitStoreOfScalar(*AI++, LV); + } else ---------------- This definitely deserves a comment. I don't think the assertion is right; the condition is that the type is legal for a field in a struct that can be passed directly, and while that does exclude `__weak` (because the struct will have to be passed indirectly) and `__autoreleasing` (because that's not legal in a struct), it doesn't exclude `__unsafe_unretained`. This function is implementing an operation that's broadly meaningful (it's a store-init of an owned value, in contrast to a store-init with an unowned value which is what `isInit` is implementing) but not extensively used in the C frontend. On some level, I feel like we should probably teach `EmitStoreThroughLValue` to handle that properly, but that's a more significant refactor. It does look like this change isn't enough to handle `__ptrauth`, which will assume that the source value is signed appropriately for the unqualified type when probably it should be signed appropriately for the qualifier (which, like `__weak`, cannot be address-discriminated because it would stop being passed directly). Probably the default case should be to use `EmitStoreOfScalar`, and `EmitStoreThroughLValue` should only be used if the l-value is a bit-field (the only non-simple case that can actually happen from drilling down to a field). The same logic applies on the load side in the abstract, except that it is only causing problems for `__ptrauth` (well, if we change the behavior above) because loads from the ARC qualifiers don't implicitly retain. Regardless, analogously to this, `EmitRValueForField` should only be calling `EmitLoadOfLValue` for bit-fields and should otherwise call `EmitLoadOfScalar`. Please add a comment on both sides making it clear that the logic is expected to be parallel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70935/new/ https://reviews.llvm.org/D70935 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
