Skip to content

Commit b00c91a

Browse files
committed
[MemoryMapper] use thread-local variant for thread-local tests
Clarify this code does not have access to a TaskDispatcher, and thus is not permitted use of threads, by using a more efficient single-threaded monostate variant, instead of making it appear that this test could potentially be a cause of thread-safety concerns (notably deadlock).
1 parent 9abbec6 commit b00c91a

File tree

2 files changed

+33
-27
lines changed

2 files changed

+33
-27
lines changed

llvm/unittests/ExecutionEngine/Orc/MemoryMapperTest.cpp

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,40 +12,46 @@
1212
#include "llvm/Testing/Support/Error.h"
1313
#include "gtest/gtest.h"
1414

15+
#include <variant>
16+
1517
using namespace llvm;
1618
using namespace llvm::orc;
1719
using namespace llvm::orc::shared;
1820

1921
namespace {
2022

2123
Expected<ExecutorAddrRange> reserve(MemoryMapper &M, size_t NumBytes) {
22-
std::promise<MSVCPExpected<ExecutorAddrRange>> P;
23-
auto F = P.get_future();
24-
M.reserve(NumBytes, [&](auto R) { P.set_value(std::move(R)); });
25-
return F.get();
24+
std::variant<std::monostate, Expected<ExecutorAddrRange>> Result;
25+
M.reserve(NumBytes, [&](auto R) { Result = std::move(R); });
26+
assert(!std::holds_alternative<std::monostate>(Result) &&
27+
"MemoryMapper operations should complete synchronously in tests");
28+
return std::move(std::get<Expected<ExecutorAddrRange>>(Result));
2629
}
2730

2831
Expected<ExecutorAddr> initialize(MemoryMapper &M,
2932
MemoryMapper::AllocInfo &AI) {
30-
std::promise<MSVCPExpected<ExecutorAddr>> P;
31-
auto F = P.get_future();
32-
M.initialize(AI, [&](auto R) { P.set_value(std::move(R)); });
33-
return F.get();
33+
std::variant<std::monostate, Expected<ExecutorAddr>> Result;
34+
M.initialize(AI, [&](auto R) { Result = std::move(R); });
35+
assert(!std::holds_alternative<std::monostate>(Result) &&
36+
"MemoryMapper operations should complete synchronously in tests");
37+
return std::move(std::get<Expected<ExecutorAddr>>(Result));
3438
}
3539

3640
Error deinitialize(MemoryMapper &M,
3741
const std::vector<ExecutorAddr> &Allocations) {
38-
std::promise<MSVCPError> P;
39-
auto F = P.get_future();
40-
M.deinitialize(Allocations, [&](auto R) { P.set_value(std::move(R)); });
41-
return F.get();
42+
std::variant<std::monostate, Error> Result;
43+
M.deinitialize(Allocations, [&](auto R) { Result = std::move(R); });
44+
assert(!std::holds_alternative<std::monostate>(Result) &&
45+
"MemoryMapper operations should complete synchronously in tests");
46+
return std::move(std::get<Error>(Result));
4247
}
4348

4449
Error release(MemoryMapper &M, const std::vector<ExecutorAddr> &Reservations) {
45-
std::promise<MSVCPError> P;
46-
auto F = P.get_future();
47-
M.release(Reservations, [&](auto R) { P.set_value(std::move(R)); });
48-
return F.get();
50+
std::variant<std::monostate, Error> Result;
51+
M.release(Reservations, [&](auto R) { Result = std::move(R); });
52+
assert(!std::holds_alternative<std::monostate>(Result) &&
53+
"MemoryMapper operations should complete synchronously in tests");
54+
return std::move(std::get<Error>(Result));
4955
}
5056

5157
// A basic function to be used as both initializer/deinitializer

llvm/unittests/ExecutionEngine/Orc/WrapperFunctionUtilsTest.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#include "llvm/Testing/Support/Error.h"
1212
#include "gtest/gtest.h"
1313

14-
#include <future>
14+
#include <variant>
1515

1616
using namespace llvm;
1717
using namespace llvm::orc;
@@ -114,29 +114,29 @@ static void voidNoopAsync(unique_function<void(SPSEmpty)> SendResult) {
114114

115115
static WrapperFunctionResult voidNoopAsyncWrapper(const char *ArgData,
116116
size_t ArgSize) {
117-
std::promise<WrapperFunctionResult> RP;
118-
auto RF = RP.get_future();
117+
std::variant<std::monostate, WrapperFunctionResult> Result;
119118

120119
WrapperFunction<void()>::handleAsync(
121-
ArgData, ArgSize,
122-
[&](WrapperFunctionResult R) { RP.set_value(std::move(R)); },
120+
ArgData, ArgSize, [&](WrapperFunctionResult R) { Result = std::move(R); },
123121
voidNoopAsync);
124122

125-
return RF.get();
123+
assert(!std::holds_alternative<std::monostate>(Result) &&
124+
"handleAsync should complete synchronously in tests");
125+
return std::move(std::get<WrapperFunctionResult>(Result));
126126
}
127127

128128
static WrapperFunctionResult addAsyncWrapper(const char *ArgData,
129129
size_t ArgSize) {
130-
std::promise<WrapperFunctionResult> RP;
131-
auto RF = RP.get_future();
130+
std::variant<std::monostate, WrapperFunctionResult> Result;
132131

133132
WrapperFunction<int32_t(int32_t, int32_t)>::handleAsync(
134-
ArgData, ArgSize,
135-
[&](WrapperFunctionResult R) { RP.set_value(std::move(R)); },
133+
ArgData, ArgSize, [&](WrapperFunctionResult R) { Result = std::move(R); },
136134
[](unique_function<void(int32_t)> SendResult, int32_t X, int32_t Y) {
137135
SendResult(X + Y);
138136
});
139-
return RF.get();
137+
assert(!std::holds_alternative<std::monostate>(Result) &&
138+
"handleAsync should complete synchronously in tests");
139+
return std::move(std::get<WrapperFunctionResult>(Result));
140140
}
141141

142142
TEST(WrapperFunctionUtilsTest, WrapperFunctionCallAndHandleAsyncVoid) {

0 commit comments

Comments
 (0)