[Clang][Sema] Implement proposed resolution for CWG2847

[sdkrystian]

Per the approved resolution for CWG2847, [temp.expl.spec] p8 will state:

An explicit specialization shall not have a trailing requires-clause unless it declares a function template.

We already implement this partially insofar that a diagnostic is issued upon instantiation of A<int> in the following example:

template<typename>
struct A
{
    template<typename>
    void f();

    template<>
    void f<int>() requires true; // error: non-templated function cannot have a requires clause
};
template struct A<int>;  // note: in instantiation of template class 'A<int>' requested here

This patch adds a bespoke diagnostic for such declarations, and moves the point of diagnosis for non-templated functions with trailing requires-clauses from CheckFunctionDeclaration to ActOnFunctionDeclarator (there is no point in diagnosing this during instantiation since we already have all the necessary information when parsing the declaration).

[llvmbot]

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Per the approved resolution for CWG2847, [temp.expl.spec] p8 will state:
> An explicit specialization shall not have a trailing requires-clause unless it declares a function template.

We already implement this partially insofar that a diagnostic is issued upon instantiation of A<int> in the following example:

template<typename>
struct A
{
    template<typename>
    void f();

    template<>
    void f<int>() requires true; // error: non-templated function cannot have a requires clause
};
template struct A<int>;  // note: in instantiation of template class 'A<int>' requested here

This patch adds a bespoke diagnostic for such declarations, and moves the point of diagnosis for non-templated functions with trailing requires-clauses from CheckFunctionDeclaration to ActOnFunctionDeclarator (there is no point in diagnosing this during instantiation since we already have all the necessary information when parsing the declaration).

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
• (modified) clang/lib/Sema/SemaDecl.cpp (+54-49)
• (added) clang/test/CXX/drs/dr28xx.cpp (+41)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f76e7a3392183..b4dc4feee8e63 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2982,6 +2982,9 @@ def err_trailing_requires_clause_on_deduction_guide : Error<
   "deduction guide cannot have a requires clause">;
 def err_constrained_non_templated_function
     : Error<"non-templated function cannot have a requires clause">;
+def err_non_temp_spec_requires_clause : Error<
+  "%select{explicit|friend}0 specialization cannot have a trailing requires clause "
+  "unless it declares a function template">;
 def err_reference_to_function_with_unsatisfied_constraints : Error<
   "invalid reference to function %0: constraints not satisfied">;
 def err_requires_expr_local_parameter_default_argument : Error<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 481d952d2389b..18a5d93ab8e8c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10440,6 +10440,60 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
            diag::ext_operator_new_delete_declared_inline)
         << NewFD->getDeclName();
 
+    if (Expr *TRC = NewFD->getTrailingRequiresClause()) {
+      // C++20 [dcl.decl.general]p4:
+      //   The optional requires-clause in an init-declarator or
+      //   member-declarator shall be present only if the declarator declares a
+      //   templated function.
+      //
+      // C++20 [temp.pre]p8:
+      //   An entity is templated if it is
+      //     - a template,
+      //     - an entity defined or created in a templated entity,
+      //     - a member of a templated entity,
+      //     - an enumerator for an enumeration that is a templated entity, or
+      //     - the closure type of a lambda-expression appearing in the
+      //       declaration of a templated entity.
+      //
+      //   [Note 6: A local class, a local or block variable, or a friend
+      //   function defined in a templated entity is a templated entity.
+      //   — end note]
+      //
+      //   A templated function is a function template or a function that is
+      //   templated. A templated class is a class template or a class that is
+      //   templated. A templated variable is a variable template or a variable
+      //   that is templated.
+      if (!FunctionTemplate) {
+        if (isFunctionTemplateSpecialization || isMemberSpecialization) {
+          // C++ [temp.expl.spec]p8 (proposed resolution for CWG2847):
+          //   An explicit specialization shall not have a trailing
+          //   requires-clause unless it declares a function template.
+          //
+          // Since a friend function template specialization cannot be
+          // definition, and since a non-template friend declaration with a
+          // trailing requires-clause must be a definition, we diagnose
+          // friend function template specializations with trailing
+          // requires-clauses on the same path as explicit specializations
+          // even though they aren't necessarily prohibited by the same
+          // language rule.
+          Diag(TRC->getBeginLoc(), diag::err_non_temp_spec_requires_clause)
+              << isFriend;
+        } else if (isFriend && NewFD->isTemplated() &&
+                   !D.isFunctionDefinition()) {
+          // C++ [temp.friend]p9:
+          //   A non-template friend declaration with a requires-clause shall be
+          //   a definition.
+          Diag(NewFD->getBeginLoc(),
+               diag::err_non_temp_friend_decl_with_requires_clause_must_be_def);
+          NewFD->setInvalidDecl();
+        } else if (!NewFD->isTemplated() ||
+                   !(isa<CXXMethodDecl>(NewFD) || D.isFunctionDefinition())) {
+          Diag(TRC->getBeginLoc(),
+               diag::err_constrained_non_templated_function);
+        }
+      }
+    }
+
     // We do not add HD attributes to specializations here because
     // they may have different constexpr-ness compared to their
     // templates and, after maybeAddCUDAHostDeviceAttrs() is applied,
