Skip to content

Commit

Permalink
Add a macro to mark unused variables
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arthaud authored and facebook-github-bot committed Nov 27, 2023
1 parent a023dfd commit 8e291da
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 23 deletions.
11 changes: 11 additions & 0 deletions include/sparta/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include <cassert>
#include <exception>

#include <boost/config.hpp>
Expand Down Expand Up @@ -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<void>(v)

namespace sparta::exception_impl {

template <class E>
Expand Down
16 changes: 8 additions & 8 deletions include/sparta/IntervalDomain.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

#pragma once

#include <cassert>
#include <limits>
#include <ostream>

#include <boost/functional/hash.hpp>

#include <sparta/AbstractDomain.h>
#include <sparta/Exceptions.h>

namespace sparta {

Expand Down Expand Up @@ -55,21 +55,21 @@ class IntervalDomain final : public AbstractDomain<IntervalDomain<Num>> {

/* [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};
}

Expand All @@ -87,7 +87,7 @@ class IntervalDomain final : public AbstractDomain<IntervalDomain<Num>> {

/* 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;
}

Expand All @@ -96,7 +96,7 @@ class IntervalDomain final : public AbstractDomain<IntervalDomain<Num>> {
* Guaranteed to be greater than or equal to lower_bound().
*/
Num upper_bound() const {
assert(!is_bottom());
SPARTA_ASSERT(!is_bottom());
return m_ub;
}

Expand Down
5 changes: 3 additions & 2 deletions include/sparta/ThreadPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@

#pragma once

#include <cassert>
#include <condition_variable>
#include <functional>
#include <mutex>
#include <queue>
#include <thread>

#include <sparta/Exceptions.h>

namespace sparta {

// The AsyncRunner provides a way to run work on a separate thread. The main
Expand Down Expand Up @@ -79,7 +80,7 @@ class ThreadPool : public AsyncRunner {
void run_async_bound(std::function<void()> bound_f) override {
{
std::lock_guard<std::mutex> 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;
Expand Down
30 changes: 17 additions & 13 deletions include/sparta/WorkQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <algorithm>
#include <atomic>
#include <boost/optional/optional.hpp>
#include <cassert>
#include <chrono>
#include <condition_variable>
#include <exception>
Expand All @@ -21,6 +20,7 @@
#include <utility>

#include <sparta/Arity.h>
#include <sparta/Exceptions.h>
#include <sparta/ThreadPool.h>

namespace sparta {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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<Input>(std::move(m_initial_tasks.at(i)));
Expand All @@ -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
Expand Down Expand Up @@ -291,7 +294,7 @@ WorkQueue<Input, Executor>::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<WorkerState<Input>>(
i, &m_state_counters, m_can_push_task));
Expand All @@ -301,13 +304,13 @@ WorkQueue<Input, Executor>::WorkQueue(Executor executor,
template <class Input, typename Executor>
void WorkQueue<Input, Executor>::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 <class Input, typename Executor>
void WorkQueue<Input, Executor>::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));
}

Expand Down Expand Up @@ -397,18 +400,19 @@ void WorkQueue<Input, Executor>::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) {
std::rethrow_exception(exception);
}

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);
}
}

Expand Down

0 comments on commit 8e291da

Please sign in to comment.