-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AST] Teach TextNodeDumper to print the "implicit" bit for coroutine AST nodes #77311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesFull diff: https://github.com/llvm/llvm-project/pull/77311.diff 3 Files Affected:
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index 2f4ed082a0c7ad..dd9ed56fc584a7 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -252,6 +252,8 @@ class TextNodeDumper
void VisitGotoStmt(const GotoStmt *Node);
void VisitCaseStmt(const CaseStmt *Node);
void VisitReturnStmt(const ReturnStmt *Node);
+ void VisitCoawaitExpr(const CoawaitExpr* Node);
+ void VisitCoreturnStmt(const CoreturnStmt *Node);
void VisitCompoundStmt(const CompoundStmt *Node);
void VisitConstantExpr(const ConstantExpr *Node);
void VisitCallExpr(const CallExpr *Node);
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index e8274fcd5cfe9c..d53688ea862e34 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1094,6 +1094,16 @@ void clang::TextNodeDumper::VisitReturnStmt(const ReturnStmt *Node) {
}
}
+void clang::TextNodeDumper::VisitCoawaitExpr(const CoawaitExpr* Node) {
+ if (Node->isImplicit())
+ OS << " implicit";
+}
+
+void clang::TextNodeDumper::VisitCoreturnStmt(const CoreturnStmt *Node) {
+ if (Node->isImplicit())
+ OS << " implicit";
+}
+
void TextNodeDumper::VisitConstantExpr(const ConstantExpr *Node) {
if (Node->hasAPValueResult())
AddChild("value",
diff --git a/clang/test/AST/ast-dump-coroutine.cpp b/clang/test/AST/ast-dump-coroutine.cpp
new file mode 100644
index 00000000000000..f760809f53689a
--- /dev/null
+++ b/clang/test/AST/ast-dump-coroutine.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -std=c++20 \
+// RUN: -fsyntax-only -ast-dump -ast-dump-filter test | FileCheck %s
+
+#include "Inputs/std-coroutine.h"
+
+using namespace std;
+
+struct Task {
+ struct promise_type {
+ std::suspend_always initial_suspend() { return {}; }
+ Task get_return_object() {
+ return std::coroutine_handle<promise_type>::from_promise(*this);
+ }
+ std::suspend_always final_suspend() noexcept { return {}; }
+ std::suspend_always return_void() { return {}; }
+ void unhandled_exception() {}
+
+ auto await_transform(int s) {
+ struct awaiter {
+ promise_type *promise;
+ bool await_ready() { return true; }
+ int await_resume() { return 1; }
+ void await_suspend(std::coroutine_handle<>) {}
+ };
+
+ return awaiter{this};
+ }
+ };
+
+ Task(std::coroutine_handle<promise_type> promise);
+
+ std::coroutine_handle<promise_type> handle;
+};
+
+// Verify the implicit AST nodes for coroutines.
+Task test() {
+ co_await 1;
+}
+// CHECK: |-DeclStmt {{.*}}
+// CHECK-NEXT: | `-VarDecl {{.*}} implicit used __promise
+// CHECK-NEXT: | `-CXXConstructExpr {{.*}}
+// CHECK-NEXT: |-ExprWithCleanups {{.*}} 'void'
+// CHECK-NEXT: | `-CoawaitExpr {{.*}} 'void' implicit
+// ...
+// FIXME: the CoreturnStmt should be marked as implicit
+// CHECK: CoreturnStmt {{.*}} <col:6>
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, other nodes also do this.
But please take a look at suggestions for improving the test before landing.
// CHECK-NEXT: | `-VarDecl {{.*}} implicit used __promise | ||
// CHECK-NEXT: | `-CXXConstructExpr {{.*}} | ||
// CHECK-NEXT: |-ExprWithCleanups {{.*}} 'void' | ||
// CHECK-NEXT: | `-CoawaitExpr {{.*}} 'void' implicit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this node implicit for initial suspend, for a call to await_transform
or both? It would be useful to clarify by matching some child nodes or the dump of CoawaitExpr
itself.
Also, could you add matching for both implicit and non-implicit co_await
and co_return
instances to show that the bit does not incidentally gets set/unset all the time?
Fixing CoreturnStmt
in a follow-up does make a lot of sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this node implicit for initial suspend, for a call to await_transform or both?
This implicit node is just for initial suspend, it is the InitSuspend
substmt of the CoroutineBodyStmt
.
Added some more tests and matches for the written co_await
and co_return
statements.
No description provided.