[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