Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][Sema] Allow elaborated-type-specifiers that declare member class template explict specializations #78720

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

sdkrystian
Copy link
Member

According to [dcl.type.elab] p2:

If an elaborated-type-specifier is the sole constituent of a declaration, the declaration is ill-formed unless it is an explicit specialization, an explicit instantiation or it has one of the following forms [...]

Consider the following:

template<typename T>
struct A 
{
    template<typename U>
    struct B;
};

template<>
template<typename U>
struct A<int>::B; // #1

The elaborated-type-specifier at #1 declares an explicit specialization (which is itself a template). We currently (incorrectly) reject this, and this PR fixes that.

I moved the point at which elaborated-type-specifiers with nested-name-specifiers are diagnosed from ParsedFreeStandingDeclSpec to ActOnTag for two reasons: ActOnTag isn't called for explicit instantiations and partial/explicit specializations, and because it's where we determine if a member specialization is being declared.

With respect to diagnostics, I am currently issuing the diagnostic without marking the declaration as invalid or returning early, which results in more diagnostics that I think is necessary. I would like feedback regarding what the "correct" behavior should be here.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Jan 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 19, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Krystian Stasiowski (sdkrystian)

Changes

According to [[dcl.type.elab] p2](http://eel.is/c++draft/dcl.type.elab#2):
> If an elaborated-type-specifier is the sole constituent of a declaration, the declaration is ill-formed unless it is an explicit specialization, an explicit instantiation or it has one of the following forms [...]

Consider the following:

template&lt;typename T&gt;
struct A 
{
    template&lt;typename U&gt;
    struct B;
};

template&lt;&gt;
template&lt;typename U&gt;
struct A&lt;int&gt;::B; // #<!-- -->1

The elaborated-type-specifier at #<!-- -->1 declares an explicit specialization (which is itself a template). We currently (incorrectly) reject this, and this PR fixes that.

I moved the point at which elaborated-type-specifiers with nested-name-specifiers are diagnosed from ParsedFreeStandingDeclSpec to ActOnTag for two reasons: ActOnTag isn't called for explicit instantiations and partial/explicit specializations, and because it's where we determine if a member specialization is being declared.

With respect to diagnostics, I am currently issuing the diagnostic without marking the declaration as invalid or returning early, which results in more diagnostics that I think is necessary. I would like feedback regarding what the "correct" behavior should be here.


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

12 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-2)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+23-21)
  • (modified) clang/test/CXX/class.access/p4.cpp (+1)
  • (added) clang/test/CXX/dcl.dcl/dcl.enum/p1.cpp (+30)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p1.cpp (+65-1)
  • (modified) clang/test/CXX/drs/dr16xx.cpp (-1)
  • (modified) clang/test/CXX/module/module.interface/p2-2.cpp (+4)
  • (modified) clang/test/SemaCXX/enum-scoped.cpp (+2)
  • (modified) clang/test/SemaCXX/nested-name-spec.cpp (+7-4)
  • (modified) clang/test/SemaTemplate/elaborated-type-specifier.cpp (+2)
  • (modified) clang/test/SemaTemplate/qualified-id.cpp (+1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 392f694065a242b..a8a9f974a716041 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -821,6 +821,8 @@ Bug Fixes in This Version
 - Fix an issue with missing symbol definitions when the first coroutine
   statement appears in a discarded ``if constexpr`` branch.
   Fixes (`#78290 <https://github.com/llvm/llvm-project/issues/78290>`_)
+- Clang now accepts elaborated-type-specifiers that explicitly specialize
+  a member class template for an implicit instantiation of a class template.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 03b0122d1c08f75..e93f0281977ad0c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7129,8 +7129,7 @@ def warn_standalone_specifier : Warning<"'%0' ignored on this declaration">,
 def ext_standalone_specifier : ExtWarn<"'%0' is not permitted on a declaration "
   "of a type">, InGroup<MissingDeclarations>;
 def err_standalone_class_nested_name_specifier : Error<
-  "forward declaration of %select{class|struct|interface|union|enum|enum class|enum struct}0 cannot "
-  "have a nested name specifier">;
+  "forward declaration of %0 cannot have a nested name specifier">;
 def err_typecheck_sclass_func : Error<"illegal storage class on function">;
 def err_static_block_func : Error<
   "function declared in block scope cannot have 'static' storage class">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index cbb8ed8ce34d1bd..d7475f994825cda 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -5206,25 +5206,6 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
     return ActOnFriendTypeDecl(S, DS, TemplateParams);
   }
 
