Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 15c49b9

Browse filesBrowse files
ChuanqiXu9ChuanqiXu9
committed
[Coroutines] [CodeGen] Don't change AST in CodeGen/Coroutines
The root source of other odd bugs. We performed a hack in CodeGen/Coroutines. But we didn't recognize that the CodeGen is a consumer of AST. The CodeGen shouldn't change AST in any ways. It'll break the assumption about the ASTConsumer in Clang's framework, which may break any other clang-based tools which depends on multiple consumers to work together. The fix here is simple. But I am not super happy about the test. It is too specific and verbose. We can remove this if we can get the signature of the AST in ASTContext.
1 parent 44b9f5e commit 15c49b9
Copy full SHA for 15c49b9

File tree

Expand file treeCollapse file tree

3 files changed

+208
-1
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+208
-1
lines changed

‎clang/lib/CodeGen/CGCoroutine.cpp

Copy file name to clipboardExpand all lines: clang/lib/CodeGen/CGCoroutine.cpp
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,9 +942,16 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
942942
if (Stmt *Ret = S.getReturnStmt()) {
943943
// Since we already emitted the return value above, so we shouldn't
944944
// emit it again here.
945-
if (GroManager.DirectEmit)
945+
Expr *PreviousRetValue = nullptr;
946+
if (GroManager.DirectEmit) {
947+
PreviousRetValue = cast<ReturnStmt>(Ret)->getRetValue();
946948
cast<ReturnStmt>(Ret)->setRetValue(nullptr);
949+
}
947950
EmitStmt(Ret);
951+
// Set the return value back. The code generator, as the AST **Consumer**,
952+
// shouldn't change the AST.
953+
if (PreviousRetValue)
954+
cast<ReturnStmt>(Ret)->setRetValue(PreviousRetValue);
948955
}
949956

950957
// LLVM require the frontend to mark the coroutine.

‎clang/unittests/Frontend/CMakeLists.txt

