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

[Yichi Lee via Phabricator via cfe-commits]

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 
> > > > > > > > void func() {
> > > > > > > >   static_assert(__builtin_offsetof(Ty, Ty::Foo) == 0, "");
> > > > > > > > }
> > > > > > > > 
> > > > > > > > void inst() {
> > > > > > > >   func();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > 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 
> > > > > struct Z {
> > > > >   static_assert(!__builtin_offsetof(T, template Q::x));
> > > > > };
> > > > > 
> > > > > struct A {
> > > > >   template  using Q = T;
> > > > >   int x;
> > > > > };
> > > > > 
> > > > > Z 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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yichi Lee via Phabricator via cfe-commits]

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 
> > > > > > void func() {
> > > > > >   static_assert(__builtin_offsetof(Ty, Ty::Foo) == 0, "");
> > > > > > }
> > > > > > 
> > > > > > void inst() {
> > > > > >   func();
> > > > > > }
> > > > > > ```
> > > > > 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 
> > > struct Z {
> > >   static_assert(!__builtin_offsetof(T, template Q::x));
> > > };
> > > 
> > > struct A {
> > >   template  using Q = T;
> > >   int x;
> > > };
> > > 
> > > Z 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

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 added a comment.

Is there any code handling the nested qualifier that I can reference? I tried 
to modify the `OffsetOfNode`, which allows me to handle the basic template case 
but still failed on the nested case.


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

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

[Yichi Lee via Phabricator via cfe-commits]

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'}}
+

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 
> > > > void func() {
> > > >   static_assert(__builtin_offsetof(Ty, Ty::Foo) == 0, "");
> > > > }
> > > > 
> > > > void inst() {
> > > >   func();
> > > > }
> > > > ```
> > > 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 
> struct Z {
>   static_assert(!__builtin_offsetof(T, template Q::x));
> };
> 
> struct A {
>   template  using Q = T;
>   int x;
> };
> 
> Z za;
> ```
Wow. Does it mean we cannot simply parse the identifier, `::`, `.` and brackets 
in `__builtin_offsetof`?


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

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 marked an inline comment as done.
yichi170 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:59
 
+- Improved ``__builtin_offsetof`` support, allowing qualified name in member 
designator. This fixes [Issue 
64154](https://github.com/llvm/llvm-project/issues/64154).
 

aaron.ballman wrote:
> 
> 





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'}}
+

aaron.ballman wrote:
> There's one more test I'd like to see:
> ```
> struct S {
>   int Foo;
> };
> 
> template 
> void func() {
>   static_assert(__builtin_offsetof(Ty, Ty::Foo) == 0, "");
> }
> 
> void inst() {
>   func();
> }
> ```
It would get the compile error in the current patch, but I think it should be 
compiled without any error, right?


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

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 updated this revision to Diff 548632.
yichi170 marked 4 inline comments as done.
yichi170 added a comment.

Applied the suggestions. Thanks for giving me feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/offsetof.cpp

Index: clang/test/SemaCXX/offsetof.cpp
===
--- clang/test/SemaCXX/offsetof.cpp
+++ clang/test/SemaCXX/offsetof.cpp
@@ -98,3 +98,10 @@
 B x;
   }, a);
 }
+
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; static int static_a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+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'}}
+
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11028,7 +11028,7 @@
   for (unsigned I = 0, N = E->getNumComponents(); I != N; ++I) {
 const OffsetOfNode  = E->getComponent(I);
 Component Comp;
-Comp.isBrackets = true;
+Comp.Kind = Sema::OffsetOfComponent::Brackets;
 Comp.LocStart = ON.getSourceRange().getBegin();
 Comp.LocEnd = ON.getSourceRange().getEnd();
 switch (ON.getKind()) {
@@ -11039,14 +11039,14 @@
 return ExprError();
 
   ExprChanged = ExprChanged || Index.get() != FromIndex;
-  Comp.isBrackets = true;
+  Comp.Kind = Sema::OffsetOfComponent::Brackets;
   Comp.U.E = Index.get();
   break;
 }
 
 case OffsetOfNode::Field:
 case OffsetOfNode::Identifier:
-  Comp.isBrackets = false;
+  Comp.Kind = Sema::OffsetOfComponent::Identifier;
   Comp.U.IdentInfo = ON.getFieldName();
   if (!Comp.U.IdentInfo)
 continue;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16612,7 +16612,7 @@
   SmallVector Comps;
   SmallVector Exprs;
   for (const OffsetOfComponent  : Components) {
-if (OC.isBrackets) {
+if (OC.Kind == OffsetOfComponent::Brackets) {
   // Offset of an array sub-field.  TODO: Should we allow vector elements?
   if (!CurrentType->isDependentType()) {
 const ArrayType *AT = Context.getAsArrayType(CurrentType);
@@ -16682,6 +16682,10 @@
   << SourceRange(Components[0].LocStart, OC.LocEnd)
   << CurrentType))
 DidWarnAboutNonPOD = true;
+
+  if (OC.Kind == OffsetOfComponent::Qualifier &&
+  RD->getIdentifier() == OC.U.IdentInfo)
+continue;
 }
 
 // Look for the field.
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2635,16 +2635,23 @@
 SmallVector Comps;
 
 Comps.push_back(Sema::OffsetOfComponent());
-Comps.back().isBrackets = false;
+Comps.back().Kind = Sema::OffsetOfComponent::Identifier;
 Comps.back().U.IdentInfo = Tok.getIdentifierInfo();
 Comps.back().LocStart = Comps.back().LocEnd = ConsumeToken();
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' identifier
+if (Tok.is(tok::coloncolon) && getLangOpts().CPlusPlus) {
+  Comps.back().Kind = Sema::OffsetOfComponent::Qualifier;
+} else if (Tok.is(tok::coloncolon) && !getLangOpts().CPlusPlus) {
+  Res = ExprError();
+  break;
+}
+
 Comps.push_back(Sema::OffsetOfComponent());
-Comps.back().isBrackets = false;
+Comps.back().Kind = Sema::OffsetOfComponent::Identifier;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2660,7 +2667,7 @@
 
 // offsetof-member-designator: offsetof-member-design '[' expression ']'
 Comps.push_back(Sema::OffsetOfComponent());
-Comps.back().isBrackets = true;
+Comps.back().Kind = Sema::OffsetOfComponent::Brackets;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6035,11 +6035,15 @@
   // 

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 updated this revision to Diff 547764.
yichi170 marked an inline comment as not done.
yichi170 added a comment.

updated test in `test/SemaCXX/offsetof.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/offsetof.c
  clang/test/SemaCXX/offsetof.cpp


