[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)

[Yuxuan Chen via cfe-commits]

https://github.com/yuxuanchen1997 created https://github.com/llvm/llvm-project/pull/70567

This PR is proposing a fix for https://github.com/llvm/llvm-project/issues/65971.

Previously, given a coroutine like this
```
task foo(int a) {
  co_return;
}
```
Parameter `a` is never used. However, because C++ coroutines move constructs the variable to a heap allocated coroutine activation frame, we considered all parameters referenced. When diagnosing unused parameters, we cannot distinguish if the variable reference was due to coroutine parameter moves. 


Compiler Explorer shows that GCC warns against this case correctly, but clang does not: https://godbolt.org/z/Wo7dfqeaf

This change adds a flag `LastReferenceInCoroutineParamMoves` to `Decl`s. All other references to the `Decl` will clear the flag, so we are still able to tell if the reference is from the user defined coroutine body. 

>From ba6c43cc63e9ae3c8d174406ced4df839af3e0a7 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Fri, 27 Oct 2023 16:37:40 -0700
Subject: [PATCH] [Clang] Coroutines: warn against unused parameters in C++
 coroutines with -Wunused-parameters

---
 clang/include/clang/AST/DeclBase.h            | 18 +++++++++++-
 clang/lib/Sema/SemaCoroutine.cpp              |  6 ++++
 clang/lib/Sema/SemaDecl.cpp                   |  5 +++-
 .../warn-unused-parameters-coroutine.cpp      | 28 +++++++++++++++++++
 4 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp

diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 978e4255e877ec2..dc78ee37d16bea2 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -306,6 +306,11 @@ class alignas(8) Decl {
   /// are regarded as "referenced" but not "used".
   unsigned Referenced : 1;
 
+  /// Whether the last reference to this declaration happened in Coroutine
+  /// Parameter moves. Otherwise the reference caused by such moves would
+  /// prevent a warning against unused parameters in all coroutines.
+  unsigned LastReferenceInCoroutineParamMoves : 1;
+
   /// Whether this declaration is a top-level declaration (function,
   /// global variable, etc.) that is lexically inside an objc container
   /// definition.
@@ -609,11 +614,22 @@ class alignas(8) Decl {
   /// Whether any declaration of this entity was referenced.
   bool isReferenced() const;
 
+  bool isLastReferenceInCoroutineParamMoves() const {
+    return LastReferenceInCoroutineParamMoves;
+  }
+
+  void setLastReferenceInCoroutineParamMoves(bool V = true) {
+    LastReferenceInCoroutineParamMoves = true;
+  }
+
   /// Whether this declaration was referenced. This should not be relied
   /// upon for anything other than debugging.
   bool isThisDeclarationReferenced() const { return Referenced; }
 
-  void setReferenced(bool R = true) { Referenced = R; }
+  void setReferenced(bool R = true) {
+    Referenced = R;
+    LastReferenceInCoroutineParamMoves = false;
+  }
 
   /// Whether this declaration is a top-level declaration (function,
   /// global variable, etc.) that is lexically inside an objc container
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index cfaa93fbea4dd37..2936c39941e2922 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1964,9 +1964,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation Loc) {
     if (PD->getType()->isDependentType())
       continue;
 
+    bool PDRefBefore = PD->isReferenced();
+
     ExprResult PDRefExpr =
         BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
                          ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+    if (!PDRefBefore)
+      PD->setLastReferenceInCoroutineParamMoves();
+
     if (PDRefExpr.isInvalid())
       return false;
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c4979b51e68f5e2..06487d3595de557 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15089,7 +15089,10 @@ void Sema::DiagnoseUnusedParameters(ArrayRef<ParmVarDecl *> Parameters) {
     return;
 
   for (const ParmVarDecl *Parameter : Parameters) {
-    if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+    if (Parameter->isReferenced() && !Parameter->isLastReferenceInCoroutineParamMoves())
+      continue;
+
+    if (Parameter->getDeclName() &&
         !Parameter->hasAttr<UnusedAttr>() &&
         !Parameter->getIdentifier()->isPlaceholder()) {
       Diag(Parameter->getLocation(), diag::warn_unused_parameter)
diff --git a/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp b/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
new file mode 100644
index 000000000000000..0fd26ea4a21be79
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-parameter -verify -std=c++20 %s
+
+#include "Inputs/std-coroutine.h"
+
+struct awaitable {
+  bool await_ready() noexcept;
+  void await_resume() noexcept;
+  void await_suspend(std::coroutine_handle<>) noexcept;
+};
+
+struct task : awaitable {
+  struct promise_type {
+    task get_return_object() noexcept;
+    awaitable initial_suspend() noexcept;
+    awaitable final_suspend() noexcept;
+    void unhandled_exception() noexcept;
+    void return_void() noexcept;
+  };
+};
+
+task foo(int a) { // expected-warning{{unused parameter 'a'}}
+  co_return;
+}
+
+task bar(int a, int b) { // expected-warning{{unused parameter 'b'}}
+  a = a + 1;
+  co_return;
+}