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:
> > 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!
> I was wrong, we need another approach.
>
> I think the grammar is actually
> ```
> member-designator:
> qualified-id
> member-designator.qualified-id
> member-designator.qualified-id
> ```
> IE, we should support https://godbolt.org/z/eEq8snMc8
>
> Unfortunately, this looks like a much bigger change that we envisioned when
> we tagged this as a good first issue, to the extent I'm not sure what is
> actually the right design is would be.
>
> For each component I imagine we want to store
> `NestedNameSpecifierLoc + DeclarationNameInfo`
>
> The parser would have to produce a CXXScopeSpec + UnqualifiedId pair for each
> component.
>
> The expression is dependent if any of the component is type dependent,
>
> `OffsetOfNode` would have to change, but i think we can get away
> Only changing the identifier case (ie the dependent case)
>
Would it be better for me to close this patch and submit a new one if I find
out how to implement it? I hope others won't hesitate to contribute because I'm
working on this. I don't want to cause any delays in the release plan!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157201/new/
https://reviews.llvm.org/D157201
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
