[llvm-branch-commits] [clang] [Clang] Fix -Wunused-parameter for implicit coroutine uses (backport #217518) (PR #218584)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-coroutines

Author: Lane0218

<details>
<summary>Changes</summary>

Backport of 01fec9890ad8a4980beb8ec1945320e126ddbdcd from `main`.

This fixes `-Wunused-parameter` handling for implicit coroutine parameter uses during allocation-function lookup and promise initialization.

The backport required a manual resolution in `clang/docs/ReleaseNotes.md`.

Validation:
- `clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp`: 1/1 passed
- `clang/test/SemaCXX/coroutine*.cpp`: 23/23 passed

/cherry-pick-of 217518

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


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+5) 
- (modified) clang/lib/Sema/SemaCoroutine.cpp (+30-3) 
- (modified) clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp (+44) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 59576a299c593..695beb91b3929 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -590,6 +590,11 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the
 
 ### Improvements to Clang's diagnostics
 
+- Fixed `-Wunused-parameter` to diagnose coroutine parameters that are only
+  considered during allocation function lookup or promise object
+  initialization, while not diagnosing parameters passed to the selected
+  allocation function or promise constructor. (#GH217501)
+
 - Fixed bug in `-Wdocumentation` so that it correctly handles explicit
   function template instantiations (#64087).
 
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 7f9b1d642cf9d..4b99e4454e57a 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -462,6 +462,12 @@ static ExprResult buildPromiseCall(Sema &S, VarDecl *Promise,
   return buildMemberCall(S, PromiseRef.get(), Loc, Name, Args);
 }
 
+static void markCoroutineParametersReferenced(FunctionDecl &FD) {
+  for (auto *PD : FD.parameters())
+    if (!PD->getType()->isDependentType())
+      PD->setReferenced();
+}
+
 VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
   assert(isa<FunctionDecl>(CurContext) && "not in a function scope");
   auto *FD = cast<FunctionDecl>(CurContext);
@@ -555,6 +561,10 @@ VarDecl *Sema::buildCoroutinePromise(SourceLocation Loc) {
         VD->setInit(MaybeCreateExprWithCleanups(Result.get()));
         VD->setInitStyle(VarDecl::CallInit);
         CheckCompleteVariableDeclaration(VD);
+        // The constructor is selected with the coroutine parameter copies as
+        // arguments. Mark the original parameters as referenced for
+        // -Wunused-parameter.
+        markCoroutineParametersReferenced(*FD);
       }
     } else
       ActOnUninitializedDecl(VD);
@@ -1385,9 +1395,14 @@ static bool collectPlacementArgs(Sema &S, FunctionDecl &FD, SourceLocation Loc,
 
     // Build a reference to the parameter.
     auto PDLoc = PD->getLocation();
+    // Preserve the referenced state for unused parameter diagnostics.
+    bool DeclReferenced = PD->isReferenced();
     ExprResult PDRefExpr =
         S.BuildDeclRefExpr(PD, PD->getOriginalType().getNonReferenceType(),
                            ExprValueKind::VK_LValue, PDLoc);
+
+    PD->setReferenced(DeclReferenced);
+
     if (PDRefExpr.isInvalid())
       return false;
 
@@ -1443,6 +1458,8 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
 
   FunctionDecl *OperatorNew = nullptr;
   SmallVector<Expr *, 1> PlacementArgs;
+  // Track whether PlacementArgs still refer to the coroutine parameters.
+  bool PlacementArgsFromCoroutine = false;
   DeclarationName NewName =
       S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
 
@@ -1491,8 +1508,11 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
   // We don't expect to call to global operator new with (size, p0, …, pn).
   // So if we choose to lookup the allocation function in global scope, we
   // shouldn't lookup placement arguments.
-  if (PromiseContainsNew && !collectPlacementArgs(S, FD, Loc, PlacementArgs))
-    return false;
+  if (PromiseContainsNew) {
+    if (!collectPlacementArgs(S, FD, Loc, PlacementArgs))
+      return false;
+    PlacementArgsFromCoroutine = true;
+  }
 
   LookupAllocationFunction();
 
@@ -1558,6 +1578,7 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
     if (!StdNoThrow)
       return false;
     PlacementArgs = {StdNoThrow};
+    PlacementArgsFromCoroutine = false;
     OperatorNew = nullptr;
     LookupAllocationFunction(AllocationFunctionScope::Global);
   }
@@ -1644,8 +1665,14 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
       isAlignedAllocation(IAP.PassAlignment))
     NewArgs.push_back(FrameAlignment);
 
-  if (OperatorNew->getNumParams() > NewArgs.size())
+  // getNumParams() does not include an ellipsis, but a variadic allocation
+  // function still receives the coroutine parameters as placement arguments.
+  if (OperatorNew->isVariadic() ||
+      OperatorNew->getNumParams() > NewArgs.size()) {
     llvm::append_range(NewArgs, PlacementArgs);
+    if (PlacementArgsFromCoroutine)
+      markCoroutineParametersReferenced(FD);
+  }
 
   ExprResult NewExpr =
       S.BuildCallExpr(S.getCurScope(), NewRef.get(), Loc, NewArgs, Loc);
diff --git a/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp b/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
index fee379d869112..ef0c767dbc802 100644
--- a/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
+++ b/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
@@ -8,8 +8,12 @@ struct awaitable {
   void await_suspend(std::coroutine_handle<>) noexcept;
 };
 
+struct promise_arg {};
+
 struct task : awaitable {
   struct promise_type {
+    promise_type();
+    promise_type(promise_arg);
     task get_return_object() noexcept;
     awaitable initial_suspend() noexcept;
     awaitable final_suspend() noexcept;
@@ -18,10 +22,50 @@ struct task : awaitable {
   };
 };
 
+struct allocation_arg {};
+
+struct task_with_new {
+  struct promise_type {
+    void *operator new(decltype(sizeof(0)));
+    void *operator new(decltype(sizeof(0)), allocation_arg);
+    task_with_new get_return_object();
+    awaitable initial_suspend();
+    awaitable final_suspend() noexcept;
+    void unhandled_exception();
+    void return_void();
+  };
+};
+
+struct task_with_variadic_new {
+  struct promise_type {
+    void *operator new(decltype(sizeof(0)), ...);
+    task_with_variadic_new get_return_object();
+    awaitable initial_suspend();
+    awaitable final_suspend() noexcept;
+    void unhandled_exception();
+    void return_void();
+  };
+};
+
 task foo(int a) { // expected-warning{{unused parameter 'a'}}
   co_return;
 }
 
+task promise_constructor_uses_parameter(promise_arg a) { co_return; }
+
+task_with_new class_specific_new_fallback(
+    int a) { // expected-warning{{unused parameter 'a'}}
+  co_return;
+}
+
+task_with_new placement_allocation_uses_parameter(allocation_arg a) {
+  co_return;
+}
+
+task_with_variadic_new variadic_allocation_uses_parameter(int a) {
+  co_return;
+}
+
 task bar(int a, int b) { // expected-warning{{unused parameter 'b'}}
   a = a + 1;
   co_return;

``````````

</details>


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