[Clang] prevent setting default lexical access specifier for missing primary declarations

[a-tarasyuk]

Fixes #112208

[llvmbot]

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #112208

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2)
• (modified) clang/lib/Sema/SemaAccess.cpp (+2-1)
• (modified) clang/test/SemaCXX/enum.cpp (+8)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 817e3abef8d566..64ffdcde045a3a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -517,6 +517,8 @@ Bug Fixes to C++ Support
   certain situations. (#GH47400), (#GH90896)
 - Fix erroneous templated array size calculation leading to crashes in generated code. (#GH41441)
 - During the lookup for a base class name, non-type names are ignored. (#GH16855)
+- Fixed an assertion failure when the default lexical access specifier was set for missing
+  primary declarations. (#GH112208)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index df6edb21a50dee..8b4a5b70669d84 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -39,7 +39,8 @@ bool Sema::SetMemberAccessSpecifier(NamedDecl *MemberDecl,
                                     AccessSpecifier LexicalAS) {
   if (!PrevMemberDecl) {
     // Use the lexical access specifier.
-    MemberDecl->setAccess(LexicalAS);
+    if (LexicalAS != AS_none)
+      MemberDecl->setAccess(LexicalAS);
     return false;
   }
 
diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp
index 9c398cc8da886c..44042d8bf5cfc8 100644
--- a/clang/test/SemaCXX/enum.cpp
+++ b/clang/test/SemaCXX/enum.cpp
@@ -143,3 +143,11 @@ struct PR28903 {
     })
   };
 };
+
+namespace GH112208 {
+class C {
+  enum E { e = 0 };
+  void f(int, enum E;); // expected-error {{ISO C++ forbids forward references to 'enum' types}} \
+                        // expected-error {{unexpected ';' before ')'}}
+};
+}

[a-tarasyuk]

@shafik #112208 (comment) when PrevMemberDecl exists but isn't equal to LexicalAS — with AS_none excluded

llvm-project/clang/lib/Sema/SemaAccess.cpp

Line 48 in 0205667

if (LexicalAS != AS_none && LexicalAS != PrevMemberDecl->getAccess()) {

I suppose AS_none should be excluded when PrevMemberDecl is unknown. What do you think?

[shafik]

I feel like we should be catching this in Sema::ActOnTag here:

llvm-project/clang/lib/Sema/SemaDecl.cpp

Lines 17938 to 17951 in b497010

	if ((Name || Kind == TagTypeKind::Enum) &&
	getNonFieldDeclScope(S)->isFunctionPrototypeScope()) {
	if (getLangOpts().CPlusPlus) {
	// C++ [dcl.fct]p6:
	// Types shall not be defined in return or parameter types.
	if (TUK == TagUseKind::Definition && !IsTypeSpecifier) {
	Diag(Loc, diag::err_type_defined_in_param_type)
	<< Name;
	Invalid = true;
	}
	} else if (!PrevDecl) {
	Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
	}
	}

It looks like we are not b/c TUK is TUK_Declataion but we should not be forward declaring here either.

The current diagnostic does not fit this case though and maybe it has to be tweaked some more.

CC @AaronBallman

[shafik]

Please also add more details to the summary, it is helpful to have some idea what you think the underlying problem is and why this fix is correct before reviewing.

[a-tarasyuk]

The current diagnostic does not fit this case though and maybe it has to be tweaked some more.

@shafik Thanks for the feedback! I initially considered marking the new declaration invalid based on TUK_Declaration, but I wasn’t entirely sure because of its relation to the new declaration

llvm-project/clang/lib/Sema/SemaDecl.cpp

Lines 17953 to 17954 in 7be1dc0

	if (Invalid)
	New->setInvalidDecl();

I’ve made changes to handle this case based on TUK_Declaration. If we need to add a diagnostic message (something like a general message saying "enum" cannot appear here), just let me know and I’ll add it...

[a-tarasyuk]

I need to check the tests on Windows to ensure these changes aren't causing any issues, as the CI failed during the Windows checks...