[PATCH] D157201: [Clang] Support qualified name as member designator in offsetof

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/test/SemaCXX/offsetof.cpp:106
+int x3[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1]; // expected-error{{no member named 'static_a'}}
+int x4[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // expected-error{{no member named 'X2'}}
+
----------------
yichi170 wrote:
> hubert.reinterpretcast wrote:
> > aaron.ballman wrote:
> > > yichi170 wrote:
> > > > aaron.ballman wrote:
> > > > > There's one more test I'd like to see:
> > > > > ```
> > > > > struct S {
> > > > >   int Foo;
> > > > > };
> > > > > 
> > > > > template <typename Ty>
> > > > > void func() {
> > > > >   static_assert(__builtin_offsetof(Ty, Ty::Foo) == 0, "");
> > > > > }
> > > > > 
> > > > > void inst() {
> > > > >   func<S>();
> > > > > }
> > > > > ```
> > > > It would get the compile error in the current patch, but I think it should be compiled without any error, right?
> > > Correct, that should be accepted: https://godbolt.org/z/1f6a9Yaxa
> > Should expect this to pass too:
> > ```
> > template <typename T>
> > struct Z {
> >   static_assert(!__builtin_offsetof(T, template Q<T>::x));
> > };
> > 
> > struct A {
> >   template <typename T> using Q = T;
> >   int x;
> > };
> > 
> > Z<A> za;
> > ```
> Wow. Does it mean we cannot simply parse the identifier, `::`, `.` and brackets in `__builtin_offsetof`?
GCC seems to support that. 

We probably want to call `ParseOptionalCXXScopeSpecifier` and store the `NestedNameSpecifierLoc` we'd get from it (and then parse the (sequence of) identifier(s) corresponding to the member) as we do now.

The documentation of https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Offsetof.html#index-_005f_005fbuiltin_005foffsetof 
seems inaccurate,

it seems to be

`"__builtin_offsetof" "(" typename ","  nested-name-specifier offsetof_member_designator ")"`


Note that you will have to take care of transforming the nested name in TreeTransform and handle type dependencies. Let me know if you have further questions,
it's more involved than what you signed for. Sorry for not spotting that earlier (Thanks @hubert.reinterpretcast !)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201