From 8e291da9249ec8375165196d50756bc28289c94d Mon Sep 17 00:00:00 2001 From: Maxime Arthaud Date: Mon, 27 Nov 2023 09:28:48 -0800 Subject: [PATCH] Add a macro to mark unused variables Summary: When compiling with both `-Wunused-variable` and `-DNDEBUG`, we are getting errors due to unused variables. This is because we have variables that are created and only used in assertions. Let's introduce a `SPARTA_UNUSED` macro that does a static cast to void, which should get optimized away. Let's also use this to introduce `SPARTA_ASSERT` which is just an alias for the C assert, for now. Reviewed By: GerbenJavado Differential Revision: D51588387 fbshipit-source-id: f8ccc03bace8112f6b26fdad850c5fad47f388c4 --- include/sparta/Exceptions.h | 11 +++++++++++ include/sparta/IntervalDomain.h | 16 ++++++++-------- include/sparta/ThreadPool.h | 5 +++-- include/sparta/WorkQueue.h | 30 +++++++++++++++++------------- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/include/sparta/Exceptions.h b/include/sparta/Exceptions.h index f8428c8..cab3b85 100644 --- a/include/sparta/Exceptions.h +++ b/include/sparta/Exceptions.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include @@ -69,6 +70,16 @@ using operation_name = } \ while (0) +/* + * An assert-like macro that terminates the program. + */ +#define SPARTA_ASSERT(m) assert(m) + +/* + * Indicates that the given variable is unused, to prevent compiler warnings. + */ +#define SPARTA_UNUSED_VARIABLE(v) static_cast(v) + namespace sparta::exception_impl { template diff --git a/include/sparta/IntervalDomain.h b/include/sparta/IntervalDomain.h index ae47565..52bab15 100644 --- a/include/sparta/IntervalDomain.h +++ b/include/sparta/IntervalDomain.h @@ -7,13 +7,13 @@ #pragma once -#include #include #include #include #include +#include namespace sparta { @@ -55,21 +55,21 @@ class IntervalDomain final : public AbstractDomain> { /* [lb, ub] */ static IntervalDomain finite(Num lb, Num ub) { - assert(MIN < lb && "interval not bounded below."); - assert(lb <= ub && "interval inverted."); - assert(ub < MAX && "interval not bounded above."); + SPARTA_ASSERT(MIN < lb && "interval not bounded below."); + SPARTA_ASSERT(lb <= ub && "interval inverted."); + SPARTA_ASSERT(ub < MAX && "interval not bounded above."); return {lb, ub}; } /* [lb, +inf] */ static IntervalDomain bounded_below(Num lb) { - assert(MIN < lb && "interval underflow"); + SPARTA_ASSERT(MIN < lb && "interval underflow"); return {lb, MAX}; } /* [-inf, ub] */ static IntervalDomain bounded_above(Num ub) { - assert(ub < MAX && "interval overflow."); + SPARTA_ASSERT(ub < MAX && "interval overflow."); return {MIN, ub}; } @@ -87,7 +87,7 @@ class IntervalDomain final : public AbstractDomain> { /* Inclusive lower-bound of the interval, assuming interval is not bottom. */ Num lower_bound() const { - assert(!is_bottom()); + SPARTA_ASSERT(!is_bottom()); return m_lb; } @@ -96,7 +96,7 @@ class IntervalDomain final : public AbstractDomain> { * Guaranteed to be greater than or equal to lower_bound(). */ Num upper_bound() const { - assert(!is_bottom()); + SPARTA_ASSERT(!is_bottom()); return m_ub; } diff --git a/include/sparta/ThreadPool.h b/include/sparta/ThreadPool.h index 95bdec5..d6d7cdf 100644 --- a/include/sparta/ThreadPool.h +++ b/include/sparta/ThreadPool.h @@ -7,13 +7,14 @@ #pragma once -#include #include #include #include #include #include +#include + namespace sparta { // The AsyncRunner provides a way to run work on a separate thread. The main @@ -79,7 +80,7 @@ class ThreadPool : public AsyncRunner { void run_async_bound(std::function bound_f) override { { std::lock_guard lock(m_mutex); - assert(!m_joining); + SPARTA_ASSERT(!m_joining); if (m_waiting == 0) { m_threads.push_back(create_thread(std::move(bound_f))); return; diff --git a/include/sparta/WorkQueue.h b/include/sparta/WorkQueue.h index c15179f..542ced6 100644 --- a/include/sparta/WorkQueue.h +++ b/include/sparta/WorkQueue.h @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -21,6 +20,7 @@ #include #include +#include #include namespace sparta { @@ -132,7 +132,7 @@ class WorkerState final { * `WorkQueue::add_item()` as the latter is not thread-safe. */ void push_task(Input task) { - assert(m_can_push_task); + SPARTA_ASSERT(m_can_push_task); auto* node = new Node{std::move(task), m_additional_tasks.load()}; do { if (node->prev == nullptr) { @@ -152,7 +152,8 @@ class WorkerState final { void set_running(bool running) { if (m_running && !running) { auto num = m_state_counters->num_running.fetch_sub(1); - assert(num > 0); + SPARTA_ASSERT(num > 0); + SPARTA_UNUSED_VARIABLE(num); } else if (!m_running && running) { m_state_counters->num_running.fetch_add(1); } @@ -178,7 +179,8 @@ class WorkerState final { if (i < size) { if (size - 1 == i) { auto num = m_state_counters->num_non_empty_initial.fetch_sub(1); - assert(num > 0); + SPARTA_ASSERT(num > 0); + SPARTA_UNUSED_VARIABLE(num); } other->set_running(true); return boost::optional(std::move(m_initial_tasks.at(i))); @@ -194,7 +196,8 @@ class WorkerState final { // node holds the element we intend to remove. if (node->prev == nullptr) { auto num = m_state_counters->num_non_empty_additional.fetch_sub(1); - assert(num > 0); + SPARTA_ASSERT(num > 0); + SPARTA_UNUSED_VARIABLE(num); } // We can't just delete the node right here, as there may be racing // pop_tasks that read the `->prev` field above. So we stack it away in @@ -291,7 +294,7 @@ WorkQueue::WorkQueue(Executor executor, m_state_counters(num_threads), m_can_push_task(push_tasks_while_running), m_async_runner(async_runner) { - assert(num_threads >= 1); + SPARTA_ASSERT(num_threads >= 1); for (unsigned int i = 0; i < m_num_threads; ++i) { m_states.emplace_back(std::make_unique>( i, &m_state_counters, m_can_push_task)); @@ -301,13 +304,13 @@ WorkQueue::WorkQueue(Executor executor, template void WorkQueue::add_item(Input task) { m_insert_idx = (m_insert_idx + 1) % m_num_threads; - assert(m_insert_idx < m_states.size()); + SPARTA_ASSERT(m_insert_idx < m_states.size()); m_states[m_insert_idx]->m_initial_tasks.push_back(std::move(task)); } template void WorkQueue::add_item(Input task, size_t worker_id) { - assert(worker_id < m_states.size()); + SPARTA_ASSERT(worker_id < m_states.size()); m_states[worker_id]->m_initial_tasks.push_back(std::move(task)); } @@ -397,7 +400,7 @@ void WorkQueue::run_all() { run_in_parallel(worker); for (size_t i = 0; i < m_num_threads; ++i) { - assert(!m_states[i]->m_running); + SPARTA_ASSERT(!m_states[i]->m_running); } if (exception) { @@ -405,10 +408,11 @@ void WorkQueue::run_all() { } for (size_t i = 0; i < m_num_threads; ++i) { - assert(m_states[i]->m_next_initial_task.load(std::memory_order_relaxed) == - m_states[i]->m_initial_tasks.size()); - assert(m_states[i]->m_additional_tasks.load(std::memory_order_relaxed) == - nullptr); + SPARTA_ASSERT( + m_states[i]->m_next_initial_task.load(std::memory_order_relaxed) == + m_states[i]->m_initial_tasks.size()); + SPARTA_ASSERT(m_states[i]->m_additional_tasks.load( + std::memory_order_relaxed) == nullptr); } }