Skip to content

Commit e3ce9bb

Browse files
committed
[clang-tidy][performance-unnecessary-value-param] Avoid in coroutines
Summary: Replacing by-value parameters with passing by-reference is not safe for coroutines because the caller may be executed in parallel with the callee, which increases the chances of resulting in dangling references and hard-to-find crashes. Test Plan: check-clang-tools
1 parent f133eae commit e3ce9bb

File tree

2 files changed

+82
-0
lines changed

2 files changed

+82
-0
lines changed

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
6565
TK_AsIs,
6666
functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
6767
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
68+
unless(hasBody(coroutineBodyStmt())),
6869
has(typeLoc(forEach(ExpensiveValueParamDecl))),
6970
decl().bind("functionDecl"))),
7071
this);
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// RUN: %check_clang_tidy -std=c++20-or-later %s performance-unnecessary-value-param %t -- -fix-errors
2+
3+
namespace std {
4+
5+
template <typename R, typename...> struct coroutine_traits {
6+
using promise_type = typename R::promise_type;
7+
};
8+
9+
template <typename Promise = void> struct coroutine_handle;
10+
11+
template <> struct coroutine_handle<void> {
12+
static coroutine_handle from_address(void *addr) noexcept {
13+
coroutine_handle me;
14+
me.ptr = addr;
15+
return me;
16+
}
17+
void operator()() { resume(); }
18+
void *address() const noexcept { return ptr; }
19+
void resume() const { }
20+
void destroy() const { }
21+
bool done() const { return true; }
22+
coroutine_handle &operator=(decltype(nullptr)) {
23+
ptr = nullptr;
24+
return *this;
25+
}
26+
coroutine_handle(decltype(nullptr)) : ptr(nullptr) {}
27+
coroutine_handle() : ptr(nullptr) {}
28+
// void reset() { ptr = nullptr; } // add to P0057?
29+
explicit operator bool() const { return ptr; }
30+
31+
protected:
32+
void *ptr;
33+
};
34+
35+
template <typename Promise> struct coroutine_handle : coroutine_handle<> {
36+
using coroutine_handle<>::operator=;
37+
38+
static coroutine_handle from_address(void *addr) noexcept {
39+
coroutine_handle me;
40+
me.ptr = addr;
41+
return me;
42+
}
43+
44+
Promise &promise() const {
45+
return *reinterpret_cast<Promise *>(
46+
__builtin_coro_promise(ptr, alignof(Promise), false));
47+
}
48+
static coroutine_handle from_promise(Promise &promise) {
49+
coroutine_handle p;
50+
p.ptr = __builtin_coro_promise(&promise, alignof(Promise), true);
51+
return p;
52+
}
53+
};
54+
55+
struct suspend_always {
56+
bool await_ready() noexcept { return false; }
57+
void await_suspend(std::coroutine_handle<>) noexcept {}
58+
void await_resume() noexcept {}
59+
};
60+
61+
} // namespace std
62+
63+
struct ReturnObject {
64+
struct promise_type {
65+
ReturnObject get_return_object() { return {}; }
66+
std::suspend_always initial_suspend() { return {}; }
67+
std::suspend_always final_suspend() noexcept { return {}; }
68+
void unhandled_exception() {}
69+
std::suspend_always yield_value(int value) { return {}; }
70+
};
71+
};
72+
73+
struct A {
74+
A(const A&);
75+
};
76+
77+
ReturnObject evaluateModels(const A timeMachineId) {
78+
// No change for non-coroutine function expected because it is not safe.
79+
// CHECK-FIXES: ReturnObject evaluateModels(const A timeMachineId) {
80+
co_return;
81+
}

0 commit comments

Comments
 (0)