aaron.ballman added inline comments.
================ Comment at: clang/test/Sema/offsetof.c:79 + int a; + struct B // no-error, struct B is not defined within __builtin_offsetof directly + { ---------------- inclyc wrote: > inclyc wrote: > > aaron.ballman wrote: > > > inclyc wrote: > > > > aaron.ballman wrote: > > > > > I think this is defensible. The wording in the standard is "If the > > > > > specified type defines a new type or if the specified member is a > > > > > bit-field, the behavior is undefined." and the specified type in this > > > > > case is `struct A`; that `struct A` happens to also define `struct B` > > > > > is immaterial. > > > > > > > > > > However, the intent behind the change to the rule is to support older > > > > > implementations of `offsetof` to protect them from having to deal > > > > > with a case like: `offsetof(struct S { int a, b }, b);` where > > > > > `offsetof` is a macro and thus the comma between `a` and `b` is > > > > > treated as a separator. So there's a part of me that wonders if we > > > > > want to also support diagnosing this case. But then we'd have to look > > > > > at the declarator context more recursively to see whether any of the > > > > > contexts on the stack are an `offsetof` context and that might be > > > > > tricky. > > > > > > > > > > Thoughts? > > > > FWIW, gcc seems just rejects all definitions in this context. (Perhaps > > > > during Parsing the statements). If we add a bool state to the Parser > > > > (just using RAII object as before) struct B will trigger diagnostic > > > > error because the state "ParsingOffsetof" is passed into inner > > > > declaration. > > > GCC accepts currently: https://godbolt.org/z/oEvzjW6Ee but you're correct > > > regarding switching back to an RAII object being an easier way to address > > > the nested declarations. > > > > > > Let me think on this situation a bit.... > > > GCC accepts currently > > > > C++: https://godbolt.org/z/fon8e7dzf > ``` > <source>: In function 'int main()': > <source>:3:3: error: types may not be defined within '__builtin_offsetof' > 3 | { > | ^ > <source>:6:5: error: types may not be defined within '__builtin_offsetof' > 6 | { > | ^ > Compiler returned: 1 > ``` C++ is a different language in this case though. In C, you can generally define types anywhere you can spell a type, and in C++ you cannot. e.g., `void func(struct S { int x, y; } s);` is valid in C and invalid in C++. 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