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

Reply via email to