[PATCH] D133574: [C2x] reject type definitions in offsetof
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 13 09:01:55 PST 2023
aaron.ballman reopened this revision.
aaron.ballman added reviewers: clang-vendors, mgorny, thesamesam.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
In D133574#4051782 <https://reviews.llvm.org/D133574#4051782>, @bkramer wrote:
> GitHub finds around 1.9k instances of this pattern to compute `alignof`. There's a lot of duplicates and `#ifdefs` so the real number is going to be smaller, but it seems to be quite common. The annoying part is that there is no `_Alignof` in C99 so for many of those projects there is no easy fix.
>
> Can this be demoted to a warning?
>
> https://github.com/search?type=code&q=%2Foffsetof%5Cs*%5C%28%5Cs*struct%5Cs*%5C%7B%2F+lang%3Ac
Adding clang-vendors/others with opinions on diagnostic severity because this could be disruptive.
1. Yes, we can.
2. Blarg. I have sympathy because the standard was unclear on this point for so long. I don't have as much sympathy for "there's no easy fix" -- the code is not portable as-is (it's UB), and Clang supports `_Alignof` in C99 and earlier, so for Clang there is an easy fix.
3. Instead, should we downgrade to a warning-defaults-to-error instead of just a warning? That gives these folks a path forward while still making it clear to users "don't do this." But if we think we won't reasonably be able to turn this into an error, I think a warning is better.
4. Alternatively, should we document that we support this as an extension and only give an extension warning for it?
Regardless, we need to either make a decision shortly or we should revert these changes until after the Clang 16 branch and re-land after that to give ourselves more time to make a decision. If we don't have an obvious "this is the right approach" answer and an implementation by Jan 19 (next Thursday), I think we should revert to give ourselves breathing room.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133574/new/
https://reviews.llvm.org/D133574
More information about the cfe-commits
mailing list