[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/72346
This change aims to fix https://github.com/llvm/llvm-project/issues/70375
It appears to me that the logic here should be handling specializations in
general. Not just partial specialization. It also seems that both the comment
before the block and the `isInstantiationOf(ClassTemplate, SpecTemplate)` below
agree with my judgement.
The issue might just be a mistake that someone mistaken specialization as a
special case of partial specializations, while it's actually the other way
around.
Needs some experts to comment here if this is the right fix.
The code that caused clang ICE is added as a test case.
>From f238608b792f69b93eb445ee596125f3e20acf39 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:52:21 -0800
Subject: [PATCH 1/4] [Clang] Fix ICE caused by mishandling template
specialization in instantiation lookup
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 011356e08a04297..2ef0986dd4d2235 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,9 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplatePartialSpecializationDecl *PartialSpec
- = dyn_cast(Record))
- ClassTemplate =
PartialSpec->getSpecializedTemplate()->getCanonicalDecl();
+else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+ ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
// it.
>From 9a79db980951299c93cc4530230717af6f4668b3 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:55:10 -0800
Subject: [PATCH 2/4] formatting
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2ef0986dd4d2235..07b3488a21e670b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,7 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+else if (ClassTemplateSpecializationDecl *Spec =
+ dyn_cast(Record))
ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
>From df884c7127bb2f0956521adc479a923c62220d7c Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:04:00 -0800
Subject: [PATCH 3/4] Provide test case
---
.../member-template-specialization.cpp| 31 +++
1 file changed, 31 insertions(+)
create mode 100644 clang/test/SemaCXX/member-template-specialization.cpp
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
new file mode 100644
index 000..baf5bd6ae7bb544
--- /dev/null
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// Verify that the inner template specialization can be found
+
+template
+struct S {
+ static void bar() {
+Ty t;
+t.foo();
+ }
+
+ static void take(Ty&) {}
+};
+
+template
+struct Outer {
+ template
+ struct Inner;
+
+ using U = S>;
+
+ template <>
+ struct Inner {
+void foo() {
+ U::take(*this);
+}
+ };
+};
+
+int main() {
+ Outer::U::bar();
+}
>From 7cdbce7944e051aed5b1f8ab789ce1e43f3e58b3 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:13:23 -0800
Subject: [PATCH 4/4] missing expected-no-diagnostics
---
clang/test/SemaCXX/member-template-specialization.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
index baf5bd6ae7bb544..29d46ec9c1e44fc 100644
--- a/clang/test/SemaCXX/member-template-specialization.cpp
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+// expected-no-diagnostics
+
// Verify that the inner template specialization can be found
template
___
cfe-commits mailing list
cfe
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72346
>From f238608b792f69b93eb445ee596125f3e20acf39 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:52:21 -0800
Subject: [PATCH 1/5] [Clang] Fix ICE caused by mishandling template
specialization in instantiation lookup
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 011356e08a04297..2ef0986dd4d2235 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,9 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplatePartialSpecializationDecl *PartialSpec
- = dyn_cast(Record))
- ClassTemplate =
PartialSpec->getSpecializedTemplate()->getCanonicalDecl();
+else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+ ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
// it.
>From 9a79db980951299c93cc4530230717af6f4668b3 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:55:10 -0800
Subject: [PATCH 2/5] formatting
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2ef0986dd4d2235..07b3488a21e670b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,7 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+else if (ClassTemplateSpecializationDecl *Spec =
+ dyn_cast(Record))
ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
>From df884c7127bb2f0956521adc479a923c62220d7c Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:04:00 -0800
Subject: [PATCH 3/5] Provide test case
---
.../member-template-specialization.cpp| 31 +++
1 file changed, 31 insertions(+)
create mode 100644 clang/test/SemaCXX/member-template-specialization.cpp
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
new file mode 100644
index 000..baf5bd6ae7bb544
--- /dev/null
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// Verify that the inner template specialization can be found
+
+template
+struct S {
+ static void bar() {
+Ty t;
+t.foo();
+ }
+
+ static void take(Ty&) {}
+};
+
+template
+struct Outer {
+ template
+ struct Inner;
+
+ using U = S>;
+
+ template <>
+ struct Inner {
+void foo() {
+ U::take(*this);
+}
+ };
+};
+
+int main() {
+ Outer::U::bar();
+}
>From 7cdbce7944e051aed5b1f8ab789ce1e43f3e58b3 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:13:23 -0800
Subject: [PATCH 4/5] missing expected-no-diagnostics
---
clang/test/SemaCXX/member-template-specialization.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
index baf5bd6ae7bb544..29d46ec9c1e44fc 100644
--- a/clang/test/SemaCXX/member-template-specialization.cpp
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+// expected-no-diagnostics
+
// Verify that the inner template specialization can be found
template
>From 23e4a9457c9d4428245a9d6593b97a33ed538ce7 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 15 Nov 2023 10:10:02 -0800
Subject: [PATCH 5/5] change stale comment
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 07b3488a21e670b..08f4ba00fc9f7de 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6207,7 +6207,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
return D;
// Determine whether this record is t
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
yuxuanchen1997 wrote: @AaronBallman , mind providing some feedback on this patch? I think this can solve #70375 https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
@@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -verify -fsyntax-only %s yuxuanchen1997 wrote: Can it be more elaborate like `GH70735-member-template-specialization.cpp`? https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
@@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -verify -fsyntax-only %s yuxuanchen1997 wrote: gotcha, will do. https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
yuxuanchen1997 wrote: Thanks for the review, @erichkeane. I am wondering what you mean by this needs "release note"? https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72346
>From 739f1802bbfa4a7dc454e24535423b64701ac500 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:52:21 -0800
Subject: [PATCH 1/8] [Clang] Fix ICE caused by mishandling template
specialization in instantiation lookup
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 011356e08a04297..2ef0986dd4d2235 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,9 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplatePartialSpecializationDecl *PartialSpec
- = dyn_cast(Record))
- ClassTemplate =
PartialSpec->getSpecializedTemplate()->getCanonicalDecl();
+else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+ ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
// it.
>From 9f823a95f56dd743421886f348bb10451e509cf1 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 20:55:10 -0800
Subject: [PATCH 2/8] formatting
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2ef0986dd4d2235..07b3488a21e670b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6211,7 +6211,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
ClassTemplateDecl *ClassTemplate = Record->getDescribedClassTemplate();
if (ClassTemplate)
ClassTemplate = ClassTemplate->getCanonicalDecl();
-else if (ClassTemplateSpecializationDecl *Spec =
dyn_cast(Record))
+else if (ClassTemplateSpecializationDecl *Spec =
+ dyn_cast(Record))
ClassTemplate = Spec->getSpecializedTemplate()->getCanonicalDecl();
// Walk the current context to find either the record or an instantiation
of
>From 0b0c94bbce2e02d5ddebc961d991274bae03f8ef Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:04:00 -0800
Subject: [PATCH 3/8] Provide test case
---
.../member-template-specialization.cpp| 31 +++
1 file changed, 31 insertions(+)
create mode 100644 clang/test/SemaCXX/member-template-specialization.cpp
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
new file mode 100644
index 000..baf5bd6ae7bb544
--- /dev/null
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// Verify that the inner template specialization can be found
+
+template
+struct S {
+ static void bar() {
+Ty t;
+t.foo();
+ }
+
+ static void take(Ty&) {}
+};
+
+template
+struct Outer {
+ template
+ struct Inner;
+
+ using U = S>;
+
+ template <>
+ struct Inner {
+void foo() {
+ U::take(*this);
+}
+ };
+};
+
+int main() {
+ Outer::U::bar();
+}
>From 3cddae9a22c67e43456c5ff4e682b208c0b727b9 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 14 Nov 2023 21:13:23 -0800
Subject: [PATCH 4/8] missing expected-no-diagnostics
---
clang/test/SemaCXX/member-template-specialization.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/test/SemaCXX/member-template-specialization.cpp
b/clang/test/SemaCXX/member-template-specialization.cpp
index baf5bd6ae7bb544..29d46ec9c1e44fc 100644
--- a/clang/test/SemaCXX/member-template-specialization.cpp
+++ b/clang/test/SemaCXX/member-template-specialization.cpp
@@ -1,4 +1,6 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+// expected-no-diagnostics
+
// Verify that the inner template specialization can be found
template
>From 52820b186c733ef52c464094395c0b1f3f7c3a00 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 15 Nov 2023 10:10:02 -0800
Subject: [PATCH 5/8] change stale comment
---
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 07b3488a21e670b..08f4ba00fc9f7de 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6207,7 +6207,7 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc,
NamedDecl *D,
return D;
// Determine whether this record is t
[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)
yuxuanchen1997 wrote: I have made the suggested changes and this is ready for another round of review. https://github.com/llvm/llvm-project/pull/72346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/72935
None
>From 13b54167ea0dfe4f39de7b29c3e61489aa683caa Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 16:06:44 -0800
Subject: [PATCH 1/2] Attempt to fix init sus ret destroy
---
clang/lib/CodeGen/CGCoroutine.cpp | 5 -
clang/lib/CodeGen/CGException.cpp | 1 +
clang/lib/CodeGen/CodeGenFunction.cpp | 8
clang/lib/CodeGen/CodeGenFunction.h | 2 ++
4 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..78df459113090e5 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -229,6 +229,8 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
// Exception handling requires additional IR. If the 'await_resume' function
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
@@ -244,6 +246,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
+// We circumvent emitting the entire try stmt to get rvalue of the expr.
CGF.EnterCXXTryStmt(*TryStmt);
}
@@ -708,9 +711,9 @@ void CodeGenFunction::EmitCoroutineBody(const
CoroutineBodyStmt &S) {
CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
-EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
+EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
diff --git a/clang/lib/CodeGen/CGException.cpp
b/clang/lib/CodeGen/CGException.cpp
index bae8babb8efe4a8..99c30a5837ac1bb 100644
--- a/clang/lib/CodeGen/CGException.cpp
+++ b/clang/lib/CodeGen/CGException.cpp
@@ -1202,6 +1202,7 @@ void CodeGenFunction::popCatchScope() {
void CodeGenFunction::ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
unsigned NumHandlers = S.getNumHandlers();
+
EHCatchScope &CatchScope = cast(*EHStack.begin());
assert(CatchScope.getNumHandlers() == NumHandlers);
llvm::BasicBlock *DispatchBlock = CatchScope.getCachedEHDispatchBlock();
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 2199d7b58fb96e6..1a4bca20cc1a112 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2925,3 +2925,11 @@ llvm::Value
*CodeGenFunction::emitBoolVecConversion(llvm::Value *SrcVec,
return Builder.CreateShuffleVector(SrcVec, ShuffleMask, Name);
}
+
+void CodeGenFunction::printEHStack() const {
+ llvm::dbgs() << "EHStack: ";
+ for (const auto& scope : EHStack) {
+llvm::dbgs() << scope.getKind() << " ";
+ }
+ llvm::dbgs() << "\n";
+}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h
b/clang/lib/CodeGen/CodeGenFunction.h
index 618e78809db408b..36a82b63d0b1ff6 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -769,6 +769,8 @@ class CodeGenFunction : public CodeGenTypeCache {
return CurrentFuncletPad && isa(CurrentFuncletPad);
}
+ void printEHStack() const;
+
/// pushFullExprCleanup - Push a cleanup to be run at the end of the
/// current full-expression. Safe against the possibility that
/// we're currently inside a conditionally-evaluated expression.
>From 72fcc02a032e28517d87c8c0fe8279b9833bc9a9 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH 2/2] remove try stmt around initial suspend, merge with main
try
---
clang/lib/CodeGen/CGCoroutine.cpp | 55 +--
1 file changed, 8 insertions(+), 47 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 78df459113090e5..4c171f36ff2e17c 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -231,24 +231,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Never create an LValue for an initial suspend.
assert(Kind != AwaitKind::Init || !forLValue);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
-Coro.ResumeEHVar =
-CGF.CreateTempAlloca(Builder
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72935
>From 1eae15f7880574d4e5e8a9ea8757883aa136fa44 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH] remove try stmt around initial suspend, merge with main try
---
clang/lib/CodeGen/CGCoroutine.cpp | 52 +--
1 file changed, 8 insertions(+), 44 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..bbbc35357db9e87 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -229,23 +229,8 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
-Coro.ResumeEHVar =
-CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
-Builder.CreateFlagStore(true, Coro.ResumeEHVar);
-
-auto Loc = S.getResumeExpr()->getExprLoc();
-auto *Catch = new (CGF.getContext())
-CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
-auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
- FPOptionsOverride(), Loc, Loc);
-TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
-CGF.EnterCXXTryStmt(*TryStmt);
- }
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
LValueOrRValue Res;
if (forLValue)
@@ -253,11 +238,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
@@ -711,38 +691,22 @@ void CodeGenFunction::EmitCoroutineBody(const
CoroutineBodyStmt &S) {
EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
-CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-
if (CurCoro.Data->ExceptionHandler) {
- // If we generated IR to record whether an exception was thrown from
- // 'await_resume', then use that IR to determine whether the coroutine
- // body should be skipped.
- // If we didn't generate the IR (perhaps because 'await_resume' was
marked
- // as 'noexcept'), then we skip this check.
- BasicBlock *ContBB = nullptr;
- if (CurCoro.Data->ResumeEHVar) {
-BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
-ContBB = createBasicBlock("coro.resumed.cont");
-Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
- "coro.resumed.eh");
-Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
-EmitBlock(BodyBB);
- }
-
auto Loc = S.getBeginLoc();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
CurCoro.Data->ExceptionHandler);
+
auto *TryStmt =
CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
EnterCXXTryStmt(*TryStmt);
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
ExitCXXTryStmt(*TryStmt);
-
- if (ContBB)
-EmitBlock(ContBB);
-}
-else {
+} else {
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, S.getBody());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72935
>From 354b4390725f9142838e2c8d3d09b058f9519c10 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH] remove try stmt around initial suspend, merge with main try
---
clang/lib/CodeGen/CGCoroutine.cpp | 62 ---
1 file changed, 8 insertions(+), 54 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..1317e075f69fa6b 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,16 +129,6 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
-}
-
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -229,23 +219,8 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
-Coro.ResumeEHVar =
-CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
-Builder.CreateFlagStore(true, Coro.ResumeEHVar);
-
-auto Loc = S.getResumeExpr()->getExprLoc();
-auto *Catch = new (CGF.getContext())
-CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
-auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
- FPOptionsOverride(), Loc, Loc);
-TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
-CGF.EnterCXXTryStmt(*TryStmt);
- }
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
LValueOrRValue Res;
if (forLValue)
@@ -253,11 +228,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
@@ -711,38 +681,22 @@ void CodeGenFunction::EmitCoroutineBody(const
CoroutineBodyStmt &S) {
EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
-CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-
if (CurCoro.Data->ExceptionHandler) {
- // If we generated IR to record whether an exception was thrown from
- // 'await_resume', then use that IR to determine whether the coroutine
- // body should be skipped.
- // If we didn't generate the IR (perhaps because 'await_resume' was
marked
- // as 'noexcept'), then we skip this check.
- BasicBlock *ContBB = nullptr;
- if (CurCoro.Data->ResumeEHVar) {
-BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
-ContBB = createBasicBlock("coro.resumed.cont");
-Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
- "coro.resumed.eh");
-Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
-EmitBlock(BodyBB);
- }
-
auto Loc = S.getBeginLoc();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
CurCoro.Data->ExceptionHandler);
+
auto *TryStmt =
CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
EnterCXXTryStmt(*TryStmt);
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
ExitCXXTryStmt(*TryStmt);
-
- if (ContBB)
-EmitBlock(ContBB);
-}
-else {
+} else {
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, S.getBody());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/72935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/72935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72935
>From 1179379f36e4e52b8478af001a409546e3ff5859 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH] remove try stmt around initial suspend, merge with main try
---
clang/lib/CodeGen/CGCoroutine.cpp | 63 ---
1 file changed, 8 insertions(+), 55 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..30b3bcefa48efff 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,16 +129,6 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
-}
-
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -229,23 +219,8 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
-Coro.ResumeEHVar =
-CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
-Builder.CreateFlagStore(true, Coro.ResumeEHVar);
-
-auto Loc = S.getResumeExpr()->getExprLoc();
-auto *Catch = new (CGF.getContext())
-CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
-auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
- FPOptionsOverride(), Loc, Loc);
-TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
-CGF.EnterCXXTryStmt(*TryStmt);
- }
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
LValueOrRValue Res;
if (forLValue)
@@ -253,11 +228,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
@@ -708,41 +678,24 @@ void CodeGenFunction::EmitCoroutineBody(const
CoroutineBodyStmt &S) {
CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
-EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
-CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-
if (CurCoro.Data->ExceptionHandler) {
- // If we generated IR to record whether an exception was thrown from
- // 'await_resume', then use that IR to determine whether the coroutine
- // body should be skipped.
- // If we didn't generate the IR (perhaps because 'await_resume' was
marked
- // as 'noexcept'), then we skip this check.
- BasicBlock *ContBB = nullptr;
- if (CurCoro.Data->ResumeEHVar) {
-BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
-ContBB = createBasicBlock("coro.resumed.cont");
-Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
- "coro.resumed.eh");
-Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
-EmitBlock(BodyBB);
- }
-
auto Loc = S.getBeginLoc();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
CurCoro.Data->ExceptionHandler);
+
auto *TryStmt =
CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
EnterCXXTryStmt(*TryStmt);
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
ExitCXXTryStmt(*TryStmt);
-
- if (ContBB)
-EmitBlock(ContBB);
-}
-else {
+} else {
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, S.getBody());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/72935
>From 9d89a0bf81290b58344f4d5f220f62a3a256618d Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 20 Nov 2023 17:01:05 -0800
Subject: [PATCH] remove try stmt around initial suspend, merge with main try
---
clang/lib/CodeGen/CGCoroutine.cpp | 69 ---
1 file changed, 8 insertions(+), 61 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..6a6cd91baf6bea9 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -46,12 +46,6 @@ struct clang::CodeGen::CGCoroData {
// The promise type's 'unhandled_exception' handler, if it defines one.
Stmt *ExceptionHandler = nullptr;
- // A temporary i1 alloca that stores whether 'await_resume' threw an
- // exception. If it did, 'true' is stored in this variable, and the coroutine
- // body must be skipped. If the promise type does not define an exception
- // handler, this is null.
- llvm::Value *ResumeEHVar = nullptr;
-
// Stores the jump destination just before the coroutine memory is freed.
// This is the destination that every suspend point jumps to for the cleanup
// branch.
@@ -129,16 +123,6 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
-}
-
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -229,23 +213,8 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
- // Exception handling requires additional IR. If the 'await_resume' function
- // is marked as 'noexcept', we avoid generating this additional IR.
- CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
-Coro.ResumeEHVar =
-CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
-Builder.CreateFlagStore(true, Coro.ResumeEHVar);
-
-auto Loc = S.getResumeExpr()->getExprLoc();
-auto *Catch = new (CGF.getContext())
-CXXCatchStmt(Loc, /*exDecl=*/nullptr, Coro.ExceptionHandler);
-auto *TryBody = CompoundStmt::Create(CGF.getContext(), S.getResumeExpr(),
- FPOptionsOverride(), Loc, Loc);
-TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
-CGF.EnterCXXTryStmt(*TryStmt);
- }
+ // Never create an LValue for an initial suspend.
+ assert(Kind != AwaitKind::Init || !forLValue);
LValueOrRValue Res;
if (forLValue)
@@ -253,11 +222,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
@@ -708,41 +672,24 @@ void CodeGenFunction::EmitCoroutineBody(const
CoroutineBodyStmt &S) {
CurCoro.Data->CurrentAwaitKind = AwaitKind::Init;
CurCoro.Data->ExceptionHandler = S.getExceptionHandler();
-EmitStmt(S.getInitSuspendStmt());
CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB);
-CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
-
if (CurCoro.Data->ExceptionHandler) {
- // If we generated IR to record whether an exception was thrown from
- // 'await_resume', then use that IR to determine whether the coroutine
- // body should be skipped.
- // If we didn't generate the IR (perhaps because 'await_resume' was
marked
- // as 'noexcept'), then we skip this check.
- BasicBlock *ContBB = nullptr;
- if (CurCoro.Data->ResumeEHVar) {
-BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
-ContBB = createBasicBlock("coro.resumed.cont");
-Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
- "coro.resumed.eh");
-Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
-EmitBlock(BodyBB);
- }
-
auto Loc = S.getBeginLoc();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
CurCoro.Data->ExceptionHandler);
+
auto *TryStmt =
CXXTryStmt::Create(getContext(), Loc, S.getBody(), &Catch);
EnterCXXTryStmt(*TryStmt);
+ EmitStmt(S.getInitSuspendStmt());
+ CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Ex
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
yuxuanchen1997 wrote: Deleting `memberCallExpressionCanThrow` should be ok. It has been buggy before (the statement wasn't even a `CXXMemberCallExp`). It shouldn't matter if the init suspend await resume is noexcept or not. The old logic did not generate for case where the coro's exception handler wasn't there either. https://github.com/llvm/llvm-project/pull/72935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/73073
This change aims to fix an ICE in issue
https://github.com/llvm/llvm-project/issues/63803
The crash happens in `ExitCXXTryStmt` because `EmitAnyExpr()` adds additional
cleanup to the `EHScopeStack`. This messes up the assumption in
`ExitCXXTryStmt` that the top of the stack should be a `EHCatchScope`.
However, since we never read a value returned from `await_resume()` of an init
suspend, we can skip the part that builds this `RValue`.
>From f782f36c42f9bc1246837bf7ff2142919794580b Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From f782f36c42f9bc1246837bf7ff2142919794580b Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/2] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From 9f98b9d0b812b1027bfc4d963b353feeac36834b Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/2] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 49 +++
1 file changed, 49 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..78fc88a071d5c74
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+// Nontrivial type caused crash!
+NontrivialType await_resume() noexcept {
+ return {};
+}
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From e7d1ae077d7d301094b663166cc0c14c706d2110 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/2] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From f2cba69fc1c06fac2bdea5d3c66ac0b752248646 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/2] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 49 +++
1 file changed, 49 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..78fc88a071d5c74
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+// Nontrivial type caused crash!
+NontrivialType await_resume() noexcept {
+ return {};
+}
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/73073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From e7d1ae077d7d301094b663166cc0c14c706d2110 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/2] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From e11b48867b2f02a095d043d57cf7830702f47ed6 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/2] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 46 +++
1 file changed, 46 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..c4b8da327f5c140
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE when `initial_suspend()`'s `await_resume()` returns a non-trivially destructible type (PR #72935)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/72935 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From e7d1ae077d7d301094b663166cc0c14c706d2110 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/3] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From e11b48867b2f02a095d043d57cf7830702f47ed6 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/3] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 46 +++
1 file changed, 46 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..c4b8da327f5c140
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
>From 231760360962cf980f06f2b83d1467fd2c9f1078 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:52:34 -0800
Subject: [PATCH 3/3] Add release note entry
---
clang/docs/ReleaseNotes.rst | 4
clang/lib/CodeGen/CGCoroutine.cpp | 2 ++
2 files changed, 6 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 157afd9e8629152..9f9f83c673b7ba7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -592,6 +592,7 @@ Bug Fixes in This Version
- Clang now properly diagnoses use of stand-alone OpenMP directives after a
label (including ``case`` or ``default`` labels).
+
Before:
.. code-block:: c++
@@ -610,6 +611,9 @@ Bug Fixes in This Version
inside a lambda. (`#61460
`_)
- Fix crash during instantiation of some class template specializations within
class
templates. Fixes (`#70375
`_)
+- Fix crash during code generation of C++ coroutine initial suspend when the
+ return type of await_resume is not trivially destructible.
+ Fixes (`#63803 `_)
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/CodeGen/CGC
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From e7d1ae077d7d301094b663166cc0c14c706d2110 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/3] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From e11b48867b2f02a095d043d57cf7830702f47ed6 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/3] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 46 +++
1 file changed, 46 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..c4b8da327f5c140
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
>From c88474fa608219bf1c52fa09e91572dc85ddd1f8 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:52:34 -0800
Subject: [PATCH 3/3] Add release note entry
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/CodeGen/CGCoroutine.cpp | 2 ++
2 files changed, 5 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 157afd9e8629152..b65106b9106d4d7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -610,6 +610,9 @@ Bug Fixes in This Version
inside a lambda. (`#61460
`_)
- Fix crash during instantiation of some class template specializations within
class
templates. Fixes (`#70375
`_)
+- Fix crash during code generation of C++ coroutine initial suspend when the
return
+ type of await_resume is not trivially destructible.
+ Fixes (`#63803 `_)
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..ec30c974253d95f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -130,6 +130,8 @@ static SmallString<32>
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73073
>From e7d1ae077d7d301094b663166cc0c14c706d2110 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:06:31 -0800
Subject: [PATCH 1/4] [Clang][coro] Fix crash on emitting init suspend if the
return type of await_resume() is not trivially destructible
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 7e449d5af3423cf..aaf122c0f83bc47 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -245,6 +245,15 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
FPOptionsOverride(), Loc, Loc);
TryStmt = CXXTryStmt::Create(CGF.getContext(), Loc, TryBody, Catch);
CGF.EnterCXXTryStmt(*TryStmt);
+CGF.EmitStmt(TryBody);
+// We don't use EmitCXXTryStmt here. We need to store to ResumeEHVar that
+// doesn't exist in the body.
+Builder.CreateFlagStore(false, Coro.ResumeEHVar);
+CGF.ExitCXXTryStmt(*TryStmt);
+LValueOrRValue Res;
+// We are not supposed to obtain the value from init suspend
await_resume().
+Res.RV = RValue::getIgnored();
+return Res;
}
LValueOrRValue Res;
@@ -253,11 +262,6 @@ static LValueOrRValue
emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co
else
Res.RV = CGF.EmitAnyExpr(S.getResumeExpr(), aggSlot, ignoreResult);
- if (TryStmt) {
-Builder.CreateFlagStore(false, Coro.ResumeEHVar);
-CGF.ExitCXXTryStmt(*TryStmt);
- }
-
return Res;
}
>From e11b48867b2f02a095d043d57cf7830702f47ed6 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:35:10 -0800
Subject: [PATCH 2/4] add test case for the previously crashing case
---
.../coro-init-await-nontrivial-return.cpp | 46 +++
1 file changed, 46 insertions(+)
create mode 100644
clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
new file mode 100644
index 000..c4b8da327f5c140
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK: init.ready:
+// CHECK-NEXT: store i1 true, ptr {{.*}}
+// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: store i1 false, ptr {{.*}}
>From c88474fa608219bf1c52fa09e91572dc85ddd1f8 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 19:52:34 -0800
Subject: [PATCH 3/4] Add release note entry
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/CodeGen/CGCoroutine.cpp | 2 ++
2 files changed, 5 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 157afd9e8629152..b65106b9106d4d7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -610,6 +610,9 @@ Bug Fixes in This Version
inside a lambda. (`#61460
`_)
- Fix crash during instantiation of some class template specializations within
class
templates. Fixes (`#70375
`_)
+- Fix crash during code generation of C++ coroutine initial suspend when the
return
+ type of await_resume is not trivially destructible.
+ Fixes (`#63803 `_)
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..ec30c974253d95f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -130,6 +130,8 @@ static SmallString<32>
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/73073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines code gen check for member call noexcept nested in a temp expr (PR #73160)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/73160
Previously we were not properly skipping the generation of the `try { }` block
around the `init_suspend.await_resume()` if the `await_resume` is not returning
void. The reason being that the resume expression was wrapped in a
`CXXBindTemporaryExpr` and the first dyn_cast failed, silently ignoring the
noexcept. This only mattered for `init_suspend` because it had its own try
block.
This patch changes that to extract the sub expression when we see a
`CXXBindTemporaryExpr`. Another version of this patch also wanted to assert by
`cast` and as far as I understand it should be a valid
assumption. I can change to that if upstream prefers.
>From c17522689ef4449d0c8c464dcdd3092c081c07e6 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 14 --
.../coro-init-await-nontrivial-return.cpp | 50 +--
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..724d471cc9d78b6 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -12,9 +12,10 @@
#include "CGCleanup.h"
#include "CodeGenFunction.h"
-#include "llvm/ADT/ScopeExit.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
using namespace clang;
using namespace CodeGen;
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
@@ -233,7 +241,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
+ ResumeExprCanThrow(S)) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
index c4b8da327f5c140..0e50a616d6ef7c2 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -3,6 +3,7 @@
#include "Inputs/coroutine.h"
+namespace can_throw {
struct NontrivialType {
~NontrivialType() {}
};
@@ -38,9 +39,52 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
-// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN9can_throw14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() noexcept { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+Task get_return_object() {
+return Task{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+Task coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN8no_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN8no_throw14NontrivialTypeD1Ev(
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/c
[clang] [Clang] Coroutines code gen check for member call noexcept nested in a temp expr (PR #73160)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
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
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 Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
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
Date: Fri, 27 Oct 2023 16:37:40 -0700
Subject: [PATCH 1/2] [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 Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/2] fix small bug: setLastReferenceInCoroutineParamMoves
should respect pa
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
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
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 Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/3] fix small bug: setLastReferenceInCoroutineParamMoves
should respect pa
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/70567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
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
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 Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/3] fix small bug: setLastReferenceInCoroutineParamMoves
should respect pa
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/70567
>From 77b9cba0aaa1157cc323f2f3ef7b1cef536ef147 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Fri, 27 Oct 2023 16:37:40 -0700
Subject: [PATCH 1/5] [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 38ac406b14adadf..3af42cae9ba0420 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1965,9 +1965,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 824267acbb1c04e..6ce4871152a4e78 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15092,7 +15092,10 @@ void
Sema::DiagnoseUnusedParameters(ArrayRef Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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 c094a4fa29142b33cb15c41f5544395d3e87473c Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/5] fix small bug: setLastReferenceInCoroutineParamMoves
should respect pa
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
yuxuanchen1997 wrote: @ChuanqiXu9 , I just updated the PR. Do you mind checking if using a `RecursiveASTVisitor` is appropriate for this? I am a first time contributor and would appreciate more feedback. Thank you. https://github.com/llvm/llvm-project/pull/70567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
@@ -3165,7 +3165,16 @@ class Sema final {
/// Diagnose any unused parameters in the given sequence of
/// ParmVarDecl pointers.
- void DiagnoseUnusedParameters(ArrayRef Parameters);
+ ///
+ /// Normally, we check if the parameter decls have the Referenced bit set.
+ /// C++ Coroutines, however, are a special case due to the existences of
+ /// parameter moves (See Sema::buildCoroutineParameterMoves), the parameters
+ /// are always referenced in coroutines. Therefore, in the case of
coroutines,
+ /// CoroutineBodyRefs must be passed to correctly diagnose parameter usages
+ /// as written by the user.
+ void DiagnoseUnusedParameters(
+ ArrayRef Parameters,
+ llvm::SmallSet *CoroutineBodyRefs = nullptr);
yuxuanchen1997 wrote:
If so, what about adding an assertion in the original version of
`DiagnoseUnusedParameters`? Or should we keep the interface as is (allowing
coroutines to be diagnosed in the same function, but handle the special case in
`DiagnoseUnusedParameters` without changing the interface?)
`CoroutineParameterMoves` does not give me what I want. It points me to `Stmt`s
that moved the parameters while I need the opposite -- All other references to
the parameters.
https://github.com/llvm/llvm-project/pull/70567
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/70567
>From 77b9cba0aaa1157cc323f2f3ef7b1cef536ef147 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Fri, 27 Oct 2023 16:37:40 -0700
Subject: [PATCH 1/6] [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 38ac406b14adadf..3af42cae9ba0420 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1965,9 +1965,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 824267acbb1c04e..6ce4871152a4e78 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15092,7 +15092,10 @@ void
Sema::DiagnoseUnusedParameters(ArrayRef Parameters) {
return;
for (const ParmVarDecl *Parameter : Parameters) {
-if (!Parameter->isReferenced() && Parameter->getDeclName() &&
+if (Parameter->isReferenced() &&
!Parameter->isLastReferenceInCoroutineParamMoves())
+ continue;
+
+if (Parameter->getDeclName() &&
!Parameter->hasAttr() &&
!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 000..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 c094a4fa29142b33cb15c41f5544395d3e87473c Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Sat, 28 Oct 2023 12:42:01 -0700
Subject: [PATCH 2/6] fix small bug: setLastReferenceInCoroutineParamMoves
should respect pa
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
@@ -3165,7 +3165,16 @@ class Sema final {
/// Diagnose any unused parameters in the given sequence of
/// ParmVarDecl pointers.
- void DiagnoseUnusedParameters(ArrayRef Parameters);
+ ///
+ /// Normally, we check if the parameter decls have the Referenced bit set.
+ /// C++ Coroutines, however, are a special case due to the existences of
+ /// parameter moves (See Sema::buildCoroutineParameterMoves), the parameters
+ /// are always referenced in coroutines. Therefore, in the case of
coroutines,
+ /// CoroutineBodyRefs must be passed to correctly diagnose parameter usages
+ /// as written by the user.
+ void DiagnoseUnusedParameters(
+ ArrayRef Parameters,
+ llvm::SmallSet *CoroutineBodyRefs = nullptr);
yuxuanchen1997 wrote:
> Sorry, what do you mean by adding an assertion to DiagnoseUnusedParameters?
> Do you say doing so after adopting my suggest? Then, it sounds good but how
> can we do that? And it sounds not meaningful.
I didn't make it clear. I was asking what you may think about an assertion in
DiagnoseUnusedParameters that !CurFunction()->isCoroutine()
> An instinct idea now is to mark all the parameters as non referenced/used
> after building CoroutineParameterMoves. But it needs to be verified.
New to clang so I can't tell if I did something wrong. But the compiler crashed
when I initially tried that.
I have to sign off now. Catch up with you soon.
https://github.com/llvm/llvm-project/pull/70567
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/70567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/70567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Warn against unused parameters in C++ coroutines with `-Wunused-parameters` (PR #70567)
yuxuanchen1997 wrote: @ChuanqiXu9 , I think I got it wrong last time. It really should be a trivial change. Let me prepare a new PR. https://github.com/llvm/llvm-project/pull/70567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/70973
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 patch addresses this issue by preserving the original `ParmVarDecl`'s
`Referenced` state.
>From 54255c560c01a1fd017c52898b8688e24c4397e5 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 1 Nov 2023 11:59:57 -0700
Subject: [PATCH] [Clang] Preserve coroutine parameter referenced state
---
clang/lib/Sema/SemaCoroutine.cpp | 6
.../warn-unused-parameters-coroutine.cpp | 28 +++
2 files changed, 34 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 38ac406b14adadf..281b7d68d361fcc 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state of the parameters for diagnosis purposes.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
+
if (PDRefExpr.isInvalid())
return false;
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 000..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;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/70973
>From 453dd8c6892923118c8952e92fdb7375f7ed4877 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 1 Nov 2023 11:59:57 -0700
Subject: [PATCH] [Clang] Preserve coroutine parameter referenced state
---
clang/lib/Sema/SemaCoroutine.cpp | 6
.../warn-unused-parameters-coroutine.cpp | 28 +++
2 files changed, 34 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 38ac406b14adadf..bee80db8d166a68 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
+
if (PDRefExpr.isInvalid())
return false;
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 000..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;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/70973
>From 3be73f10d376739460f0c22d3292bc0de8f64b82 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 1 Nov 2023 11:59:57 -0700
Subject: [PATCH] [Clang] Preserve coroutine parameter referenced state
---
clang/lib/Sema/SemaCoroutine.cpp | 6
.../warn-unused-parameters-coroutine.cpp | 34 +++
2 files changed, 40 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 38ac406b14adadf..bee80db8d166a68 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
+
if (PDRefExpr.isInvalid())
return false;
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 000..b4c01550f9f7883
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unused-parameters-coroutine.cpp
@@ -0,0 +1,34 @@
+// 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;
+}
+
+void create_closure() {
+ auto closure = [](int c) -> task { // expected-warning{{unused parameter
'c'}}
+co_return;
+ };
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
yuxuanchen1997 wrote: @ChuanqiXu9 it was my mistake. This patch does work. https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
ExprResult PDRefExpr =
BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);
yuxuanchen1997 wrote:
`BuildDeclRefExpr` builds a `DeclRef` `Expr` and marks the referenced `Decl` as
`Referenced`.
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation
Loc) {
if (PD->getType()->isDependentType())
continue;
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
yuxuanchen1997 wrote:
`PD->isReferenced()` may be true. The order `Sema` processes things is based on
how Parser is sending information to it. I can try to explain based on my
limited understanding and corroborate with a test case. So please feel free to
correct me if I am wrong.
Let's take a look at this very simple coroutine. Suppose `task` is defined
somewhere as a return object.
```
task foo(int a, int b) {
a = a + 1;
co_return;
}
```
Sema is first asked to compile `foo` like a normal function. Decl `a` and `b`
are processed normally. When we use `a` in the `a = a + 1` statement,
references are setup.
When the parser encounters `co_return`, it calls `Sema::ActOnCoreturn`.
What `Sema::ActOnCoreturn` (and its siblings handling other `co_` family of
statements) does is to call `Sema::ActOnCoroutineBodyStart`. It checks if clang
is already treating `foo` as a coroutine. It will find out that, no. We are not
compiling `foo` like a coroutine yet, let's perform necessary checks and
establish this context. `buildCoroutineParameterMoves` is called as a result.
At that time `a` already has references we need to carry along.
Suppose that our function has subsequent `co_*` statements, `Sema` knows that
we already established the context and doesn't build them again.
The aforementioned `foo` coroutine will cause clang ICE if we
`assert(!PD->isReferenced())` here.
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/70973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
yuxuanchen1997 wrote:
I find this a little overkill for what we need. The old code here didn't intend
to handle more cases either because it just expected the AST to be in a
specific shape (`CXXMemberCallExpr`). This fix is merely just to amend its
functionality to work on a temporary expression as well should there be a
return value to be discarded. It should be clear intent here that we don't care
about anything else. (Ideally, I want an assertion here that it's either of the
two `Expr` types)
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73160
>From 08e2293255a504043fe404cceaeb3ff1fc0dc344 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 11 -
.../coro-init-await-nontrivial-return.cpp | 44 ++-
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..8aebc5563757cba 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,7 +129,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
@@ -233,7 +240,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
+ ResumeExprCanThrow(S)) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
index c4b8da327f5c140..5d24841091f339c 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -7,6 +7,7 @@ struct NontrivialType {
~NontrivialType() {}
};
+namespace can_throw {
struct Task {
struct promise_type;
using handle_type = std::coroutine_handle;
@@ -38,9 +39,48 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct InitNoThrowTask {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() noexcept { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+InitNoThrowTask get_return_object() {
+return InitNoThrowTask{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+InitNoThrowTask coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN8no_throw15InitNoThrowTask23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -38,9 +39,52 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
-// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN9can_throw14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
yuxuanchen1997 wrote:
I applied your suggested change. I think this name is still bikesheddable
though.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][NFC] Refactor suspend emit logic in coroutine codegen (PR #73564)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/73564
Depends on https://github.com/llvm/llvm-project/pull/73160
I am not a big fan of the `struct LValueOrRValue`. It appears to me that the
intent is to use it like a union, without explicitly saying `union`. While you
don't have the risk of UB when reading the wrong member, it's still an annoying
thing that it doesn't carry any tag on which arm is chosen.
This PR is not trying to make functional changes. Instead, I am proposing this
new style of using a helper class and its methods to emit different values.
Before:
```
emitSuspendExpression(*this, *CurCoro.Data, E, CurCoro.Data->CurrentAwaitKind,
aggSlot, ignoreResult, /*forLValue*/false).RV
```
After:
```
SuspendExpressionEmitter(*this, *CurCoro.Data, E,
CurCoro.Data->CurrentAwaitKind).EmitAsRValue(aggSlot, ignoreResult)
```
>From 08e2293255a504043fe404cceaeb3ff1fc0dc344 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH 1/2] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 11 -
.../coro-init-await-nontrivial-return.cpp | 44 ++-
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..8aebc5563757cba 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,7 +129,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
@@ -233,7 +240,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
+ ResumeExprCanThrow(S)) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
index c4b8da327f5c140..5d24841091f339c 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -7,6 +7,7 @@ struct NontrivialType {
~NontrivialType() {}
};
+namespace can_throw {
struct Task {
struct promise_type;
using handle_type = std::coroutine_handle;
@@ -38,9 +39,48 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct InitNoThrowTask {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType await_resume() noexcept { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+InitNoThrowTask get_return_object() {
+return InitNoThrowTask{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+InitNoThrowTask coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN8no_throw15InitNoThrowTask23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+}
>From 619d4517550d6b3226fbb48146419ae68bb18694 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 22 Nov 2023 13:38:54 -0800
Subject: [PATCH 2/2] Refactor how we generate RValue vs LValue coawait
expressions
---
clang/lib/CodeGen/CGCoroutine.cpp | 272 +-
1 file changed, 152
[clang] [Clang][NFC] Refactor suspend emit logic in coroutine codegen (PR #73564)
https://github.com/yuxuanchen1997 ready_for_review https://github.com/llvm/llvm-project/pull/73564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
yuxuanchen1997 wrote:
I guess you are correct that this is easy to miss some cases. In my case I
missed the fact that the destructor of the `BindTempExpr->getTemporary()` can
throw if marked `noexcept(false)`. I updated the PR to cover this case and
including corresponding tests.
Next up I will try to see how many cases I'll need to cover in a recursive
check on `children()` like the one in `Sema::checkFinalSuspendNoThrow`.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73160
>From dbd06bc001c0e00436fcacac231d9d80b443edf4 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH 1/2] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 28 ++--
.../coro-init-await-nontrivial-return.cpp | 66 ++-
2 files changed, 88 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..37ac5bca38c7049 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,12 +129,32 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool FunctionProtoNoexcept(const FunctionProtoType *Proto) {
+ return isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot;
+}
+
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its SubExpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+auto *Temporary = BindTempExpr->getTemporary();
+const auto *DtorProto = Temporary->getDestructor()
+->getCanonicalDecl()
+->getType()
+->getAs();
+bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto);
+if (DtorCanThrow) {
+ return true;
+}
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
+ if (FunctionProtoNoexcept(Proto))
return false;
return true;
}
@@ -233,7 +253,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
+ ResumeExprCanThrow(S)) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
index c4b8da327f5c140..052b4e235e739fe 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -7,6 +7,11 @@ struct NontrivialType {
~NontrivialType() {}
};
+struct NontrivialTypeWithThrowingDtor {
+ ~NontrivialTypeWithThrowingDtor() noexcept(false) {}
+};
+
+namespace can_throw {
struct Task {
struct promise_type;
using handle_type = std::coroutine_handle;
@@ -38,9 +43,66 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+template
+struct NoexceptResumeTask {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+R await_resume() noexcept { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+NoexceptResumeTask get_return_object() {
+return NoexceptResumeTask{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+namespace no_throw {
+using InitNoThrowTask = NoexceptResumeTask;
+
+InitNoThrowTask coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN18NoexceptResumeTaskI14NontrivialTypeE23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+}
+
+namespace throwing_dtor {
+using InitTaskWithThrowingDtor =
NoexceptResumeTask;
+
+InitTaskWithThrowingDtor coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN13t
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
yuxuanchen1997 wrote: @ChuanqiXu9, I updated the check like you suggested, and it should look much nicer now. The only caveat is that the `children()` call isn't going to cover the implicit destructor call in a `CXXBindTemporaryExpr`. That pattern matching case is here to stay and likely there are many other types implicit calls that make this analysis unsound (See https://eel.is/c++draft/except.spec#6). In reality none of them can reach here though. https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73160
>From dbd06bc001c0e00436fcacac231d9d80b443edf4 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH 1/3] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 28 ++--
.../coro-init-await-nontrivial-return.cpp | 66 ++-
2 files changed, 88 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..37ac5bca38c7049 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,12 +129,32 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool FunctionProtoNoexcept(const FunctionProtoType *Proto) {
+ return isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot;
+}
+
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its SubExpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+auto *Temporary = BindTempExpr->getTemporary();
+const auto *DtorProto = Temporary->getDestructor()
+->getCanonicalDecl()
+->getType()
+->getAs();
+bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto);
+if (DtorCanThrow) {
+ return true;
+}
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
+ if (FunctionProtoNoexcept(Proto))
return false;
return true;
}
@@ -233,7 +253,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
&CGF, CGCoroData &Co
// is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
- memberCallExpressionCanThrow(S.getResumeExpr())) {
+ ResumeExprCanThrow(S)) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
index c4b8da327f5c140..052b4e235e739fe 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -7,6 +7,11 @@ struct NontrivialType {
~NontrivialType() {}
};
+struct NontrivialTypeWithThrowingDtor {
+ ~NontrivialTypeWithThrowingDtor() noexcept(false) {}
+};
+
+namespace can_throw {
struct Task {
struct promise_type;
using handle_type = std::coroutine_handle;
@@ -38,9 +43,66 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+template
+struct NoexceptResumeTask {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+R await_resume() noexcept { return {}; }
+};
+
+struct promise_type {
+void return_void() {}
+void unhandled_exception() {}
+initial_suspend_awaiter initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+NoexceptResumeTask get_return_object() {
+return NoexceptResumeTask{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+namespace no_throw {
+using InitNoThrowTask = NoexceptResumeTask;
+
+InitNoThrowTask coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN18NoexceptResumeTaskI14NontrivialTypeE23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+}
+
+namespace throwing_dtor {
+using InitTaskWithThrowingDtor =
NoexceptResumeTask;
+
+InitTaskWithThrowingDtor coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN13t
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
+// Check if function can throw based on prototype noexcept, also works for
+// destructors which are implicitly noexcept but can be marked noexcept(false).
+static bool FunctionCanThrow(const FunctionDecl *D) {
+ const auto *Proto = D->getType()->getAs();
+ if (!Proto) {
+// Function proto is not found, we conservatively assume throwing.
+return true;
+ }
+ return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) ||
+ Proto->canThrow() != CT_Cannot;
+}
+
+static bool ResumeStmtCanThrow(const Stmt *S) {
+ if (const auto *CE = dyn_cast(S)) {
+const auto *Callee = CE->getDirectCallee();
+if (!Callee)
+ // We don't have direct callee. Conservatively assume throwing.
+ return true;
+
+if (FunctionCanThrow(Callee))
+ return true;
yuxuanchen1997 wrote:
The logic doesn't go this way.
If a function can throw, the task of this (conservative) analysis is to return
true and nothing else needs to be done. Otherwise, fall through and recursively
visit all children, which may include Stmts that throw.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][NFC] Refactor suspend emit logic in coroutine codegen (PR #73564)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73564
>From 5f5ebec41c90366bf3c7ec1ee53154ba7afcb849 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 22 Nov 2023 13:38:54 -0800
Subject: [PATCH] Refactor how we generate RValue vs LValue coawait expressions
---
clang/lib/CodeGen/CGCoroutine.cpp | 332 +-
1 file changed, 185 insertions(+), 147 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index 888d30bfb3e1d6a..472c260f60068bd 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,49 +129,7 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
&Coro, AwaitKind Kind) {
return Prefix;
}
-// Check if function can throw based on prototype noexcept, also works for
-// destructors which are implicitly noexcept but can be marked noexcept(false).
-static bool FunctionCanThrow(const FunctionDecl *D) {
- const auto *Proto = D->getType()->getAs();
- if (!Proto) {
-// Function proto is not found, we conservatively assume throwing.
-return true;
- }
- return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) ||
- Proto->canThrow() != CT_Cannot;
-}
-
-static bool ResumeStmtCanThrow(const Stmt *S) {
- if (const auto *CE = dyn_cast(S)) {
-const auto *Callee = CE->getDirectCallee();
-if (!Callee)
- // We don't have direct callee. Conservatively assume throwing.
- return true;
-
-if (FunctionCanThrow(Callee))
- return true;
-
-// Fall through to visit the children.
- }
-
- if (const auto *TE = dyn_cast(S)) {
-// Special handling of CXXBindTemporaryExpr here as calling of Dtor of the
-// temporary is not part of `children()` as covered in the fall through.
-// We need to mark entire statement as throwing if the destructor of the
-// temporary throws.
-const auto *Dtor = TE->getTemporary()->getDestructor();
-if (FunctionCanThrow(Dtor))
- return true;
-
-// Fall through to visit the children.
- }
-
- for (const auto *child : S->children())
-if (ResumeStmtCanThrow(child))
- return true;
-
- return false;
-}
+namespace {
// Emit suspend expression which roughly looks like:
//
@@ -200,117 +158,198 @@ static bool ResumeStmtCanThrow(const Stmt *S) {
//
// See llvm's docs/Coroutines.rst for more details.
//
-namespace {
- struct LValueOrRValue {
-LValue LV;
-RValue RV;
- };
-}
-static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData
&Coro,
-CoroutineSuspendExpr const &S,
-AwaitKind Kind, AggValueSlot aggSlot,
-bool ignoreResult, bool forLValue) {
- auto *E = S.getCommonExpr();
-
- auto Binder =
- CodeGenFunction::OpaqueValueMappingData::bind(CGF, S.getOpaqueValue(),
E);
- auto UnbindOnExit = llvm::make_scope_exit([&] { Binder.unbind(CGF); });
-
- auto Prefix = buildSuspendPrefixStr(Coro, Kind);
- BasicBlock *ReadyBlock = CGF.createBasicBlock(Prefix + Twine(".ready"));
- BasicBlock *SuspendBlock = CGF.createBasicBlock(Prefix + Twine(".suspend"));
- BasicBlock *CleanupBlock = CGF.createBasicBlock(Prefix + Twine(".cleanup"));
-
- // If expression is ready, no need to suspend.
- CGF.EmitBranchOnBoolExpr(S.getReadyExpr(), ReadyBlock, SuspendBlock, 0);
-
- // Otherwise, emit suspend logic.
- CGF.EmitBlock(SuspendBlock);
-
- auto &Builder = CGF.Builder;
- llvm::Function *CoroSave = CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_save);
- auto *NullPtr = llvm::ConstantPointerNull::get(CGF.CGM.Int8PtrTy);
- auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});
-
- CGF.CurCoro.InSuspendBlock = true;
- auto *SuspendRet = CGF.EmitScalarExpr(S.getSuspendExpr());
- CGF.CurCoro.InSuspendBlock = false;
-
- if (SuspendRet != nullptr && SuspendRet->getType()->isIntegerTy(1)) {
-// Veto suspension if requested by bool returning await_suspend.
-BasicBlock *RealSuspendBlock =
-CGF.createBasicBlock(Prefix + Twine(".suspend.bool"));
-CGF.Builder.CreateCondBr(SuspendRet, RealSuspendBlock, ReadyBlock);
-CGF.EmitBlock(RealSuspendBlock);
+class SuspendExpressionEmitter final {
+public:
+ SuspendExpressionEmitter(CodeGenFunction &CGF, CGCoroData &Coro,
+ CoroutineSuspendExpr const &S, AwaitKind Kind)
+ : CGF(CGF), Coro(Coro), SuspendExpr(S), Kind(Kind),
+SuspendPrefix(buildSuspendPrefixStr(Coro, Kind)) {
+CommonExpr = SuspendExpr.getCommonExpr();
+Binder = CodeGenFunction::OpaqueValueMappingData::bind(
+CGF, SuspendExpr.getOpaqueValue(), CommonExpr);
}
- // Emit the suspend point.
- const bool IsFinalSuspend = (Kind == AwaitKind::Final);
- llvm::Function *CoroSuspend =
- CGF.CGM.getIntrinsic(llvm::Intrinsic::coro_suspend);
- auto *SuspendResult = Builder.CreateCall(
- CoroSuspend, {SaveCall, Builder.getInt1(IsFinalSuspe
[clang] [Clang] Fix ICE where C++ Template Instantiation failed to handle attributed lambdas (PR #76523)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/76523
This PR is proposing a fix for
https://github.com/llvm/llvm-project/issues/76521.
Clang used to assume that during template instantiation, Lambda expressions can
only have `FunctionProtoTypeLoc`s. However, this is not true for certain
attributes like `__attribute__((pcs("aapcs-vfp")))`, whose interpretation
happens after template instantiation. This PR changes the transformation logic
for lambdas.
>From 09b339575b506317a5fbf5c4d52d3e5d0243a3c3 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Thu, 21 Dec 2023 14:48:52 -0800
Subject: [PATCH] attempt to fix the crash
---
clang/docs/ReleaseNotes.rst | 3 +
clang/lib/Sema/TreeTransform.h| 71 ++-
clang/test/SemaCXX/template-instantiation.cpp | 15
3 files changed, 72 insertions(+), 17 deletions(-)
create mode 100644 clang/test/SemaCXX/template-instantiation.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e21ec78a1e8a77..888a11bef15e7e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -856,6 +856,9 @@ Bug Fixes to AST Handling
- Fixed a bug where RecursiveASTVisitor fails to visit the
initializer of a bitfield.
`Issue 64916 `_
+- Fixed a bug where Template Instantiation failed to handle Lambda Expressions
+ with certain types of Attributes.
+ (`#76521 `_)
Miscellaneous Bug Fixes
^^^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7df5bf0cb71370..5e9b5184570704 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -674,6 +674,10 @@ class TreeTransform {
Qualifiers ThisTypeQuals,
Fn TransformExceptionSpec);
+ template
+ QualType TransformAttributedType(TypeLocBuilder &TLB, AttributedTypeLoc TL,
+ Fn TransformModifiedType);
+
bool TransformExceptionSpec(SourceLocation Loc,
FunctionProtoType::ExceptionSpecInfo &ESI,
SmallVectorImpl &Exceptions,
@@ -7050,12 +7054,12 @@
TreeTransform::TransformElaboratedType(TypeLocBuilder &TLB,
return Result;
}
-template
+template
+template
QualType TreeTransform::TransformAttributedType(
-TypeLocBuilder &TLB,
-AttributedTypeLoc TL) {
+TypeLocBuilder &TLB, AttributedTypeLoc TL, Fn TransformModifiedTypeFn) {
const AttributedType *oldType = TL.getTypePtr();
- QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());
+ QualType modifiedType = TransformModifiedTypeFn(TLB, TL.getModifiedLoc());
if (modifiedType.isNull())
return QualType();
@@ -7099,6 +7103,15 @@ QualType TreeTransform::TransformAttributedType(
return result;
}
+template
+QualType TreeTransform::TransformAttributedType(TypeLocBuilder &TLB,
+ AttributedTypeLoc TL)
{
+ return getDerived().TransformAttributedType(
+ TLB, TL, [&](TypeLocBuilder &TLB, TypeLoc ModifiedLoc) -> QualType {
+return getDerived().TransformType(TLB, ModifiedLoc);
+ });
+}
+
template
QualType TreeTransform::TransformBTFTagAttributedType(
TypeLocBuilder &TLB, BTFTagAttributedTypeLoc TL) {
@@ -13600,32 +13613,56 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
// transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
{
-TypeSourceInfo *OldCallOpTSI = E->getCallOperator()->getTypeSourceInfo();
-auto OldCallOpFPTL =
-OldCallOpTSI->getTypeLoc().getAs();
+auto OldCallOpTypeLoc =
+E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
+
+auto TransformFunctionProtoTypeLoc =
+[this](TypeLocBuilder &TLB, FunctionProtoTypeLoc FPTL) -> QualType {
+ SmallVector ExceptionStorage;
+ TreeTransform *This = this; // Work around gcc.gnu.org/PR56135.
+ return this->TransformFunctionProtoType(
+ TLB, FPTL, nullptr, Qualifiers(),
+ [&](FunctionProtoType::ExceptionSpecInfo &ESI, bool &Changed) {
+return This->TransformExceptionSpec(FPTL.getBeginLoc(), ESI,
+ExceptionStorage, Changed);
+ });
+};
+QualType NewCallOpType;
TypeLocBuilder NewCallOpTLBuilder;
-SmallVector ExceptionStorage;
-TreeTransform *This = this; // Work around gcc.gnu.org/PR56135.
-QualType NewCallOpType = TransformFunctionProtoType(
-NewCallOpTLBuilder, OldCallOpFPTL, nullptr, Qualifiers(),
-[&](FunctionProtoType::ExceptionSpecInfo &ESI, bool &Changed) {
- return This->TransformExceptionSpec(OldCallOpFPTL.getBeginLo
[clang] [Clang] Fix ICE where C++ Template Instantiation failed to handle attributed lambdas (PR #76523)
yuxuanchen1997 wrote: Gentle Ping. https://github.com/llvm/llvm-project/pull/76523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE where C++ Template Instantiation failed to handle attributed lambdas (PR #76523)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/76523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE where C++ Template Instantiation failed to handle attributed lambdas (PR #76523)
yuxuanchen1997 wrote: > @yuxuanchen1997 the test you added is failing due to an additional diagnostic > being produced on PS4/PS5 targets causing the test to fail. Can you please > take a look? > > * https://lab.llvm.org/buildbot/#/builders/139/builds/56396 > * https://lab.llvm.org/buildbot/#/builders/216/builds/32469 Let me find an alternative test. https://github.com/llvm/llvm-project/pull/76523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix failing CI with different test case attribute & test macro (PR #76863)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/76863
As in title,
https://github.com/llvm/llvm-project/commit/a8f43974260ec244d78336d2530f8fc097753580
broke CI due to the calling convention not available on certain targets. This
patch uses a simpler calling convention and enables the test only when the
attribute exists. It's verified that this test crashes the compiler before
a8f43974260ec244d78336d2530f8fc097753580 so it's the same effect as the
previous test. Disabling the test on platforms that don't have the calling
convention is fine because it's guarding against a frontend bug.
>From a51fcbc766cddbee5a0ec5e75c7b716ddb952a72 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 3 Jan 2024 12:18:21 -0800
Subject: [PATCH] Use a different calling convention attribute
---
clang/test/SemaCXX/template-instantiation.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/test/SemaCXX/template-instantiation.cpp
b/clang/test/SemaCXX/template-instantiation.cpp
index e714e070a206f5..a8ecafc401d9d0 100644
--- a/clang/test/SemaCXX/template-instantiation.cpp
+++ b/clang/test/SemaCXX/template-instantiation.cpp
@@ -3,13 +3,15 @@
namespace GH76521 {
+#if __has_attribute(preserve_most)
template
void foo() {
- auto l = []() __attribute__((pcs("aapcs-vfp"))) {};
+ auto l = []() __attribute__((preserve_most)) {};
}
void bar() {
foo();
}
+#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix ICE where C++ Template Instantiation failed to handle attributed lambdas (PR #76523)
yuxuanchen1997 wrote: @dyung https://github.com/llvm/llvm-project/pull/76863 https://github.com/llvm/llvm-project/pull/76523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix failing CI with different test case attribute & test macro (PR #76863)
yuxuanchen1997 wrote: Like this? https://github.com/llvm/llvm-project/pull/76863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix failing CI with different test case attribute & test macro (PR #76863)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/76863
>From 8c33419f2217a7eb98a591813c6ce6c5185ff6c7 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Wed, 3 Jan 2024 12:18:21 -0800
Subject: [PATCH] Use a different calling convention attribute
---
clang/test/SemaCXX/template-instantiation.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaCXX/template-instantiation.cpp
b/clang/test/SemaCXX/template-instantiation.cpp
index e714e070a206f5..8543af0d5428d0 100644
--- a/clang/test/SemaCXX/template-instantiation.cpp
+++ b/clang/test/SemaCXX/template-instantiation.cpp
@@ -1,11 +1,11 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -Wno-ignored-attributes %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -verify -fsyntax-only %s
// expected-no-diagnostics
namespace GH76521 {
template
void foo() {
- auto l = []() __attribute__((pcs("aapcs-vfp"))) {};
+ auto l = []() __attribute__((preserve_most)) {};
}
void bar() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix failing CI with different test case attribute & host triple (PR #76863)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/76863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix failing CI with different test case attribute & host triple (PR #76863)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/76863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
yuxuanchen1997 wrote: Great. Thanks! https://github.com/llvm/llvm-project/pull/85325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -13830,62 +13820,40 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
- // Transform the type of the original lambda's call operator.
- // The transformation MUST be done in the CurrentInstantiationScope since
- // it introduces a mapping of the original to the newly created
- // transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
- {
-auto OldCallOpTypeLoc =
-E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
-
-auto TransformFunctionProtoTypeLoc =
-[this](TypeLocBuilder &TLB, FunctionProtoTypeLoc FPTL) -> QualType {
- SmallVector ExceptionStorage;
- return this->TransformFunctionProtoType(
- TLB, FPTL, nullptr, Qualifiers(),
- [&](FunctionProtoType::ExceptionSpecInfo &ESI, bool &Changed) {
-return TransformExceptionSpec(FPTL.getBeginLoc(), ESI,
- ExceptionStorage, Changed);
- });
-};
-
-QualType NewCallOpType;
-TypeLocBuilder NewCallOpTLBuilder;
-
-if (auto ATL = OldCallOpTypeLoc.getAs()) {
- NewCallOpType = this->TransformAttributedType(
- NewCallOpTLBuilder, ATL,
- [&](TypeLocBuilder &TLB, TypeLoc TL) -> QualType {
-return TransformFunctionProtoTypeLoc(
-TLB, TL.castAs());
- });
-} else {
- auto FPTL = OldCallOpTypeLoc.castAs();
- NewCallOpType = TransformFunctionProtoTypeLoc(NewCallOpTLBuilder, FPTL);
-}
+ auto OldCallOpTypeLoc =
+ E->getCallOperator()->getTypeSourceInfo()->getTypeLoc();
-if (NewCallOpType.isNull())
- return ExprError();
-NewCallOpTSI =
-NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
- }
+ QualType NewCallOpType;
yuxuanchen1997 wrote:
ditto.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -13830,62 +13820,40 @@
TreeTransform::TransformLambdaExpr(LambdaExpr *E) {
getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
TPL);
- // Transform the type of the original lambda's call operator.
- // The transformation MUST be done in the CurrentInstantiationScope since
- // it introduces a mapping of the original to the newly created
- // transformed parameters.
TypeSourceInfo *NewCallOpTSI = nullptr;
yuxuanchen1997 wrote:
Branches don't exist anymore, inline this decl to where it's assigned?
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
yuxuanchen1997 wrote:
Is this assumption true? I added the two to my original PR because that's the
two I have seen but no confidence.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)
@@ -3140,13 +3140,35 @@ const FunctionType
*ASTContext::adjustFunctionType(const FunctionType *T,
return cast(Result.getTypePtr());
}
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+ QualType ResultType) {
+ // Might be wrapped in a macro qualified type.
+ if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+ }
+
+ // Might have a calling-convention attribute.
+ if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+ }
+
+ // Anything else must be a function type. Rebuild it with the new return
+ // value.
yuxuanchen1997 wrote:
I think what would be required for this type of work is to have the general
ability to traverse `Type`s and `TypeLoc`s. Implementation like this + the one
I provided in `TreeTransform::TransformLambdaExpr` is error prone and wouldn't
scale.
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
https://github.com/yuxuanchen1997 created
https://github.com/llvm/llvm-project/pull/85684
This patch aims to fix https://github.com/llvm/llvm-project/issues/77111
The original issue is crash-on-invalid. However, Clang currently accepts
programs where `await_suspend` returns a type that's convertible to
`std::coroutine_handle<>`. This is more relaxed than the standard. Under this
assumption, we mishandled the case where the return type has a user defined
destructor. The crash happens before we can check the return value of
`await_suspend`.
This path handles the case where the expression is wrapped in a
`CXXBindTemporaryExpr` and refactored a little bit how the diagnosis logic to
make it more explicit as suggested by a previous `FIXME` in `mayTailCall`.
I am also ok with changing the patch to reject programs whose `await_suspend`
doesn't strictly return `std::coroutine_handle` if upstream prefers.
>From 08de54f02038795924a6e5fdbcf51a496fcedf56 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 18 Mar 2024 10:45:20 -0700
Subject: [PATCH] Check if Coroutine await_suspend type returns the right type
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/include/clang/Sema/Sema.h | 2 +
clang/lib/Sema/SemaCoroutine.cpp | 75 +++--
clang/lib/Sema/SemaExprCXX.cpp| 84 +--
clang/test/SemaCXX/coroutines.cpp | 28 +--
5 files changed, 116 insertions(+), 75 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e97902564af08..f99170445c76b6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11701,7 +11701,7 @@ def err_coroutine_promise_new_requires_nothrow : Error<
def note_coroutine_promise_call_implicitly_required : Note<
"call to %0 implicitly required by coroutine function here">;
def err_await_suspend_invalid_return_type : Error<
- "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+ "return type of 'await_suspend' is required to be 'void' or 'bool' or
convertible to 'std::coroutine_handle<>' (have %0)"
>;
def note_await_ready_no_bool_conversion : Note<
"return type of 'await_ready' is required to be contextually convertible to
'bool'"
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 95ea5ebc7f1ac1..4976ff96b03d5b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7011,6 +7011,8 @@ class Sema final {
ExprResult BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
ArrayRef Args,
SourceLocation RParenLoc);
+ bool EvaluateBinaryTypeTrait(TypeTrait BTT, QualType LhsT, QualType RhsT,
+ SourceLocation KeyLoc);
/// ActOnArrayTypeTrait - Parsed one of the binary type trait support
/// pseudo-functions.
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 736632857efc36..fbe230737404fa 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -331,16 +331,12 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base,
SourceLocation Loc,
// coroutine.
static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
SourceLocation Loc) {
- if (RetType->isReferenceType())
-return nullptr;
+ assert(!RetType->isReferenceType() &&
+ "Should have diagnosed reference types.");
Type const *T = RetType.getTypePtr();
if (!T->isClassType() && !T->isStructureType())
return nullptr;
- // FIXME: Add convertability check to coroutine_handle<>. Possibly via
- // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
- // a private function in SemaExprCXX.cpp
-
ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", std::nullopt);
if (AddressExpr.isInvalid())
return nullptr;
@@ -358,6 +354,14 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr
*E,
return S.MaybeCreateExprWithCleanups(JustAddress);
}
+static bool isConvertibleToCoroutineHandle(Sema &S, QualType Ty,
+ SourceLocation Loc) {
+ QualType ErasedHandleType =
+ lookupCoroutineHandleType(S, S.Context.VoidTy, Loc);
+ return S.EvaluateBinaryTypeTrait(BTT_IsConvertible, Ty, ErasedHandleType,
+ Loc);
+}
+
/// Build calls to await_ready, await_suspend, and await_resume for a co_await
/// expression.
/// The generated AST tries to clean up temporary objects as early as
@@ -418,39 +422,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *Awa
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/85684
>From 08de54f02038795924a6e5fdbcf51a496fcedf56 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 18 Mar 2024 10:45:20 -0700
Subject: [PATCH] Check if Coroutine await_suspend type returns the right type
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/include/clang/Sema/Sema.h | 2 +
clang/lib/Sema/SemaCoroutine.cpp | 75 +++--
clang/lib/Sema/SemaExprCXX.cpp| 84 +--
clang/test/SemaCXX/coroutines.cpp | 28 +--
5 files changed, 116 insertions(+), 75 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e97902564af08..f99170445c76b6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11701,7 +11701,7 @@ def err_coroutine_promise_new_requires_nothrow : Error<
def note_coroutine_promise_call_implicitly_required : Note<
"call to %0 implicitly required by coroutine function here">;
def err_await_suspend_invalid_return_type : Error<
- "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+ "return type of 'await_suspend' is required to be 'void' or 'bool' or
convertible to 'std::coroutine_handle<>' (have %0)"
>;
def note_await_ready_no_bool_conversion : Note<
"return type of 'await_ready' is required to be contextually convertible to
'bool'"
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 95ea5ebc7f1ac1..4976ff96b03d5b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7011,6 +7011,8 @@ class Sema final {
ExprResult BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
ArrayRef Args,
SourceLocation RParenLoc);
+ bool EvaluateBinaryTypeTrait(TypeTrait BTT, QualType LhsT, QualType RhsT,
+ SourceLocation KeyLoc);
/// ActOnArrayTypeTrait - Parsed one of the binary type trait support
/// pseudo-functions.
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 736632857efc36..fbe230737404fa 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -331,16 +331,12 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base,
SourceLocation Loc,
// coroutine.
static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
SourceLocation Loc) {
- if (RetType->isReferenceType())
-return nullptr;
+ assert(!RetType->isReferenceType() &&
+ "Should have diagnosed reference types.");
Type const *T = RetType.getTypePtr();
if (!T->isClassType() && !T->isStructureType())
return nullptr;
- // FIXME: Add convertability check to coroutine_handle<>. Possibly via
- // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
- // a private function in SemaExprCXX.cpp
-
ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", std::nullopt);
if (AddressExpr.isInvalid())
return nullptr;
@@ -358,6 +354,14 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr
*E,
return S.MaybeCreateExprWithCleanups(JustAddress);
}
+static bool isConvertibleToCoroutineHandle(Sema &S, QualType Ty,
+ SourceLocation Loc) {
+ QualType ErasedHandleType =
+ lookupCoroutineHandleType(S, S.Context.VoidTy, Loc);
+ return S.EvaluateBinaryTypeTrait(BTT_IsConvertible, Ty, ErasedHandleType,
+ Loc);
+}
+
/// Build calls to await_ready, await_suspend, and await_resume for a co_await
/// expression.
/// The generated AST tries to clean up temporary objects as early as
@@ -418,39 +422,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *AwaitSuspend = [&]() -> CallExpr * {
+auto *SubExpr = BuildSubExpr(ACT::ACT_Suspend, "await_suspend",
CoroHandle);
+if (!SubExpr)
+ return nullptr;
+if (auto *E = dyn_cast(SubExpr)) {
+ // This happens when await_suspend return type is not trivially
+ // destructible. This doesn't happen for the permitted return types of
+ // such function. Diagnose it later.
+ return cast_or_null(E->getSubExpr());
+} else {
+ return cast_or_null(SubExpr);
+}
+ }();
+
if (!AwaitSuspend)
return Calls;
+
if (!AwaitSuspend->getType()->isDependentType()) {
+auto InvalidAwaitSuspendReturnType = [&](QualType RetType) {
+ // non-class prvalues always have cv-unqualified types
+ S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+ diag::err_await_suspend_invalid_
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
yuxuanchen1997 wrote: > > I am also ok with changing the patch to reject programs whose await_suspend > > doesn't strictly return std::coroutine_handle if upstream prefers. > > It should be good to do that since it makes the behavior more conforming. And > maybe it is better to do this a seperate patch. Especially if it can help > reduce the many non-semantical changes in SemaExprCXX.cpp. Would prefer separate patch. In `SemaExprCXX.cpp` there's `EvaluateBinaryTypeTraits` that I needed to expose as suggested by the comment in SemaCoroutine. Is there an alternative way to do type coercion/equivalence checks? https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
yuxuanchen1997 wrote: > I am confused. If we don't need to check whether a type can be converted to > std::coroutine_handle, why do we still need EvaluateBinaryTypeTraits or > similar things. I thought your previous comment was to show concern about exposing `EvaluateBinaryTypeTraits` contained a bunch of nonfunctional changes. Do you think it's good? https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type convertible to `std::coroutine_handle<>` (PR #85684)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/85684
>From 6887adae7500c4791a8620fa5b558e195e2c64cc Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 18 Mar 2024 10:45:20 -0700
Subject: [PATCH] Check if Coroutine await_suspend type returns the right type
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Sema/SemaCoroutine.cpp | 119 +-
clang/test/SemaCXX/coroutines.cpp | 28 -
3 files changed, 108 insertions(+), 41 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fc727cef9cd835..796b3d9d5e1190 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11707,7 +11707,7 @@ def err_coroutine_promise_new_requires_nothrow : Error<
def note_coroutine_promise_call_implicitly_required : Note<
"call to %0 implicitly required by coroutine function here">;
def err_await_suspend_invalid_return_type : Error<
- "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+ "return type of 'await_suspend' is required to be 'void', 'bool', or
'std::coroutine_handle' (have %0)"
>;
def note_await_ready_no_bool_conversion : Note<
"return type of 'await_ready' is required to be contextually convertible to
'bool'"
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 736632857efc36..2e81a83b62df51 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -137,12 +137,8 @@ static QualType lookupPromiseType(Sema &S, const
FunctionDecl *FD,
return PromiseType;
}
-/// Look up the std::coroutine_handle.
-static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
- SourceLocation Loc) {
- if (PromiseType.isNull())
-return QualType();
-
+static ClassTemplateDecl *lookupCoroutineHandleTemplate(Sema &S,
+SourceLocation Loc) {
NamespaceDecl *CoroNamespace = S.getStdNamespace();
assert(CoroNamespace && "Should already be diagnosed");
@@ -151,18 +147,32 @@ static QualType lookupCoroutineHandleType(Sema &S,
QualType PromiseType,
if (!S.LookupQualifiedName(Result, CoroNamespace)) {
S.Diag(Loc, diag::err_implied_coroutine_type_not_found)
<< "std::coroutine_handle";
-return QualType();
+return nullptr;
}
- ClassTemplateDecl *CoroHandle = Result.getAsSingle();
+ auto *CoroHandle = Result.getAsSingle();
+
if (!CoroHandle) {
Result.suppressDiagnostics();
// We found something weird. Complain about the first thing we found.
NamedDecl *Found = *Result.begin();
S.Diag(Found->getLocation(), diag::err_malformed_std_coroutine_handle);
-return QualType();
+return nullptr;
}
+ return CoroHandle;
+}
+
+/// Look up the std::coroutine_handle.
+static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
+ SourceLocation Loc) {
+ if (PromiseType.isNull())
+return QualType();
+
+ ClassTemplateDecl *CoroHandle = lookupCoroutineHandleTemplate(S, Loc);
+ if (!CoroHandle)
+return QualType();
+
// Form template argument list for coroutine_handle.
TemplateArgumentListInfo Args(Loc, Loc);
Args.addArgument(TemplateArgumentLoc(
@@ -331,16 +341,12 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base,
SourceLocation Loc,
// coroutine.
static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
SourceLocation Loc) {
- if (RetType->isReferenceType())
-return nullptr;
+ assert(!RetType->isReferenceType() &&
+ "Should have diagnosed reference types.");
Type const *T = RetType.getTypePtr();
if (!T->isClassType() && !T->isStructureType())
return nullptr;
- // FIXME: Add convertability check to coroutine_handle<>. Possibly via
- // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
- // a private function in SemaExprCXX.cpp
-
ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", std::nullopt);
if (AddressExpr.isInvalid())
return nullptr;
@@ -358,6 +364,30 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr
*E,
return S.MaybeCreateExprWithCleanups(JustAddress);
}
+static bool isSpecializationOfCoroutineHandle(Sema &S, QualType Ty,
+ SourceLocation Loc) {
+ auto *CoroutineHandleClassTemplateDecl =
+ lookupCoroutineHandleTemplate(S, Loc);
+
+ if (!CoroutineHandleClassTemplateDecl)
+return false;
+
+ auto *RecordTy = Ty->getAs();
+ if (!RecordTy)
+return false;
+
+ auto *D = RecordTy->getDecl();
+ if (!D)
+return false;
+
+ auto *SpecializationDecl = dyn_cast(D);
+ if (!SpecializationDecl)
+return false;
+
+ return CoroutineHandleClassTemplateDecl->getCanonicalDecl()
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
yuxuanchen1997 wrote: @ChuanqiXu9 I have updated the patch to check for `std::coroutine_handle` and does not rely on `SemaExprCXX` changes. Please let me know what you think. https://github.com/llvm/llvm-project/pull/85684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/85684
>From 4e10b2c4dfed3fb59ee03c716852f7fb45de081a Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 18 Mar 2024 10:45:20 -0700
Subject: [PATCH] Check if Coroutine await_suspend type returns the right type
---
clang/docs/ReleaseNotes.rst | 3 +
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Sema/SemaCoroutine.cpp | 119 +-
clang/test/SemaCXX/coroutines.cpp | 28 -
4 files changed, 111 insertions(+), 41 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d6e179ca9d6904..40d791a616b8a2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -339,6 +339,9 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- Clang now emits errors for coroutine `await_suspend` functions whose return
type is not
+ one of `void`, `bool`, or `std::coroutine_handle`.
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fc727cef9cd835..796b3d9d5e1190 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11707,7 +11707,7 @@ def err_coroutine_promise_new_requires_nothrow : Error<
def note_coroutine_promise_call_implicitly_required : Note<
"call to %0 implicitly required by coroutine function here">;
def err_await_suspend_invalid_return_type : Error<
- "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+ "return type of 'await_suspend' is required to be 'void', 'bool', or
'std::coroutine_handle' (have %0)"
>;
def note_await_ready_no_bool_conversion : Note<
"return type of 'await_ready' is required to be contextually convertible to
'bool'"
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 736632857efc36..2e81a83b62df51 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -137,12 +137,8 @@ static QualType lookupPromiseType(Sema &S, const
FunctionDecl *FD,
return PromiseType;
}
-/// Look up the std::coroutine_handle.
-static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
- SourceLocation Loc) {
- if (PromiseType.isNull())
-return QualType();
-
+static ClassTemplateDecl *lookupCoroutineHandleTemplate(Sema &S,
+SourceLocation Loc) {
NamespaceDecl *CoroNamespace = S.getStdNamespace();
assert(CoroNamespace && "Should already be diagnosed");
@@ -151,18 +147,32 @@ static QualType lookupCoroutineHandleType(Sema &S,
QualType PromiseType,
if (!S.LookupQualifiedName(Result, CoroNamespace)) {
S.Diag(Loc, diag::err_implied_coroutine_type_not_found)
<< "std::coroutine_handle";
-return QualType();
+return nullptr;
}
- ClassTemplateDecl *CoroHandle = Result.getAsSingle();
+ auto *CoroHandle = Result.getAsSingle();
+
if (!CoroHandle) {
Result.suppressDiagnostics();
// We found something weird. Complain about the first thing we found.
NamedDecl *Found = *Result.begin();
S.Diag(Found->getLocation(), diag::err_malformed_std_coroutine_handle);
-return QualType();
+return nullptr;
}
+ return CoroHandle;
+}
+
+/// Look up the std::coroutine_handle.
+static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
+ SourceLocation Loc) {
+ if (PromiseType.isNull())
+return QualType();
+
+ ClassTemplateDecl *CoroHandle = lookupCoroutineHandleTemplate(S, Loc);
+ if (!CoroHandle)
+return QualType();
+
// Form template argument list for coroutine_handle.
TemplateArgumentListInfo Args(Loc, Loc);
Args.addArgument(TemplateArgumentLoc(
@@ -331,16 +341,12 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base,
SourceLocation Loc,
// coroutine.
static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
SourceLocation Loc) {
- if (RetType->isReferenceType())
-return nullptr;
+ assert(!RetType->isReferenceType() &&
+ "Should have diagnosed reference types.");
Type const *T = RetType.getTypePtr();
if (!T->isClassType() && !T->isStructureType())
return nullptr;
- // FIXME: Add convertability check to coroutine_handle<>. Possibly via
- // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
- // a private function in SemaExprCXX.cpp
-
ExprResult AddressExpr = buildMemberCall(S, E, Loc, "address", std::nullopt);
if (AddressExpr.isInvalid())
return nullptr;
@@ -358,6 +364,30 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr
*E,
return S.MaybeCreateExprWithCleanups(JustA
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
@@ -418,39 +448,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *AwaitSuspend = [&]() -> CallExpr * {
+auto *SubExpr = BuildSubExpr(ACT::ACT_Suspend, "await_suspend",
CoroHandle);
+if (!SubExpr)
+ return nullptr;
+if (auto *E = dyn_cast(SubExpr)) {
+ // This happens when await_suspend return type is not trivially
+ // destructible. This doesn't happen for the permitted return types of
+ // such function. Diagnose it later.
+ return cast_or_null(E->getSubExpr());
+} else {
+ return cast_or_null(SubExpr);
+}
yuxuanchen1997 wrote:
This is necessary for the diagnostic to work. If the return type of
`await_suspend` is a type that has a user defined destructor we will get a
`CXXBindTemporaryExpr`. The original `cast_or_null` here was the reason it was
crashing.
https://github.com/llvm/llvm-project/pull/85684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
@@ -418,39 +448,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *AwaitSuspend = [&]() -> CallExpr * {
+auto *SubExpr = BuildSubExpr(ACT::ACT_Suspend, "await_suspend",
CoroHandle);
+if (!SubExpr)
+ return nullptr;
+if (auto *E = dyn_cast(SubExpr)) {
+ // This happens when await_suspend return type is not trivially
+ // destructible. This doesn't happen for the permitted return types of
+ // such function. Diagnose it later.
+ return cast_or_null(E->getSubExpr());
+} else {
+ return cast_or_null(SubExpr);
+}
yuxuanchen1997 wrote:
I was hoping to obtain the return type from the CallExpr.
https://github.com/llvm/llvm-project/pull/85684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/85684
>From b843c2f71a1a43cb897b557f783d60c6bf26f687 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Mon, 18 Mar 2024 10:45:20 -0700
Subject: [PATCH] Check if Coroutine await_suspend type returns the right type
---
clang/docs/ReleaseNotes.rst | 3 +
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Sema/SemaCoroutine.cpp | 119 +-
clang/test/SemaCXX/coroutines.cpp | 28 -
4 files changed, 111 insertions(+), 41 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d6e179ca9d6904..f7b44e5e65641c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,9 @@ C++ Specific Potentially Breaking Changes
- Clang now diagnoses function/variable templates that shadow their own
template parameters, e.g. ``template void T();``.
This error can be disabled via `-Wno-strict-primary-template-shadow` for
compatibility with previous versions of clang.
+- Clang now emits errors for coroutine `await_suspend` functions whose return
type is not
+ one of `void`, `bool`, or `std::coroutine_handle`.
+
ABI Changes in This Version
---
- Fixed Microsoft name mangling of implicitly defined variables used for thread
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fc727cef9cd835..796b3d9d5e1190 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11707,7 +11707,7 @@ def err_coroutine_promise_new_requires_nothrow : Error<
def note_coroutine_promise_call_implicitly_required : Note<
"call to %0 implicitly required by coroutine function here">;
def err_await_suspend_invalid_return_type : Error<
- "return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+ "return type of 'await_suspend' is required to be 'void', 'bool', or
'std::coroutine_handle' (have %0)"
>;
def note_await_ready_no_bool_conversion : Note<
"return type of 'await_ready' is required to be contextually convertible to
'bool'"
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 736632857efc36..2e81a83b62df51 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -137,12 +137,8 @@ static QualType lookupPromiseType(Sema &S, const
FunctionDecl *FD,
return PromiseType;
}
-/// Look up the std::coroutine_handle.
-static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
- SourceLocation Loc) {
- if (PromiseType.isNull())
-return QualType();
-
+static ClassTemplateDecl *lookupCoroutineHandleTemplate(Sema &S,
+SourceLocation Loc) {
NamespaceDecl *CoroNamespace = S.getStdNamespace();
assert(CoroNamespace && "Should already be diagnosed");
@@ -151,18 +147,32 @@ static QualType lookupCoroutineHandleType(Sema &S,
QualType PromiseType,
if (!S.LookupQualifiedName(Result, CoroNamespace)) {
S.Diag(Loc, diag::err_implied_coroutine_type_not_found)
<< "std::coroutine_handle";
-return QualType();
+return nullptr;
}
- ClassTemplateDecl *CoroHandle = Result.getAsSingle();
+ auto *CoroHandle = Result.getAsSingle();
+
if (!CoroHandle) {
Result.suppressDiagnostics();
// We found something weird. Complain about the first thing we found.
NamedDecl *Found = *Result.begin();
S.Diag(Found->getLocation(), diag::err_malformed_std_coroutine_handle);
-return QualType();
+return nullptr;
}
+ return CoroHandle;
+}
+
+/// Look up the std::coroutine_handle.
+static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
+ SourceLocation Loc) {
+ if (PromiseType.isNull())
+return QualType();
+
+ ClassTemplateDecl *CoroHandle = lookupCoroutineHandleTemplate(S, Loc);
+ if (!CoroHandle)
+return QualType();
+
// Form template argument list for coroutine_handle.
TemplateArgumentListInfo Args(Loc, Loc);
Args.addArgument(TemplateArgumentLoc(
@@ -331,16 +341,12 @@ static ExprResult buildMemberCall(Sema &S, Expr *Base,
SourceLocation Loc,
// coroutine.
static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
SourceLocation Loc) {
- if (RetType->isReferenceType())
-return nullptr;
+ assert(!RetType->isReferenceType() &&
+ "Should have diagnosed reference types.");
Type const *T = RetType.getTypePtr();
if (!T->isClassType() && !T->isStructureType())
return nullptr;
- // FIXME: Add convertability check to coroutine_handle<>. Possibly via
- // EvaluateBinaryTypeTrait(BTT_IsConvertible, ...) which is at the moment
- // a private function in SemaExprCXX.cpp
-
ExprResult AddressExpr = b
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
@@ -418,39 +448,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *AwaitSuspend = [&]() -> CallExpr * {
+auto *SubExpr = BuildSubExpr(ACT::ACT_Suspend, "await_suspend",
CoroHandle);
+if (!SubExpr)
+ return nullptr;
+if (auto *E = dyn_cast(SubExpr)) {
+ // This happens when await_suspend return type is not trivially
+ // destructible. This doesn't happen for the permitted return types of
+ // such function. Diagnose it later.
+ return cast_or_null(E->getSubExpr());
+} else {
+ return cast_or_null(SubExpr);
+}
yuxuanchen1997 wrote:
I feel that that's harder than finding the `CallExpr` and then `getReturnType`.
What's your suggestion?
https://github.com/llvm/llvm-project/pull/85684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Coroutines: Properly Check if `await_suspend` return type is a `std::coroutine_handle` (PR #85684)
@@ -418,39 +448,60 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
- CallExpr *AwaitSuspend = cast_or_null(
- BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
+ auto *AwaitSuspend = [&]() -> CallExpr * {
+auto *SubExpr = BuildSubExpr(ACT::ACT_Suspend, "await_suspend",
CoroHandle);
+if (!SubExpr)
+ return nullptr;
+if (auto *E = dyn_cast(SubExpr)) {
+ // This happens when await_suspend return type is not trivially
+ // destructible. This doesn't happen for the permitted return types of
+ // such function. Diagnose it later.
+ return cast_or_null(E->getSubExpr());
+} else {
+ return cast_or_null(SubExpr);
+}
yuxuanchen1997 wrote:
Currently the diagnostic will read:
```
error: return type of 'await_suspend' is required to be 'void', 'bool', or
'std::coroutine_handle' (have 'Status')
```
With what you suggested, I cannot print the "have 'Status'" part any more.
https://github.com/llvm/llvm-project/pull/85684
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
