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

Fri Jan 27 05:32:59 PST 2023

aaron.ballman added a comment.

In D133574#4084215 <https://reviews.llvm.org/D133574#4084215>, @stilor wrote:

> In D133574#4083742 <https://reviews.llvm.org/D133574#4083742>, @aaron.ballman wrote:
>
>> In D133574#4083724 <https://reviews.llvm.org/D133574#4083724>, @asbirlea wrote:
>>
>>> Following this change, the following emits warnings: https://godbolt.org/z/cvGdMWqEa, https://godbolt.org/z/GhTP85WzE
>>>
>>> Can you please fix fwd or revert until resolved?
>>
>> What do you expect to be resolved? You're passing `-Wgnu-offsetof-extensions` which is warning you about the extensions, and `-Werror` is turning them from a warning into an error, so you're getting the behavior I'd expect based on: https://reviews.llvm.org/D133574#4059845
>
> Can you please explain how these warnings are related to the N2350? From what I see in N2350 text, it was only declaring new type definitions inside of `offsetof` to be undefined behavior. Can you please point to the language in N2350 that prohibits member access expressions like `offsetof(x, y.z)` or `offsetof(x, y[1])`?
>
> In fact, a change <https://reviews.llvm.org/D40267> some time ago explicitly removed the warning for what it called an "non-identifier designator (e.g. `offsetof(x, a.b[c])`" in response to DR496. The language from DR496 hasn't changed in the most recent publicly available draft of the standard (which includes both DR496 and N2350), so why are these "non-identifier designators" considered extensions now?

Thank you for pointing this out, you're right that there's definitely some confusion here and I didn't explain it very well.

DR496 was considered not a defect: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_496 and said "There was no discussion asserting that this extension, however, was the actual intent of the standard, and as such there is was no sentiment to accept these extensions with clarified wording. Such a change could only be made in a new revision of the standard.", and the current wording of the C2x standard matches that of the C17 (still says member-designator and not subobject).

>From my notes on the discussions of this DR:

Albuquerque 2017: "Next we moved on to DR 496 which is about offsetof. There was an open action item on this which was not written down, but the verbal report was that we should be able to safely talk about subobjects. We will leave this in open status and look at words later."
Pittsburgh 2017: "We did hit a procedural issue when dealing with DR 496 where the information from a previous PCR was not carried forward from one meeting to the next and there was some confusion as to what the "response" really is. We did mention that we want the last committee response to always be the definitive one, rather than making people pull together the history from separate locations. This is the way we usually do it, and this may have just been a process hiccup."
London 2018: "We looked at an open DR for 496 which we elected to mark C2x."

So by my understanding, my original changes removing the extension warning (in D40267 <https://reviews.llvm.org/D40267>) were jumping the gun because the committee never made the change we said we'd make. So I believe this is still an extension as far as C conformance is concerned. That said, I'll check with the convener to see if he'd be too frustrated if I filed a CD2 comment asking for `member-designator` to be replaced with `subobject` based on prior discussion, so maayyyybbeee we can fix this for C2x still.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574