aaron.ballman added a comment. In D133574#4047677 <https://reviews.llvm.org/D133574#4047677>, @inclyc wrote:
> ping :) Oh! I missed your pings because this was already marked as accepted, I'm sorry for the delay in answering your question. In D133574#3994050 <https://reviews.llvm.org/D133574#3994050>, @inclyc wrote: > Hi @aaron.ballman! seems we are failing tests on recent change > 2fc5a3410087c209567c7e4a2c457b6896c127a3 > <https://reviews.llvm.org/rG2fc5a3410087c209567c7e4a2c457b6896c127a3> > > /* The DR asked a question about whether defining a new type within offsetof > * is allowed. C2x N2350 made this explicitly undefined behavior, but Clang > * has always supported defining a type in this location, and GCC also > * supports it. > */ > (void)__builtin_offsetof(struct S { int a; }, a); > > Do you think this patch should still be merged into mainline? Or do we turn > this error into a warning? We don't currently document `__builtin_offsetof` and GCC's documentation (https://gcc.gnu.org/onlinedocs/gcc/Offsetof.html) does not explicitly claim they support this behavior, so I'm fine with diagnosing this UB when we previously didn't. If we find this breaks significant code in practice, maybe we'll come to another decision, but for now I'd say you should update that test to expect the error (and update the comment as well). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
