Skip to content

[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

Merged
merged 3 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/include/clang/AST/TextNodeDumper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions clang/lib/AST/TextNodeDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
69 changes: 69 additions & 0 deletions clang/test/AST/ast-dump-coroutine.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// 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;
};

Task test() {
co_await 1;
// Writen souce code, verify no implicit bit for the co_await expr.
// CHECK: CompoundStmt {{.*}}
// CHECK-NEXT: | `-ExprWithCleanups {{.*}} 'int'
// CHECK-NEXT: | `-CoawaitExpr {{.*}} 'int'{{$}}
// CHECK-NEXT: | |-IntegerLiteral {{.*}} <col:12> 'int' 1
// CHECK-NEXT: | |-MaterializeTemporaryExpr {{.*}} 'awaiter'
// CHECK-NEXT: | | `-CXXMemberCallExpr {{.*}} 'awaiter'
// CHECK-NEXT: | | |-MemberExpr {{.*}} .await_transform
}
// Verify the implicit AST nodes for coroutines.
// CHECK: |-DeclStmt {{.*}}
// CHECK-NEXT: | `-VarDecl {{.*}} implicit used __promise
// CHECK-NEXT: | `-CXXConstructExpr {{.*}}
// CHECK-NEXT: |-ExprWithCleanups {{.*}} 'void'
// CHECK-NEXT: | `-CoawaitExpr {{.*}} 'void' implicit
Copy link
Contributor

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!

Copy link
Collaborator Author

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.

// CHECK-NEXT: |-CXXMemberCallExpr {{.*}} 'std::suspend_always'
// CHECK-NEXT: | | `-MemberExpr {{.*}} .initial_suspend
// ...
// FIXME: the CoreturnStmt should be marked as implicit
// CHECK: CoreturnStmt {{.*}} <col:6>{{$}}

Task test2() {
// Writen souce code, verify no implicit bit for the co_return expr.
// CHECK: CompoundStmt {{.*}}
// CHECK-NEXT: | `-CoreturnStmt {{.*}} <line:{{.*}}:{{.*}}>{{$}}
co_return;
}
// Verify the implicit AST nodes for coroutines.
// CHECK: |-DeclStmt {{.*}}
// CHECK-NEXT: | `-VarDecl {{.*}} implicit used __promise
// ...
// FIXME: the CoreturnStmt should be marked as implicit
// CHECK: CoreturnStmt {{.*}} <col:6>{{$}}