Skip to content

Commit 763fd54

Browse files
committed
refactor(debug): refactor away from std::optional to avoid issues
std::optional for init data has several problems: * We need to work around a Clang bug. * It's hard to name the init data. init_data is the class name, and init_data_ is the variable name (with the trailing underscore to avoid name collisions). Refactor to use a boolean instead of std::optional. This might be more error-prone, but I think overall it's fine.
1 parent 26a2e7c commit 763fd54

File tree

2 files changed

+16
-27
lines changed

2 files changed

+16
-27
lines changed

src/quick-lint-js/debug/debug-server.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include <cstring>
1212
#include <map>
1313
#include <mongoose.h>
14-
#include <optional>
1514
#include <quick-lint-js/assert.h>
1615
#include <quick-lint-js/container/async-byte-queue.h>
1716
#include <quick-lint-js/container/byte-buffer.h>
@@ -134,7 +133,7 @@ void debug_server::start_server_thread() {
134133

135134
{
136135
lock_ptr<shared_state> state = this->state_.lock();
137-
state->init_data_.reset();
136+
state->initialized = false;
138137
state->init_error.clear();
139138
}
140139

@@ -151,9 +150,8 @@ void debug_server::stop_server_thread() {
151150

152151
result<void, debug_server_io_error> debug_server::wait_for_server_start() {
153152
lock_ptr<shared_state> state = this->state_.lock();
154-
this->initialized_.wait(state, [&] {
155-
return state->init_data_.has_value() || !state->init_error.empty();
156-
});
153+
this->initialized_.wait(
154+
state, [&] { return state->initialized || !state->init_error.empty(); });
157155

158156
if (!state->init_error.empty()) {
159157
return failed_result(debug_server_io_error{
@@ -173,7 +171,7 @@ std::string debug_server::url(std::string_view path) {
173171
std::string result;
174172
result.reserve(path.size() + 100);
175173
result += "http://"sv;
176-
result += this->state_.lock()->init_data_->actual_listen_address;
174+
result += this->state_.lock()->actual_listen_address;
177175
result += path;
178176
return result;
179177
}
@@ -184,7 +182,7 @@ std::string debug_server::websocket_url(std::string_view path) {
184182
std::string result;
185183
result.reserve(path.size() + 100);
186184
result += "ws://"sv;
187-
result += this->state_.lock()->init_data_->actual_listen_address;
185+
result += this->state_.lock()->actual_listen_address;
188186
result += path;
189187
return result;
190188
}
@@ -200,9 +198,9 @@ void debug_server::wake_up_server_thread() {
200198
}
201199

202200
void debug_server::wake_up_server_thread(lock_ptr<shared_state> &state) {
203-
if (state->init_data_.has_value()) {
201+
if (state->initialized) {
204202
char wakeup_signal[] = {0};
205-
::ssize_t rc = ::send(state->init_data_->wakeup_pipe, wakeup_signal,
203+
::ssize_t rc = ::send(state->wakeup_pipe, wakeup_signal,
206204
sizeof(wakeup_signal), /*flags=*/0);
207205
QLJS_ALWAYS_ASSERT(rc == 1);
208206
}
@@ -234,22 +232,22 @@ void debug_server::run_on_current_thread() {
234232
return;
235233
}
236234

237-
QLJS_ASSERT(!state->init_data_.has_value());
238-
state->init_data_.emplace();
235+
QLJS_ASSERT(!state->initialized);
239236

240237
// server_connection->loc is initialized synchronously, so we should be able
241238
// to use c->loc now.
242-
std::string &address = state->init_data_->actual_listen_address;
239+
std::string &address = state->actual_listen_address;
243240
address.resize(100);
244241
::mg_straddr(&server_connection->loc, address.data(), address.size());
245242
address.resize(std::strlen(address.c_str()));
246243

247-
state->init_data_->wakeup_pipe = ::mg_mkpipe(
244+
state->wakeup_pipe = ::mg_mkpipe(
248245
mgr.get(), mongoose_callback<&debug_server::wakeup_pipe_callback>(),
249246
this,
250247
/*udp=*/false);
251-
QLJS_ALWAYS_ASSERT(state->init_data_->wakeup_pipe != -1);
248+
QLJS_ALWAYS_ASSERT(state->wakeup_pipe != -1);
252249

250+
state->initialized = true;
253251
this->initialized_.notify_all();
254252
}
255253

src/quick-lint-js/debug/debug-server.h

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include <atomic>
1212
#include <memory>
1313
#include <mongoose.h>
14-
#include <optional>
1514
#include <quick-lint-js/container/result.h>
1615
#include <quick-lint-js/container/vector-profiler.h>
1716
#include <quick-lint-js/port/thread.h>
@@ -76,22 +75,14 @@ class debug_server {
7675
void http_server_callback(::mg_connection *c, int ev, void *ev_data) noexcept;
7776
void wakeup_pipe_callback(::mg_connection *c, int ev, void *ev_data) noexcept;
7877

79-
struct init_data {
80-
// HACK(strager): Clang 11 with libstdc++ 12 requires a user-declared (not
81-
// default) constructor. Otherwise, std::is_constructible_v<init_data>
82-
// returns false, causing std::optional<init_data>::emplace() to not
83-
// compile.
84-
explicit init_data() {}
85-
86-
std::string actual_listen_address;
87-
int wakeup_pipe = -1;
88-
};
89-
9078
struct shared_state {
9179
std::string requested_listen_address = "http://localhost:0";
9280

9381
// Written to by the server thread. Read by other threads.
94-
std::optional<init_data> init_data_;
82+
std::string actual_listen_address;
83+
int wakeup_pipe = -1;
84+
bool initialized = false; // When true, actual_listen_address and
85+
// wakeup_pipe are valid.
9586
std::string init_error;
9687
};
9788

0 commit comments

Comments
 (0)