Copy file name to clipboardExpand all lines: clang/unittests/Frontend/CMakeLists.txt
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_clang_unittest(FrontendTests
1010
FixedPointString.cpp
1111
FrontendActionTest.cpp
1212
CodeGenActionTest.cpp
13+
NoAlterCodeGenActionTest.cpp
1314
ParsedSourceLocationTest.cpp
1415
PCHPreambleTest.cpp
1516
ReparseWorkingDirTest.cpp
@@ -27,4 +28,5 @@ clang_target_link_libraries(FrontendTests
2728
clangCodeGen
2829
clangFrontendTool
2930
clangSerialization
31+
clangTooling
3032
)

‎clang/unittests/Frontend/NoAlterCodeGenActionTest.cpp

Copy file name to clipboard
+198Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
//===- unittests/Frontend/NoAlterCodeGenActionTest.cpp --------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// Unit tests for CodeGenAction may not alter the AST.
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "clang/AST/ASTConsumer.h"
14+
#include "clang/AST/RecursiveASTVisitor.h"
15+
#include "clang/Basic/LangStandard.h"
16+
#include "clang/CodeGen/BackendUtil.h"
17+
#include "clang/CodeGen/CodeGenAction.h"
18+
#include "clang/Frontend/CompilerInstance.h"
19+
#include "clang/Frontend/MultiplexConsumer.h"
20+
#include "clang/Lex/PreprocessorOptions.h"
21+
#include "clang/Tooling/Tooling.h"
22+
#include "llvm/Support/FormatVariadic.h"
23+
#include "llvm/Support/VirtualFileSystem.h"
24+
#include "gtest/gtest.h"
25+
26+
using namespace llvm;
27+
using namespace clang;
28+
using namespace clang::frontend;
29+
using namespace clang::tooling;
30+
31+
namespace {
32+
33+
class ASTChecker : public RecursiveASTVisitor<ASTChecker> {
34+
public:
35+
ASTContext &Ctx;
36+
ASTChecker(ASTContext &Ctx) : Ctx(Ctx) {}
37+
bool VisitReturnStmt(ReturnStmt *RS) {
38+
EXPECT_TRUE(RS->getRetValue());
39+
return true;
40+
}
41+
42+
bool VisitCoroutineBodyStmt(CoroutineBodyStmt *CS) {
43+
return VisitReturnStmt(cast<ReturnStmt>(CS->getReturnStmt()));
44+
}
45+
};
46+
47+
class ASTCheckerConsumer : public ASTConsumer {
48+
public:
49+
void HandleTranslationUnit(ASTContext &Ctx) override {
50+
ASTChecker Checker(Ctx);
51+
Checker.TraverseAST(Ctx);
52+
}
53+
};
54+
55+
class TestCodeGenAction : public EmitLLVMAction {
56+
public:
57+
using Base = EmitLLVMAction;
58+
TestCodeGenAction(llvm::LLVMContext *_VMContext = nullptr)
59+
: EmitLLVMAction(_VMContext) {}
60+
61+
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
62+
StringRef InFile) override {
63+
std::vector<std::unique_ptr<ASTConsumer>> Consumers;
64+
Consumers.push_back(std::make_unique<ASTCheckerConsumer>());
65+
Consumers.push_back(Base::CreateASTConsumer(CI, InFile));
66+
return std::make_unique<MultiplexConsumer>(std::move(Consumers));
67+
}
68+
};
69+
70+
const char *test_contents = R"cpp(
71+
72+
namespace std {
73+
74+
template <typename R, typename...> struct coroutine_traits {
75+
using promise_type = typename R::promise_type;
76+
};
77+
78+
template <typename Promise = void> struct coroutine_handle;
79+
80+
template <> struct coroutine_handle<void> {
81+
static coroutine_handle from_address(void *addr) noexcept;
82+
void operator()() { resume(); }
83+
void *address() const noexcept;
84+
void resume() const { __builtin_coro_resume(ptr); }
85+
void destroy() const { __builtin_coro_destroy(ptr); }
86+
bool done() const;
87+
coroutine_handle &operator=(decltype(nullptr));
88+
coroutine_handle(decltype(nullptr)) : ptr(nullptr) {}
89+
coroutine_handle() : ptr(nullptr) {}
90+
// void reset() { ptr = nullptr; } // add to P0057?
91+
explicit operator bool() const;
92+
93+
protected:
94+
void *ptr;
95+
};
96+
97+
template <typename Promise> struct coroutine_handle : coroutine_handle<> {
98+
using coroutine_handle<>::operator=;
99+
100+
static coroutine_handle from_address(void *addr) noexcept;
101+
102+
Promise &promise() const;
103+
static coroutine_handle from_promise(Promise &promise);
104+
};
105+
106+
template <typename _PromiseT>
107+
bool operator==(coroutine_handle<_PromiseT> const &_Left,
108+
coroutine_handle<_PromiseT> const &_Right) noexcept {
109+
return _Left.address() == _Right.address();
110+
}
111+
112+
template <typename _PromiseT>
113+
bool operator!=(coroutine_handle<_PromiseT> const &_Left,
114+
coroutine_handle<_PromiseT> const &_Right) noexcept {
115+
return !(_Left == _Right);
116+
}
117+
118+
struct noop_coroutine_promise {};
119+
120+
template <>
121+
struct coroutine_handle<noop_coroutine_promise> {
122+
operator coroutine_handle<>() const noexcept;
123+
124+
constexpr explicit operator bool() const noexcept { return true; }
125+
constexpr bool done() const noexcept { return false; }
126+
127+
constexpr void operator()() const noexcept {}
128+
constexpr void resume() const noexcept {}
129+
constexpr void destroy() const noexcept {}
130+
131+
noop_coroutine_promise &promise() const noexcept {
132+
return *static_cast<noop_coroutine_promise *>(
133+
__builtin_coro_promise(this->__handle_, alignof(noop_coroutine_promise), false));
134+
}
135+
136+
constexpr void *address() const noexcept { return __handle_; }
137+
138+
private:
139+
friend coroutine_handle<noop_coroutine_promise> noop_coroutine() noexcept;
140+
141+
coroutine_handle() noexcept {
142+
this->__handle_ = __builtin_coro_noop();
143+
}
144+
145+
void *__handle_ = nullptr;
146+
};
147+
148+
using noop_coroutine_handle = coroutine_handle<noop_coroutine_promise>;
149+
150+
inline noop_coroutine_handle noop_coroutine() noexcept { return noop_coroutine_handle(); }
151+
152+
struct suspend_always {
153+
bool await_ready() noexcept { return false; }
154+
void await_suspend(coroutine_handle<>) noexcept {}
155+
void await_resume() noexcept {}
156+
};
157+
struct suspend_never {
158+
bool await_ready() noexcept { return true; }
159+
void await_suspend(coroutine_handle<>) noexcept {}
160+
void await_resume() noexcept {}
161+
};
162+
163+
} // namespace std
164+
165+
using namespace std;
166+
167+
class invoker {
168+
public:
169+
class invoker_promise {
170+
public:
171+
invoker get_return_object() { return invoker{}; }
172+
auto initial_suspend() { return suspend_always{}; }
173+
auto final_suspend() noexcept { return suspend_always{}; }
174+
void return_void() {}
175+
void unhandled_exception() {}
176+
};
177+
using promise_type = invoker_promise;
178+
invoker() {}
179+
invoker(const invoker &) = delete;
180+
invoker &operator=(const invoker &) = delete;
181+
invoker(invoker &&) = delete;
182+
invoker &operator=(invoker &&) = delete;
183+
};
184+
185+
invoker g() {
186+
co_return;
187+
}
188+
189+
)cpp";
190+
191+
TEST(CodeGenTest, TestNonAlterTest) {
192+
EXPECT_TRUE(runToolOnCodeWithArgs(std::make_unique<TestCodeGenAction>(),
193+
test_contents,
194+
{
195+
"-std=c++20",
196+
}));
197+
}
198+
} // namespace

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.