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

[Yuxuan Chen via Openmp-commits]

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

>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 1/3] [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;
+}

>From fe7109f18286b8ba7e3e44f1fb31fc60e1ad8422 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/3] fix small bug: setLastReferenceInCoroutineParamMoves
 should respect param

---
 clang/include/clang/AST/DeclBase.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index dc78ee37d16bea2..ff519d14602014b 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -619,7 +619,7 @@ class alignas(8) Decl {
   }
 
   void setLastReferenceInCoroutineParamMoves(bool V = true) {
-    LastReferenceInCoroutineParamMoves = true;
+    LastReferenceInCoroutineParamMoves = V;
   }
 
   /// Whether this declaration was referenced. This should not be relied

>From e0282f44220008d07dd86fe195ba8e1d84ecc8c7 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Sat, 28 Oct 2023 12:46:30 -0700
Subject: [PATCH 3/3] fix clang format

---
 clang/lib/Sema/SemaDecl.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 06487d3595de557..948fdd18d720e44 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15089,11 +15089,11 @@ void Sema::DiagnoseUnusedParameters(ArrayRef<ParmVarDecl *> Parameters) {
     return;
 
   for (const ParmVarDecl *Parameter : Parameters) {
-    if (Parameter->isReferenced() && !Parameter->isLastReferenceInCoroutineParamMoves())
+    if (Parameter->isReferenced() &&
+        !Parameter->isLastReferenceInCoroutineParamMoves())
       continue;
 
-    if (Parameter->getDeclName() &&
-        !Parameter->hasAttr<UnusedAttr>() &&
+    if (Parameter->getDeclName() && !Parameter->hasAttr<UnusedAttr>() &&
         !Parameter->getIdentifier()->isPlaceholder()) {
       Diag(Parameter->getLocation(), diag::warn_unused_parameter)
         << Parameter->getDeclName();