Index: clang/test/SemaCXX/offsetof.cpp
===
--- clang/test/SemaCXX/offsetof.cpp
+++ clang/test/SemaCXX/offsetof.cpp
@@ -98,3 +98,7 @@
 B x;
   }, a);
 }
+
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; static int static_a; };
+int x2[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1]; // 
expected-error{{no member named 'static_a'}}
Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // 
expected-warning{{indirection of non-volatile null pointer}} 
expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // 
expected-error{{no member named 'X2'}}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16663,6 +16663,9 @@
<< CurrentType);
 RecordDecl *RD = RC->getDecl();
 
+if (OC.isQualifier && RD->getIdentifier() == OC.U.IdentInfo)
+  continue;
+
 // C++ [lib.support.types]p5:
 //   The macro offsetof accepts a restricted set of type arguments in this
 //   International Standard. type shall be a POD structure or a POD union
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' 
identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression 
']'
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = true;
+Comps.back().isQualifier = false;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6036,6 +6036,7 @@
   struct OffsetOfComponent {
 SourceLocation LocStart, LocEnd;
 bool isBrackets;  // true if [expr], false if .ident
+bool isQualifier;
 union {
   IdentifierInfo *IdentInfo;
   Expr *E;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -56,6 +56,7 @@
 
 C++ Language Changes
 
+- Improved `__builtin_offsetof` support, allowing qualified name in member 
designator.
 
 C++20 Feature Support
 ^


Index: clang/test/SemaCXX/offsetof.cpp
===
--- clang/test/SemaCXX/offsetof.cpp
+++ clang/test/SemaCXX/offsetof.cpp
@@ -98,3 +98,7 @@
 B x;
   }, a);
 }
+
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; static int static_a; };
+int x2[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1]; // expected-error{{no member named 'static_a'}}
Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // 

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 added inline comments.



Comment at: clang/test/Sema/offsetof.c:79
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // 
expected-error{{no member named 'X2'}}
+

Fznamznon wrote:
> It probably makes sense to also add a test with static member of a class, 
> like Aaron mentioned in GitHub - 
> https://github.com/llvm/llvm-project/issues/64154#issuecomment-1653468938 .
Hi @Fznamznon, I found that clang and clang++ output different errors with the 
following test.
```
struct X2 { int a; static int static_a; };
int x4[__builtin_offsetof(struct X2, X2::static_a) == 0 ? 1 : -1];
// expected-error{{no member named 'static_a'}}
```
C doesn't allow to have a static member in the struct, so is there any way to 
write the correct comment to make the test pass?


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

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 updated this revision to Diff 547703.
yichi170 added a comment.

updated ReleaseNotes and applied suggestions. The related issue #63443 
 is still not fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/offsetof.c


Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // 
expected-warning{{indirection of non-volatile null pointer}} 
expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // 
expected-error{{no member named 'X2'}}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16663,6 +16663,9 @@
<< CurrentType);
 RecordDecl *RD = RC->getDecl();
 
+if (OC.isQualifier && RD->getIdentifier() == OC.U.IdentInfo)
+  continue;
+
 // C++ [lib.support.types]p5:
 //   The macro offsetof accepts a restricted set of type arguments in this
 //   International Standard. type shall be a POD structure or a POD union
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' 
identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression 
']'
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = true;
+Comps.back().isQualifier = false;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6036,6 +6036,7 @@
   struct OffsetOfComponent {
 SourceLocation LocStart, LocEnd;
 bool isBrackets;  // true if [expr], false if .ident
+bool isQualifier;
 union {
   IdentifierInfo *IdentInfo;
   Expr *E;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -56,6 +56,7 @@
 
 C++ Language Changes
 
+- Improved `__builtin_offsetof` support, allowing qualified name in member 
designator.
 
 C++20 Feature Support
 ^


Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // expected-error{{no member named 'X2'}}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16663,6 +16663,9 @@
<< CurrentType);
 RecordDecl *RD = RC->getDecl();
 
+if (OC.isQualifier && RD->getIdentifier() == OC.U.IdentInfo)
+  continue;
+
 // C++ [lib.support.types]p5:
 //   The macro offsetof accepts a restricted set of type arguments in this
 //   International Standard. type shall be a POD structure or a POD union
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:16696-16697
+
+  IdentifierInfo *II = RD->getIdentifier();
+  if (II == OC.U.IdentInfo && OC.isQualifier)
+continue;

tbaeder wrote:
> 
I'll fix it along with the ReleaseNotes.


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

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 updated this revision to Diff 547542.
yichi170 added a comment.

[Clang] Support qualified name as member designator in offsetof


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157201

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/offsetof.c


Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // 
expected-warning{{indirection of non-volatile null pointer}} 
expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // 
expected-error{{no member named 'X2'}}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16692,6 +16692,10 @@
 if (!MemberDecl) {
   if ((IndirectMemberDecl = R.getAsSingle()))
 MemberDecl = IndirectMemberDecl->getAnonField();
+
+  IdentifierInfo *II = RD->getIdentifier();
+  if (II == OC.U.IdentInfo && OC.isQualifier)
+continue;
 }
 
 if (!MemberDecl) {
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' 
identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression 
']'
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = true;
+Comps.back().isQualifier = false;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6036,6 +6036,7 @@
   struct OffsetOfComponent {
 SourceLocation LocStart, LocEnd;
 bool isBrackets;  // true if [expr], false if .ident
+bool isQualifier;
 union {
   IdentifierInfo *IdentInfo;
   Expr *E;


Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -73,3 +73,8 @@
   return __builtin_offsetof(Array, array[*(int*)0]); // expected-warning{{indirection of non-volatile null pointer}} expected-note{{__builtin_trap}}
 }
 
+// https://github.com/llvm/llvm-project/issues/64154
+struct X2 { int a; };
+int x2[__builtin_offsetof(struct X2, X2::a) == 0 ? 1 : -1];
+int x3[__builtin_offsetof(struct X2, X2::X2) == 0 ? 1 : -1]; // expected-error{{no member named 'X2'}}
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16692,6 +16692,10 @@
 if (!MemberDecl) {
   if ((IndirectMemberDecl = R.getAsSingle()))
 MemberDecl = IndirectMemberDecl->getAnonField();
+
+  IdentifierInfo *II = RD->getIdentifier();
+  if (II == OC.U.IdentInfo && OC.isQualifier)
+continue;
 }
 
 if (!MemberDecl) {
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression ']'
 

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

[Yichi Lee via Phabricator via cfe-commits]

yichi170 created this revision.
Herald added a project: All.
yichi170 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157201

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16692,6 +16692,10 @@
 if (!MemberDecl) {
   if ((IndirectMemberDecl = R.getAsSingle()))
 MemberDecl = IndirectMemberDecl->getAnonField();
+
+  IdentifierInfo *II = RD->getIdentifier();
+  if (II == OC.U.IdentInfo && OC.isQualifier)
+continue;
 }
 
 if (!MemberDecl) {
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' 
identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression 
']'
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = true;
+Comps.back().isQualifier = false;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6036,6 +6036,7 @@
   struct OffsetOfComponent {
 SourceLocation LocStart, LocEnd;
 bool isBrackets;  // true if [expr], false if .ident
+bool isQualifier;
 union {
   IdentifierInfo *IdentInfo;
   Expr *E;


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16692,6 +16692,10 @@
 if (!MemberDecl) {
   if ((IndirectMemberDecl = R.getAsSingle()))
 MemberDecl = IndirectMemberDecl->getAnonField();
+
+  IdentifierInfo *II = RD->getIdentifier();
+  if (II == OC.U.IdentInfo && OC.isQualifier)
+continue;
 }
 
 if (!MemberDecl) {
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2641,10 +2641,13 @@
 
 // FIXME: This loop leaks the index expressions on error.
 while (true) {
-  if (Tok.is(tok::period)) {
+  if (Tok.is(tok::period) || Tok.is(tok::coloncolon)) {
 // offsetof-member-designator: offsetof-member-designator '.' identifier
+if (Tok.is(tok::coloncolon))
+  Comps.back().isQualifier = true;
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = false;
+Comps.back().isQualifier = false;
 Comps.back().LocStart = ConsumeToken();
 
 if (Tok.isNot(tok::identifier)) {
@@ -2661,6 +2664,7 @@
 // offsetof-member-designator: offsetof-member-design '[' expression ']'
 Comps.push_back(Sema::OffsetOfComponent());
 Comps.back().isBrackets = true;
+Comps.back().isQualifier = false;
 BalancedDelimiterTracker ST(*this, tok::l_square);
 ST.consumeOpen();
 Comps.back().LocStart = ST.getOpenLocation();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6036,6 +6036,7 @@
   struct OffsetOfComponent {
 SourceLocation LocStart, LocEnd;
 bool isBrackets;  // true if [expr], false if .ident
+bool isQualifier;
 union {
   IdentifierInfo *IdentInfo;
   Expr *E;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits