[PATCH] D133574: [C2x] reject type definitions in offsetof

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133574#3781907 <https://reviews.llvm.org/D133574#3781907>, @inclyc wrote:

> Use RAII object to maintain the Parser state
>
>> have you explored making a new DeclSpecContext and modifying isDefiningTypeSpecifierContext()? I think that would likely be a cleaner approach.
>
> Emm, I've tried passing a DeclaratorContext into `ParseTypeName()`
>
>   SourceLocation TypeLoc = Tok.getLocation();
>   InBuiltInOffsetOfBaseRAIIObject InOffsetof(*this, true);
>   TypeResult Ty = ParseTypeName(nullptr, /*Context=???*/); 
>
> But defining a new DeclaratorContext I have to complete a bunch of `case`
> statements.
>
>   // Parser.h
>   static bool isTypeSpecifier(DeclSpecContext DSC);
>   static AllowDefiningTypeSpec isDefiningTypeSpecifierContext(DeclSpecContext DSC, bool IsCPlusPlus);
>   static bool isOpaqueEnumDeclarationContext(DeclSpecContext DSC);
>   /* ... */    
>
> And I think it is somehow strange to determine these properties within
> __builtin_offsetof()? I'm not sure if it is really appropriate to define a
> special context for a built-in function. This place should only need to
> forbidden definitions, right?

The thing is: it really is a declaration context; one in which type declarations are not allowed. So yes, you do have to fill out a bunch of fully covered switches in places, but I still think that's the correct way to model this instead of introducing a one-off RAII object.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133574/new/

https://reviews.llvm.org/D133574