Hi Jason, On Tue May 6, 2025 at 10:36 PM CEST, Jason Merrill wrote: > On 5/6/25 3:05 PM, Simon Martin wrote: >> The following test case highlights two issues - see >> https://godbolt.org/z/7E1KGYreh: >> 1. We error out at both L4 and L5, while (at least) clang, EDG and MSVC >> only reject L5 >> 2. Our error message for L5 incorrectly mentions using a null pointer >> >> === cut here === >> struct A { int i; }; >> struct C : virtual public A { }; >> void foo () { >> auto res = ((C*)0)->i; // L4 > > I assume you meant to include a & in there. I did not, but I agree it'd make the code a bit less nonsensical. I'll make sure to check that the error/warnings we might emit with a & make sense in the next iteration.
>> __builtin_offsetof (C, i); // L5 >> } >> === cut here === >> >> I am not aware of anything in the standard that'd make L4 invalid, > > I think it's undefined under https://eel.is/c++draft/expr#ref-9 and/or > https://eel.is/c++draft/expr#unary.op-1 > > There's also no reasonable answer, because the offset of a virtual base > depends on the most-derived type of the object. This is a bad question > to ask, which is why it's ill-formed for offsetof. I wonder why the OP > wants to write this code? It's definitely weird code; surprisingly enough none of clang, EDG or MSVC emit any warning for it (https://godbolt.org/z/z7rYjzEWo). > But I guess pedantically we shouldn't error out on code with undefined > behavior, and the diagnostic for L4 should only be a warning. The fact that we should not error out was my thinking as well, but you are right that we should warn the user about what they're doing. I'll rework the patch to warn for L4, and error out for L5 without talking about any null pointer. Thanks, Simon
