aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Parse/Parser.h:2311 case DeclSpecContext::DSC_association: + case DeclSpecContext::DSC_offsetof: return true; ---------------- inclyc wrote: > aaron.ballman wrote: > > Is this correct? I don't think we can deduce the type from `offsetof` > > through CTAD, can we? > > > > https://godbolt.org/z/Kab6ahYe7 > > > > (This might be a good test case to add assuming we don't already have that > > coverage.) > Emm, these checks just return as the same as "type_specifier". Because that's > what we passed into "ParsingTypename" before. Hmmm, that's certainly reasonable. Can you add the test case and make sure the behavior doesn't change? ================ 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: > 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.... 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