[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
Yuxuan Chen via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 28 12:46:57 PDT 2023
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();
More information about the cfe-commits
mailing list