[PATCH] D133574: [C2x] reject type definitions in offsetof

Fri Jan 13 09:44:40 PST 2023

philnik added a comment.

In D133574#4051899 <https://reviews.llvm.org/D133574#4051899>, @aaron.ballman wrote:

> 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.

I'm not a vendor, but another option would be to make it a warning-defaults-to-error and indicate that it will be promoted to an error in a few (2?) releases. That makes it less painful, but still make it an error eventually. I don't know if clang has done anything like this before though.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133574/new/

https://reviews.llvm.org/D133574