yichi170 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'}}
+
----------------
cor3ntin wrote:
> 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 !)
Thank you for all the help! I'll take a look at it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to