[PATCH] D118511: Add a warning for not packing non-POD members in packed structs
David Blaikie via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 10 10:45:52 PST 2022
dblaikie added a comment.
In D118511#3373173 <https://reviews.llvm.org/D118511#3373173>, @tstellar wrote:
> In D118511#3373082 <https://reviews.llvm.org/D118511#3373082>, @dblaikie wrote:
>
>> In D118511#3372728 <https://reviews.llvm.org/D118511#3372728>, @jyknight wrote:
>>
>>> In D118511#3371432 <https://reviews.llvm.org/D118511#3371432>, @tstellar wrote:
>>>
>>>> I'm fine with reverting if you think this is the best solution. I just would like to conclude soon so I can make the final release candidate.
>>>
>>> ISTM that reverting the ABI change in the 14.x branch makes sense, to avoid ping-ponging the ABI for packed structs which would become non-packed (breaking ABI) in 14.x and packed again (breaking ABI) in https://reviews.llvm.org/D119051.
>>
>> Yeah - I think it'd be a pretty niche amount of code that'd churn like that, but doesn't seem super important to rush this either.
>>
>> @tstellar - can/do you want to revert this on the release branch yourself? Is that something I should do? Should I revert this on trunk (would be a bit awkward/more churny for users - maybe not a full revert, but one that leaves the new ABI version flag available as a no-op so users opting out don't need to remove the flag only to add it back in later) so it can be integrated to the release?
>
> I can revert in the release branch. Is this the commit: https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6?
Yep, that's the one!
> It doesn't seem necessary to me to revert in the main branch, but I think that can be your call.
Yeah, I'm leaning towards not reverting on main & hoping we can sort out the POD ABI issue.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118511/new/
https://reviews.llvm.org/D118511
More information about the cfe-commits
mailing list