Improve requirement clause limitation on non templated function

[erichkeane]

Summary

The current implementation 6da3d66f03f9162ef341cc67218be40e22fe9808
got a few things wrong, particularly that a template, or definition
or member in a templated entity is required to be allowed to have a
trailing requires clause.

This patch corrects this, as reproted by #61748

Fixes: #61748

[erichkeane]

clang/lib/Sema/SemaDecl.cpp

11904

Note we're using isTemplated here as a proxy for 'in a templated entity', since isTemplated is effectively `is a template OR in a templated entity'.

[cor3ntin]

LGTM except the arguably missing tests.

clang/lib/Sema/SemaDecl.cpp

11892–11893

I think we have no test for this case, might be worth adding.

[erichkeane]

clang/lib/Sema/SemaDecl.cpp

11892–11893

Interestingly, we permit that, but do NOT diagnose the lambda with a requires clause in a non-template function. Hrm....

[erichkeane]

Took Corentin's advice and tried to test the lambda condition. THIS made me discover that we didn't properly implement this for lambdas at all! So this patch NOW also implements the restriction for lambdas.

@cor3ntin : Please double check on this for me? I had to touch some lambda tests you worked on.

[erichkeane]

clang/test/Parser/cxx2b-lambdas.cpp

21

This still parses correctly, so I consider the error here to be 'correct' and still testing what we wanted.

clang/test/SemaCXX/lambda-capture-type-deduction.cpp

52

When this isn't a template, all of the requires clauses below are ill-formed. So made this a function template that hopefully @cor3ntin agrees matches his intent when he wrote this test.

[cor3ntin]

In D147070#4228303, @erichkeane wrote:

Took Corentin's advice and tried to test the lambda condition. THIS made me discover that we didn't properly implement this for lambdas at all! So this patch NOW also implements the restriction for lambdas.

@cor3ntin : Please double check on this for me? I had to touch some lambda tests you worked on.

Wow!
I think the patch still makes sense, I'm certainly glad we tested the lambda closure case!

clang/test/SemaCXX/lambda-capture-type-deduction.cpp

52

It does, thanks!
It's weird that i didn't notice the bug when writing those tests - at the same time i always felt this restriction to be a bit artificial.

[erichkeane]

Thanks for the double check on the review! I'm going to wait for the libcxx bot to come back (and for it to not be a commit at end of day!), but then will commit (unless there is new info).

clang/test/SemaCXX/lambda-capture-type-deduction.cpp

52

Oh, absolutely. Some of the limitations (on function defined inside of a function template that cannot be defined) are sensible in that they are otherwise unusable.

I suspect the reasoning is that else we have to start mangling requires clauses even on non templates, AND it makes extern "C" not work for functions with requires clauses.

[shafik]

LGTM

clang/lib/Sema/SemaDecl.cpp

11883

The cross reference was fixed: https://github.com/cplusplus/draft/pull/6215

[shafik]

clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp

27

Maybe a union case just for completeness:

template <typename T>
union U {
    void f() requires true;
};

clang/docs/ReleaseNotes.rst

 - Fix  ``getSourceRange`` on  ``VarTemplateSpecializationDecl`` and
   ``VarTemplatePartialSpecializationDecl``, which represents variable with
   the initializer, so it behaves consistently with other ``VarDecls`` and ends
   on the last token of initializer, instead of right angle bracket of
   the template argument list.
 - Fix false-positive diagnostic issued for consteval initializers of temporary
   objects.
   (`#60286 <https://github.com/llvm/llvm-project/issues/60286>`_)
-
+- Correct restriction of trailing requirements clauses on a templated function.
+  Previously we only rejected non-'templated' things, but the restrictions ALSO need
+  to limit non-defined/non-member functions as well. Additionally, we now diagnose
+  requires on lambdas when not allowed, which we previously missed.
+  (`#61748 <https://github.com/llvm/llvm-project/issues/61748>`_)
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support
 ^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaDecl.cpp

if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(NewFD)) {
 
       if (Method->isStatic())
         checkThisInStaticMemberFunctionType(Method);
     }
 
     // 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]).
     if (Expr *TRC = NewFD->getTrailingRequiresClause()) {
-      if (!NewFD->isTemplated() && !NewFD->isTemplateInstantiation())
+      // [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.
+
+      if (!NewFD->getDescribedFunctionTemplate() && // -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()) {
         Diag(TRC->getBeginLoc(), diag::err_constrained_non_templated_function);
       }
+    }
 
     if (CXXConversionDecl *Conversion = dyn_cast<CXXConversionDecl>(NewFD))
       ActOnConversionDeclarator(Conversion);
 
     // Extra checking for C++ overloaded operators (C++ [over.oper]).
     if (NewFD->isOverloadedOperator() &&
         CheckOverloadedOperatorDeclaration(NewFD)) {
       NewFD->setInvalidDecl();

clang/lib/Sema/SemaLambda.cpp

void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
   for (ParmVarDecl *P : Params) {
     if (!P->getIdentifier())
       continue;
     if (CheckRedefinition(P))
       CheckShadow(CurScope, P);
     PushOnScopeChains(P, CurScope);
   }
 
+  // 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]).
+  if (Expr *TRC = Method->getTrailingRequiresClause()) {
+    // [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.
+
+    // Note: we only have to check if this is defined in a template entity, OR
+    // if we are a template, since the rest don't apply. The requires clause
+    // applies to the call operator, which we already know is a member function,
+    // AND defined.
+    if (!Method->getDescribedFunctionTemplate() && !Method->isTemplated()) {
+      Diag(TRC->getBeginLoc(), diag::err_constrained_non_templated_function);
+    }
+  }
+
   // Enter a new evaluation context to insulate the lambda from any
   // cleanups from the enclosing full-expression.
   PushExpressionEvaluationContext(
       LSI->CallOperator->isConsteval()
           ? ExpressionEvaluationContext::ImmediateFunctionContext
           : ExpressionEvaluationContext::PotentiallyEvaluated);
 }
 

clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp

void (*pf)()
   requires true;
 
 // expected-error@+1{{trailing requires clause can only be used when declaring a function}}
 void g(int (*)() requires true);
 
 // expected-error@+1{{expected expression}}
 auto *p = new void(*)(char)
   requires true;
+
+namespace GH61748 {
+template<typename T>
+struct S {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  friend void declared_friend() requires(sizeof(T) > 1);
+  // OK, is a definition.
+  friend void defined_friend() requires(sizeof(T) > 1){}
+  // OK, is a member.
+  void member() requires(sizeof(T) > 1);
+};
+
+template<typename T>
+void ContainingFunction() {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  void bad() requires(sizeof(T) > 1);
+  // expected-error@+1 {{function definition is not allowed here}}
+  void still_bad() requires(sizeof(T) > 1) {}
+
+}
+
+void NonTemplContainingFunction() {
+  // expected-error@+1 {{non-templated function cannot have a requires clause}}
+  (void)[]() requires (sizeof(int)>1){};
+  // OK, a template.
+  auto X = [](auto) requires (sizeof(int)>1){};
+  // OK, a template.
+  auto Y = []<typename T>(T t) requires (sizeof(int)>1){};
+
+  X(1);
+  Y(1);
+}
+
+template<typename T>
+union U {
+  void f() requires true;
+};
+}

clang/test/Parser/cxx2b-lambdas.cpp

 auto L5 = [] constexpr mutable noexcept {};
 auto L6 = [s = 1] mutable {};
 auto L7 = [s = 1] constexpr mutable noexcept {};
 auto L8 = [] -> bool { return true; };
 auto L9 = []<typename T> { return true; };
 auto L10 = []<typename T> noexcept { return true; };
 auto L11 = []<typename T> -> bool { return true; };
 auto L12 = [] consteval {};
-auto L13 = []() requires true {};
+auto L13 = []() requires true {}; // expected-error{{non-templated function cannot have a requires clause}}
 auto L14 = []<auto> requires true() requires true {};
 auto L15 = []<auto> requires true noexcept {};
 auto L16 = [] [[maybe_unused]]{};
 
 auto XL0 = [] mutable constexpr mutable {};    // expected-error{{cannot appear multiple times}}
 auto XL1 = [] constexpr mutable constexpr {};  // expected-error{{cannot appear multiple times}}
 auto XL2 = []) constexpr mutable constexpr {}; // expected-error{{expected body of lambda expression}}
 auto XL3 = []( constexpr mutable constexpr {}; // expected-error{{invalid storage class specifier}} \

clang/test/SemaCXX/lambda-capture-type-deduction.cpp

void test_noexcept() {
   static_assert(noexcept([y] noexcept(is_same<const int &, decltype((y))>) {}()));
   static_assert(noexcept([y] mutable noexcept(is_same<int &, decltype((y))>) {}()));
   static_assert(noexcept([=] noexcept(is_same<const int &, decltype((y))>) {}()));
   static_assert(noexcept([=] mutable noexcept(is_same<int &, decltype((y))>) {}()));
   static_assert(noexcept([&] noexcept(is_same<int &, decltype((y))>) {}()));
   static_assert(noexcept([&] mutable noexcept(is_same<int &, decltype((y))>) {}()));
 }
 
+template<typename T>
 void test_requires() {
 
   int x;
 
   [x = 1]() requires is_same<const int &, decltype((x))> {}
   ();
   [x = 1]() mutable requires is_same<int &, decltype((x))> {}
   ();
   [x]() requires is_same<const int &, decltype((x))> {}
void test_requires() {
   ();
   [&x]() mutable requires is_same<int &, decltype((x))> {}
   ();
 
   [x = 1]() requires is_same<const int &, decltype((x))> {} ();
   [x = 1]() mutable requires is_same<int &, decltype((x))> {} ();
 }
 
+void use() {
+  test_requires<int>();
+}
+
 void err() {
   int y, z;
   (void)[x = 1]<typename T>
   requires(is_same<const int &, decltype((x))>) {};
 
   (void)[x = 1]<typename T = decltype((x))>{};
 
   (void)[=]<typename T = decltype((y))>{};