@@ -12063,55 +12117,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
         checkThisInStaticMemberFunctionType(Method);
     }
 
-    if (Expr *TRC = NewFD->getTrailingRequiresClause()) {
-      // C++20: dcl.decl.general p4:
-      // The optional requires-clause ([temp.pre]) in an init-declarator or
-      // member-declarator shall be present only if the declarator declares a
-      // templated function ([dcl.fct]).
-      //
-      // [temp.pre]/8:
-      // An entity is templated if it is
-      // - a template,
-      // - an entity defined ([basic.def]) or created ([class.temporary]) in a
-      // templated entity,
-      // - a member of a templated entity,
-      // - an enumerator for an enumeration that is a templated entity, or
-      // - the closure type of a lambda-expression ([expr.prim.lambda.closure])
-      // appearing in the declaration of a templated entity. [Note 6: A local
-      // class, a local or block variable, or a friend function defined in a
-      // templated entity is a templated entity.  — end note]
-      //
-      // A templated function is a function template or a function that is
-      // templated. A templated class is a class template or a class that is
-      // templated. A templated variable is a variable template or a variable
-      // that is templated.
-
-      bool IsTemplate = NewFD->getDescribedFunctionTemplate();
-      bool IsFriend = NewFD->getFriendObjectKind();
-      if (!IsTemplate && // -a template
-                         // defined... in a templated entity
-          !(DeclIsDefn && NewFD->isTemplated()) &&
-          // a member of a templated entity
-          !(isa<CXXMethodDecl>(NewFD) && NewFD->isTemplated()) &&
-          // Don't complain about instantiations, they've already had these
-          // rules + others enforced.
-          !NewFD->isTemplateInstantiation() &&
-          // If the function violates [temp.friend]p9 because it is missing
-          // a definition, and adding a definition would make it templated,
-          // then let that error take precedence.
-          !(!DeclIsDefn && IsFriend && NewFD->isTemplated())) {
-        Diag(TRC->getBeginLoc(), diag::err_constrained_non_templated_function);
-      } else if (!DeclIsDefn && !IsTemplate && IsFriend &&
-                 !NewFD->isTemplateInstantiation()) {
-        // C++ [temp.friend]p9:
-        //   A non-template friend declaration with a requires-clause shall be a
-        //   definition.
-        Diag(NewFD->getBeginLoc(),
-             diag::err_non_temp_friend_decl_with_requires_clause_must_be_def);
-        NewFD->setInvalidDecl();
-      }
-    }
-
     if (CXXConversionDecl *Conversion = dyn_cast<CXXConversionDecl>(NewFD))
       ActOnConversionDeclarator(Conversion);
 
diff --git a/clang/test/CXX/drs/dr28xx.cpp b/clang/test/CXX/drs/dr28xx.cpp
new file mode 100644
index 0000000000000..f8e804baa7385
--- /dev/null
+++ b/clang/test/CXX/drs/dr28xx.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++20 -verify=expected %s
+
+namespace dr2847 { // dr2847: 19
+
+template<typename>
+void i();
+
+struct A {
+  template<typename>
+  void f() requires true;
+
+  template<>
+  void f<int>() requires true; // expected-error {{explicit specialization cannot have a trailing requires clause unless it declares a function template}}
+
+  friend void i<int>() requires true; // expected-error {{friend specialization cannot have a trailing requires clause unless it declares a function template}}
+};
+
+template<typename>
+struct B {
+  void f() requires true;
+
+  template<typename>
+  void g() requires true;
+
+  template<typename>
+  void h() requires true;
+
+  template<>
+  void h<int>() requires true; // expected-error {{explicit specialization cannot have a trailing requires clause unless it declares a function template}}
+
+  friend void i<int>() requires true; // expected-error {{friend specialization cannot have a trailing requires clause unless it declares a function template}}
+};
+
+template<>
+void B<int>::f() requires true; // expected-error {{explicit specialization cannot have a trailing requires clause unless it declares a function template}}
+
+template<>
+template<typename T>
+void B<int>::g() requires true;
+
+} // namespace dr2847

[Endilll]

Since you're creating a new file in C++ DR test suite, I left several comments to guide you towards bringing this new file up to speed with the rest of the suite.

[sdkrystian]

@Endilll I applied your suggested changes... how does it look?

[Endilll]

Much better, thank you. Couple of small changes, and DR test should be good to go.

[sdkrystian]

Whoops... changed it to 202002L :)

[Endilll]

DR testing looks good now. Thank you for bearing with me!
I tried to give this a proper in-depth review, but the only corner case that comes to my mind is conversion functions. Feel free to dismiss it if you don't think it's relevant or interesting.