-  const CXXScopeSpec &SS = DS.getTypeSpecScope();
-  bool IsExplicitSpecialization =
-    !TemplateParams.empty() && TemplateParams.back()->size() == 0;
-  if (Tag && SS.isNotEmpty() && !Tag->isCompleteDefinition() &&
-      !IsExplicitInstantiation && !IsExplicitSpecialization &&
-      !isa<ClassTemplatePartialSpecializationDecl>(Tag)) {
-    // Per C++ [dcl.type.elab]p1, a class declaration cannot have a
-    // nested-name-specifier unless it is an explicit instantiation
-    // or an explicit specialization.
-    //
-    // FIXME: We allow class template partial specializations here too, per the
-    // obvious intent of DR1819.
-    //
-    // Per C++ [dcl.enum]p1, an opaque-enum-declaration can't either.
-    Diag(SS.getBeginLoc(), diag::err_standalone_class_nested_name_specifier)
-        << GetDiagnosticTypeSpecifierID(DS) << SS.getRange();
-    return nullptr;
-  }
-
   // Track whether this decl-specifier declares anything.
   bool DeclaresAnything = true;
 
@@ -17181,10 +17162,31 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
   // for non-C++ cases.
   if (TemplateParameterLists.size() > 0 ||
       (SS.isNotEmpty() && TUK != TUK_Reference)) {
-    if (TemplateParameterList *TemplateParams =
+    TemplateParameterList *TemplateParams =
             MatchTemplateParametersToScopeSpecifier(
                 KWLoc, NameLoc, SS, nullptr, TemplateParameterLists,
-                TUK == TUK_Friend, isMemberSpecialization, Invalid)) {
+                TUK == TUK_Friend, isMemberSpecialization, Invalid);
+
+    // C++23 [dcl.type.elab] p2:
+    //   If an elaborated-type-specifier is the sole constituent of a
+    //   declaration, the declaration is ill-formed unless it is an explicit
+    //   specialization, an explicit instantiation or it has one of the
+    //   following forms: [...]
+    // C++23 [dcl.enum] p1:
+    //   If the enum-head-name of an opaque-enum-declaration contains a
+    //   nested-name-specifier, the declaration shall be an explicit
+    //   specialization.
+    //
+    // FIXME: Class template partial specializations can be forward declared
+    // per CWG2213, but the resolution failed to allow qualified forward
+    // declarations. This is almost certainly unintentional, so we allow them.
+    if (TUK == TUK_Declaration && SS.isNotEmpty() && !isMemberSpecialization) {
+      Diag(SS.getBeginLoc(), diag::err_standalone_class_nested_name_specifier)
+          << TypeWithKeyword::getTagTypeKindName(Kind) << SS.getRange();
+      // Invalid = true;
+    }
+
+    if (TemplateParams) {
       if (Kind == TagTypeKind::Enum) {
         Diag(KWLoc, diag::err_enum_template);
         return true;
diff --git a/clang/test/CXX/class.access/p4.cpp b/clang/test/CXX/class.access/p4.cpp
index fef5b7aa88726ea..ca98c9f90bd89e7 100644
--- a/clang/test/CXX/class.access/p4.cpp
+++ b/clang/test/CXX/class.access/p4.cpp
@@ -611,6 +611,7 @@ namespace test21 {
   template <class T> class A<T>::Inner {};
   class B {
     template <class T> class A<T>::Inner; // expected-error{{non-friend class member 'Inner' cannot have a qualified name}}
+                                          // expected-error@-1{{forward declaration of class cannot have a nested name specifier}}
   };
 
   void test() {
diff --git a/clang/test/CXX/dcl.dcl/dcl.enum/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.enum/p1.cpp
new file mode 100644
index 000000000000000..d97859f96ff91c7
--- /dev/null
+++ b/clang/test/CXX/dcl.dcl/dcl.enum/p1.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -verify %s -std=c++11
+
+template<typename T>
+struct S0 {
+  enum E0 : int;
+
+  enum class E1;
+};
+
+struct S3 {
+  enum E2 : int;
+
+  enum class E3;
+};
+
+template<typename T>
+enum S0<T>::E0 : int; // expected-error{{cannot have a nested name specifier}}
+
+template<>
+enum S0<int>::E0 : int;
+
+template<typename T>
+enum class S0<T>::E1; // expected-error{{cannot have a nested name specifier}}
+
+template<>
+enum class S0<int>::E1;
+
+enum S3::E2 : int; // expected-error{{cannot have a nested name specifier}}
+
+enum class S3::E3; // expected-error{{cannot have a nested name specifier}}
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p1.cpp
index e3982fd6a8386fb..e2408cdc5a7a760 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p1.cpp
@@ -16,7 +16,7 @@ template<> struct N::B<int>;
 template struct N::B<float>;
 
 template<typename T> struct C;
-template<typename T> struct C<T*>; // FIXME: This is technically ill-formed, but that's not the intent.
+template<typename T> struct C<T*>;
 template<> struct C<int>;
 template struct C<float>;
 
@@ -24,3 +24,67 @@ template<typename T> struct D::A; // expected-error {{cannot have a nested name
 template<typename T> struct D::A<T*>; // FIXME: This is technically ill-formed, but that's not the intent.
 template<> struct D::A<int>;
 template struct D::A<float>;
+
+namespace qualified_decl {
+  template<typename T>
+  struct S0 {
+    struct S1;
+
+    template<typename U>
+    struct S2;
+
+    enum E0 : int;
+
+    enum class E1;
+  };
+
+  struct S3 {
+    struct S4;
+
+    template<typename T>
+    struct S5;
+
+    enum E2 : int;
+
+    enum class E3;
+  };
+
+  template<typename T>
+  struct S0<T>::S1; // expected-error{{cannot have a nested name specifier}}
+
+  template<>
+  struct S0<int>::S1;
+
+  template<typename T>
+  template<typename U>
+  struct S0<T>::S2; // expected-error{{cannot have a nested name specifier}}
+
+  template<typename T>
+  template<typename U>
+  struct S0<T>::S2<U*>;
+
+  template<>
+  template<>
+  struct S0<int>::S2<bool>;
+
+  template<>
+  template<typename U>
+  struct S0<int>::S2;
+
+  struct S3::S4; // expected-error{{cannot have a nested name specifier}}
+
+  template<typename T>
+  struct S3::S5; // expected-error{{cannot have a nested name specifier}}
+
+  struct S3::S4 f0();
+  enum S0<long>::E0 f1();
+  enum S0<long>::E1 f2();
+  enum S3::E2 f3();
+  enum S3::E3 f4();
+
+  using A0 = struct S3::S4;
+  using A1 = enum S0<long>::E0;
+  using A2 = enum S0<long>::E1;
+  using A3 = enum S3::E2;
+  using A4 = enum S3::E3;
+}
diff --git a/clang/test/CXX/drs/dr16xx.cpp b/clang/test/CXX/drs/dr16xx.cpp
index 1be4d2ab3ebe886..6ce77fbba7ceec4 100644
--- a/clang/test/CXX/drs/dr16xx.cpp
+++ b/clang/test/CXX/drs/dr16xx.cpp
@@ -99,7 +99,6 @@ namespace dr1638 { // dr1638: 3.1
 
   enum class A<unsigned>::E;
   // since-cxx11-error@-1 {{template specialization requires 'template<>'}}
-  // since-cxx11-error@-2 {{forward declaration of enum class cannot have a nested name specifier}}
   template enum class A<unsigned>::E;
   // since-cxx11-error@-1 {{enumerations cannot be explicitly instantiated}}
   enum class A<unsigned>::E *e;
diff --git a/clang/test/CXX/module/module.interface/p2-2.cpp b/clang/test/CXX/module/module.interface/p2-2.cpp
index 04904a8d86f2792..8607ea47ac1ce30 100644
--- a/clang/test/CXX/module/module.interface/p2-2.cpp
+++ b/clang/test/CXX/module/module.interface/p2-2.cpp
@@ -15,6 +15,7 @@ struct X {
 };
 
 export template <typename T> struct X<T>::iterator;               // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+                                                                  // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
 export template <typename T> void X<T>::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template <typename T> template <typename U> U X<T>::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
@@ -28,10 +29,13 @@ export struct Y {
 };
 
 export struct Y::iterator;               // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+                                         // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
 export void Y::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template <typename U> U Y::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
 export {
   template <typename T> struct X<T>::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+                                               // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
   struct Y::iterator;                          // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+                                               // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
 }
diff --git a/clang/test/SemaCXX/enum-scoped.cpp b/clang/test/SemaCXX/enum-scoped.cpp
index 2bfe7203367503b..a4da0607d74ae5b 100644
--- a/clang/test/SemaCXX/enum-scoped.cpp
+++ b/clang/test/SemaCXX/enum-scoped.cpp
@@ -146,7 +146,9 @@ namespace test5 {
 namespace test6 {
   enum A : unsigned;
   struct A::a; // expected-error {{incomplete type 'test6::A' named in nested name specifier}}
+               // expected-error@-1{{forward declaration of struct cannot have a nested name specifier}}
   enum A::b; // expected-error {{incomplete type 'test6::A' named in nested name specifier}}
+             // expected-error@-1{{forward declaration of enum cannot have a nested name specifier}}
   int A::c; // expected-error {{incomplete type 'test6::A' named in nested name specifier}}
   void A::d(); // expected-error {{incomplete type 'test6::A' named in nested name specifier}}
   void test() {
diff --git a/clang/test/SemaCXX/nested-name-spec.cpp b/clang/test/SemaCXX/nested-name-spec.cpp
index 161c5e7fdf67af1..920ef42bc156468 100644
--- a/clang/test/SemaCXX/nested-name-spec.cpp
+++ b/clang/test/SemaCXX/nested-name-spec.cpp
@@ -65,7 +65,7 @@ A::C c1;
 struct A::C c2;
 struct S : public A::C {};
 struct A::undef; // expected-error {{no struct named 'undef' in namespace 'A'}}
-
+                 // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
 namespace A2 {
   typedef int INT;
   struct RC;
@@ -280,9 +280,11 @@ template<typename T>
 struct A {
 protected:
   struct B;
-  struct B::C; // expected-error {{requires a template parameter list}} \
-               // expected-error {{no struct named 'C'}} \
-    // expected-error{{non-friend class member 'C' cannot have a qualified name}}
+  struct B::C;
+  // expected-error@-1 {{requires a template parameter list}}
+  // expected-error@-2 {{no struct named 'C'}}
+  // expected-error@-3 {{non-friend class member 'C' cannot have a qualified name}}
+  // expected-error@-4 {{forward declaration of struct cannot have a nested name specifier}}
 };
 
 template<typename T>
@@ -292,6 +294,7 @@ struct A2 {
 };
 template <typename T>
 struct A2<T>::B::C; // expected-error {{no struct named 'C'}}
+                    // expected-error@-1 {{forward declaration of struct cannot have a nested name specifier}}
 }
 
 namespace PR13033 {
diff --git a/clang/test/SemaTemplate/elaborated-type-specifier.cpp b/clang/test/SemaTemplate/elaborated-type-specifier.cpp
index 514c5f2d57f4aa8..27b3f36ee14dd8e 100644
--- a/clang/test/SemaTemplate/elaborated-type-specifier.cpp
+++ b/clang/test/SemaTemplate/elaborated-type-specifier.cpp
@@ -23,6 +23,7 @@ namespace PR6915 {
 template<typename T>
 struct DeclOrDef {
   enum T::foo; // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
+               // expected-error@-1{{forward declaration of enum cannot have a nested name specifier}}
   enum T::bar { // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
     value 
   };
@@ -31,6 +32,7 @@ struct DeclOrDef {
 namespace PR6649 {
   template <typename T> struct foo { 
     class T::bar;  // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
+                   // expected-error@-1{{forward declaration of class cannot have a nested name specifier}}
     class T::bar { int x; }; // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
   };
 }
diff --git a/clang/test/SemaTemplate/qualified-id.cpp b/clang/test/SemaTemplate/qualified-id.cpp
index 64dff1ce235359a..883d3d330d14820 100644
--- a/clang/test/SemaTemplate/qualified-id.cpp
+++ b/clang/test/SemaTemplate/qualified-id.cpp
@@ -52,5 +52,6 @@ namespace PR12291 {
     template <typename V>
     template <typename W>
     class Outer2<V>::Inner; // expected-error{{nested name specifier 'Outer2<V>::' for declaration does not refer into a class, class template or class template partial specialization}}
+                            // expected-error@-1{{forward declaration of class cannot have a nested name specifier}}
   };
 }

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to DR tests look good.

@sdkrystian sdkrystian merged commit a0d266d into llvm:main Jan 30, 2024
4 of 5 checks passed
@sdkrystian sdkrystian deleted the elab-mem-temp-spec branch February 26, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants