[Clang] SemaFunctionEffects: Fix bug where lambdas produced by template expansion weren't verified.

[dougsonos]

Lambdas are added to the list of Decls to verify using Sema::maybeAddDeclWithEffects(). Up until now this call was in ActOnLambdaExpr, which happens in the context of a template but not in the context of template expansion.

Template expansion calls BuildLambdaExpr, so move the call to Sema::maybeAddDeclWithEffects() there.

Fixing this created a not-previously-seen situation where the custom "In template expansion here" diagnostic didn't have a valid location. This breaks tests because there's a note with no location. For now I've just skipped emitting the diagnostic but maybe there's a better solution.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Doug Wyatt (dougsonos)

Changes

Lambdas are added to the list of Decls to verify using Sema::maybeAddDeclWithEffects(). Up until now this call was in ActOnLambdaExpr, which happens in the context of a template but not in the context of template expansion.

Template expansion calls BuildLambdaExpr, so move the call to Sema::maybeAddDeclWithEffects() there.

Fixing this created a not-previously-seen situation where the custom "In template expansion here" diagnostic didn't have a valid location. This breaks tests because there's a note with no location. For now I've just skipped emitting the diagnostic but maybe there's a better solution.

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaFunctionEffects.cpp (+2-1)
• (modified) clang/lib/Sema/SemaLambda.cpp (+1-2)
• (modified) clang/test/Sema/attr-nonblocking-constraints.cpp (+17)

diff --git a/clang/lib/Sema/SemaFunctionEffects.cpp b/clang/lib/Sema/SemaFunctionEffects.cpp
index 4b5ddb74b1262f..6fe4d2353a2282 100644
--- a/clang/lib/Sema/SemaFunctionEffects.cpp
+++ b/clang/lib/Sema/SemaFunctionEffects.cpp
@@ -807,7 +807,8 @@ class Analyzer {
 
     auto MaybeAddTemplateNote = [&](const Decl *D) {
       if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
-        while (FD != nullptr && FD->isTemplateInstantiation()) {
+        while (FD != nullptr && FD->isTemplateInstantiation() &&
+               FD->getPointOfInstantiation().isValid()) {
           S.Diag(FD->getPointOfInstantiation(),
                  diag::note_func_effect_from_template);
           FD = FD->getTemplateInstantiationPattern();
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index e7afa0f4c81fc4..a67c0b2b367d1a 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1950,8 +1950,6 @@ ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body) {
   LambdaScopeInfo LSI = *cast<LambdaScopeInfo>(FunctionScopes.back());
   ActOnFinishFunctionBody(LSI.CallOperator, Body);
 
-  maybeAddDeclWithEffects(LSI.CallOperator);
-
   return BuildLambdaExpr(StartLoc, Body->getEndLoc(), &LSI);
 }
 
@@ -2284,6 +2282,7 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
     case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
       break;
     }
+    maybeAddDeclWithEffects(LSI->CallOperator);
   }
 
   return MaybeBindToTemporary(Lambda);
diff --git a/clang/test/Sema/attr-nonblocking-constraints.cpp b/clang/test/Sema/attr-nonblocking-constraints.cpp
index cc9108c0a4fbd6..87cbcc9713859f 100644
--- a/clang/test/Sema/attr-nonblocking-constraints.cpp
+++ b/clang/test/Sema/attr-nonblocking-constraints.cpp
@@ -144,6 +144,23 @@ void nb9() [[clang::nonblocking]]
 		expected-note {{in template expansion here}}
 }
 
+// Make sure we verify lambdas produced from template expansions.
+struct HasTemplatedLambda {
+	void (*fptr)() [[clang::nonblocking]];
+
+	template <typename C>
+	HasTemplatedLambda(const C&)
+		: fptr{ []() [[clang::nonblocking]] {
+			auto* y = new int; // expected-warning {{lambda with 'nonblocking' attribute must not allocate or deallocate memory}}
+		} }
+	{}
+};
+
+void nb9a()
+{
+	HasTemplatedLambda bad(42);
+}
+
 void nb10(
 	void (*fp1)(), // expected-note {{function pointer cannot be inferred 'nonblocking'}}
 	void (*fp2)() [[clang::nonblocking]]

[Sirraide]

Ah yeah, I should have noticed that in review: ActOnX is generally only called during parsing, whereas BuildX is also called during template instantiation.

As for the template notes, I don’t have a good solution for those either...

Also, we do have tests for function templates and generic lambdas, right? If not, we should definitely add some.

[dougsonos]

Thanks for the quick look.

Ah yeah, I should have noticed that in review: ActOnX is generally only called during parsing, whereas BuildX is also called during template instantiation.

I actually do think it might have come up early in one of the giant reviews, but gotten lost amongst the bigger issues.

Also, we do have tests for function templates and generic lambdas, right? If not, we should definitely add some.

Something like:

template <typename T>
void TemplatedFunc(T x) [[clang::nonblocking]] {
	auto* ptr = new T; // expected-warning {{function with 'nonblocking' attribute must not allocate or deallocate memory}}
}

template <typename T>
auto TemplatedLambda = [](T x) [[clang::nonblocking]] {
	auto* ptr = new T; // expected-warning {{lambda with 'nonblocking' attribute must not allocate or deallocate memory}}
};

void nb9b() [[clang::nonblocking]] {
	TemplatedFunc(42); // expected-note {{in template expansion here}}
	TemplatedLambda<int>(42);
}

?

[Sirraide]

Something like:

The function template is pretty much what I meant, yeah, but by ‘templated lambda’ I meant either one that is an implicit template because one of its parameters is declared with auto ([](auto x)) or one that has an explicit template parameter list ([]<typename T>).

[cor3ntin]

LGTM