[Clang] Remove IDNS_Ordinary flag in IndirectFieldDecl::IdentifierNamespace

[zwuis]

There is a IDNS_Ordinary flag in IndirectFieldDecl::IdentifierNamespace so that members in nested anonymous struct/union can be found as ordinary identifiers.

struct S {
  struct { int x; };
  // Previous behaviour: `x` in previous line is found
  // Expected: nothing is found
  int arr[sizeof(x)];
};

This PR fixes this issue.

Fixes #31295.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Yanzuo Liu (zwuis)

Changes

There is a IDNS_Ordinary flag in IndirectField::IdentifierNamespace so that members in nested anonymous struct/union can be found as ordinary identifiers.

struct S {
  struct { int x; };
  // Previous behaviour: `x` in previous line is found
  // Expected: nothing is found
  int arr[sizeof(x)];
};

This PR fixes this issue.

Fixes #31295.

---

Full diff: https://github.com/llvm/llvm-project/pull/100525.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/AST/DeclBase.cpp (+1-2)
• (added) clang/test/Parser/namelookup-anonymous-struct.c (+6)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 83fd5dc31547e..24be9e26a9adf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -137,6 +137,9 @@ Miscellaneous Bug Fixes
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Fixed a crash in C due to incorrect lookup that members in nested anonymous struct/union
+  can be found as ordinary identifiers in struct/union definition. (#GH31295)
+
 OpenACC Specific Changes
 ------------------------
 
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index bc5a9206c0db2..a1f70546bde42 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -879,8 +879,6 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) {
       return IDNS_Ordinary;
     case Label:
       return IDNS_Label;
-    case IndirectField:
-      return IDNS_Ordinary | IDNS_Member;
 
     case Binding:
     case NonTypeTemplateParm:
@@ -918,6 +916,7 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) {
       return IDNS_ObjCProtocol;
 
     case Field:
+    case IndirectField:
     case ObjCAtDefsField:
     case ObjCIvar:
       return IDNS_Member;
diff --git a/clang/test/Parser/namelookup-anonymous-struct.c b/clang/test/Parser/namelookup-anonymous-struct.c
new file mode 100644
index 0000000000000..cb691c22f97ff
--- /dev/null
+++ b/clang/test/Parser/namelookup-anonymous-struct.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -std=c11 -verify %s
+
+struct GH31295 {
+  struct { int x; };
+  int arr[sizeof(x)]; // expected-error{{use of undeclared identifier 'x'}}
+};

[Sirraide]

Looks reasonable, but I’d prefer it if someone more familiar w/ the C standard than me could take another look at this.
CC @AaronBallman

[AaronBallman]

The C standard does not make this particularly clear, but the changes are correct.

Per C23 6.2.1p4 Each member of the structure has file scope (because why not) but per 6.2.3p1, each member is in a distinct name space and the member is disambiguated by the type of the expression used to access the member via the . or -> operator. That last bit is why the member cannot be an ordinary identifier within the structure itself.

So LGTM, thank you!

[zwuis]

Thank you for your review!

I don't have commit access. Please help me merge this PR if it's ready.

[cor3ntin]

@AaronBallman @Sirraide Do we have tests for C++? https://godbolt.org/z/bdvrdKso1

[Sirraide]

@AaronBallman @Sirraide Do we have tests for C++? https://godbolt.org/z/bdvrdKso1

I don’t believe the standard supports anonymous structs in C++, but it seems we’re handling them the same way as GCC, i.e. the code in the godbolt link is accepted in C++ mode by both us and GCC.

That said, I don’t know whether we have tests for this off the top of my head, but I should hope so considering that we support the feature...

[Sirraide]

Well, looks like I spoke too soon, because I can’t find any C++ tests for this particular case

[zwuis]

I don’t believe the standard supports anonymous structs in C++

Do we need tests about anonymous union? C++ standard supports it.

[AaronBallman]

I don’t believe the standard supports anonymous structs in C++

Do we need tests about anonymous union? C++ standard supports it.

Yes, we should add tests for C++ as well; good call @cor3ntin and @Sirraide!