[clang] No longer add enumeration constants to the wrong scope (PR #134998)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

<details>
<summary>Changes</summary>

Previously, the enumerators were being added both to the class context and to the namespace scope. e.g., we accepted this invalid code:
```
  struct A {
    enum E : int;
  };

  enum A::E : int  { e1 = 100, e2 };

  int func() {
    return e1; // Was accepted, now correctly rejected
  }
```
Fixes #<!-- -->23317

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


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+5) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+4-1) 
- (modified) clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp (+4-4) 
- (modified) clang/test/SemaCXX/enum.cpp (+18) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e671183522565..555fa1955ba45 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -421,6 +421,11 @@ Bug Fixes to C++ Support
 - Clang now issues an error when placement new is used to modify a const-qualified variable
   in a ``constexpr`` function. (#GH131432)
 - Clang now emits a warning when class template argument deduction for alias templates is used in C++17. (#GH133806)
+- No longer add enumerators to the scope chain when the enumeration is declared
+  within a class context but is defined out of line. Previously, the
+  enumerators were being added both to the class context and to the namespace
+  scope. (#GH23317)
+
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 540f5f23fe89a..fd853f7be74f1 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1523,7 +1523,10 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S, bool AddToContext) {
 
   // Out-of-line definitions shouldn't be pushed into scope in C++, unless they
   // are function-local declarations.
-  if (getLangOpts().CPlusPlus && D->isOutOfLine() && !S->getFnParent())
+  bool OutOfLine = D->isOutOfLine();
+  if (const auto *ECD = dyn_cast<EnumConstantDecl>(D))
+    OutOfLine = OutOfLine || cast<Decl>(ECD->getDeclContext())->isOutOfLine();
+  if (getLangOpts().CPlusPlus && OutOfLine && !S->getFnParent())
     return;
 
   // Template instantiations should also not be pushed into scope.
diff --git a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
index e5807993a7a18..f41f3f39f2784 100644
--- a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
@@ -12,7 +12,7 @@ A<int> a;
 A<int>::E a0 = A<int>().v;
 int n = A<int>::E::e1; // expected-error {{implicit instantiation of undefined member}}
 
-template<typename T> enum A<T>::E : T { e1, e2 }; // expected-note 2 {{declared here}}
+template<typename T> enum A<T>::E : T { e1, e2 };
 
 // FIXME: Now that A<T>::E is defined, we are supposed to inject its enumerators
 // into the already-instantiated class A<T>. This seems like a really bad idea,
@@ -20,7 +20,7 @@ template<typename T> enum A<T>::E : T { e1, e2 }; // expected-note 2 {{declared
 //
 // Either do as the standard says, or only include enumerators lexically defined
 // within the class in its scope.
-A<int>::E a1 = A<int>::e1; // expected-error {{no member named 'e1' in 'A<int>'; did you mean simply 'e1'?}}
+A<int>::E a1 = A<int>::e1; // expected-error {{no member named 'e1' in 'A<int>'}}
 
 A<char>::E a2 = A<char>::e2;
 
@@ -43,7 +43,7 @@ B<int>::E b1 = B<int>::E::e1;
 
 B<char>::E b2 = B<char>::E::e2;
 
-template<typename T> typename B<T>::E B<T>::g() { return e2; }
+template<typename T> typename B<T>::E B<T>::g() { return e2; } // expected-error {{use of undeclared identifier 'e2'}}
 B<short>::E b3 = B<short>().g();
 
 
@@ -94,7 +94,7 @@ D<int>::E d1 = D<int>::E::e1; // expected-error {{incomplete type 'D<int>::E'}}
 template<> enum class D<int>::E { e2 };
 D<int>::E d2 = D<int>::E::e2;
 D<char>::E d3 = D<char>::E::e1; // expected-note {{first required here}}
-D<char>::E d4 = D<char>::E::e2; // expected-error {{no member named 'e2' in 'D<char>::E'; did you mean simply 'e2'?}}
+D<char>::E d4 = D<char>::E::e2; // expected-error {{no member named 'e2' in 'D<char>::E'}}
 template<> enum class D<char>::E { e3 }; // expected-error {{explicit specialization of 'E' after instantiation}}
 
 template<> enum class D<short>::E;
diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp
index 44042d8bf5cfc..e2cda23269d76 100644
--- a/clang/test/SemaCXX/enum.cpp
+++ b/clang/test/SemaCXX/enum.cpp
@@ -151,3 +151,21 @@ class C {
                         // expected-error {{unexpected ';' before ')'}}
 };
 }
+
+#if __cplusplus >= 201103L
+namespace GH23317 {
+struct A {
+  enum E : int;
+  constexpr int foo() const;
+};
+
+enum A::E : int  { ae1 = 100, ae2 }; // expected-note {{'A::ae1' declared here}}
+
+constexpr int A::foo() const { return ae1; } // This is fine
+static_assert(A{}.foo() == 100, "oh no");
+
+int foo() {
+  return ae1; // expected-error {{use of undeclared identifier 'ae1'; did you mean 'A::ae1'?}}
+}
+} // namespace GH23317
+#endif // __cplusplus >= 201103L

``````````

</details>


https://github.com/llvm/llvm-project/pull/134998