Skip to content

Commit

Permalink
refactor: panic: prefix all macros with SILO_
Browse files Browse the repository at this point in the history
To avoid clashing with gtest's `ASSERT_EQ` and potentially
more. Prefix all macros at once.
  • Loading branch information
pflanze authored and Taepper committed Nov 18, 2024
1 parent b56821f commit aa50138
Show file tree
Hide file tree
Showing 22 changed files with 103 additions and 107 deletions.
8 changes: 4 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ endif ()
message(STATUS "Build type: ${CMAKE_BUILD_TYPE}")

set(CMAKE_CXX_FLAGS "-Wall")
set(CMAKE_CXX_FLAGS_DEBUG "-g -fsanitize=address -D DEBUG_ASSERTIONS=1")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -D DEBUG_ASSERTIONS=0")
set(CMAKE_CXX_FLAGS_DEBUG "-g -fsanitize=address -D SILO_DEBUG_ASSERTIONS=1")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -D SILO_DEBUG_ASSERTIONS=0")

# Work-around only for MacOS
if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(CMAKE_CXX_FLAGS_DEBUG "-g -D DEBUG_ASSERTIONS=1")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -D DEBUG_ASSERTIONS=0")
set(CMAKE_CXX_FLAGS_DEBUG "-g -D SILO_DEBUG_ASSERTIONS=1")
set(CMAKE_CXX_FLAGS_RELEASE "-O3 -D SILO_DEBUG_ASSERTIONS=0")
endif ()

set(CMAKE_CXX_STANDARD 20)
Expand Down
84 changes: 42 additions & 42 deletions include/silo/common/panic.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,31 @@ namespace silo::common {
/// Passes arguments to `fmt::format` (at least a format string
/// argument is required) and adds file and line information, then
/// calls `panic`.
#define PANIC(...) silo::common::panic(fmt::format(__VA_ARGS__), __FILE__, __LINE__)
#define SILO_PANIC(...) silo::common::panic(fmt::format(__VA_ARGS__), __FILE__, __LINE__)

/// Denotes a place that isn't implemented *yet*, during
/// development. Follows the same path as `PANIC` when reached.
#define TODO() silo::common::todo(__FILE__, __LINE__)
#define SILO_TODO() silo::common::todo(__FILE__, __LINE__)

[[noreturn]] void todo(const char* file, int line);

/// Denotes a place that theoretically can't be reached. Follows the
/// same path as `PANIC` when reached.
#define UNREACHABLE() silo::common::unreachable(__FILE__, __LINE__)
#define SILO_UNREACHABLE() silo::common::unreachable(__FILE__, __LINE__)

[[noreturn]] void unreachable(const char* file, int line);

/// Denotes a missing implementation. Follows the same path as `PANIC`
/// when reached.
#define UNIMPLEMENTED() silo::common::unimplemented(__FILE__, __LINE__)
#define SILO_UNIMPLEMENTED() silo::common::unimplemented(__FILE__, __LINE__)

[[noreturn]] void unimplemented(const char* file, int line);

/// Asserts that the expression `e` evaluates to true. On failure
/// calls `panic` with the stringification of the code `e` and
/// file/line information. `ASSERT` is always compiled in; if
/// performance overrides safety, use `DEBUG_ASSERT` instead.
#define ASSERT(e) \
/// file/line information. `SILO_ASSERT` is always compiled in; if
/// performance overrides safety, use `SILO_DEBUG_ASSERT` instead.
#define SILO_ASSERT(e) \
do { \
if (!(e)) { \
silo::common::assertFailure(#e, __FILE__, __LINE__); \
Expand All @@ -62,7 +62,7 @@ namespace silo::common {

[[noreturn]] void assertFailure(const char* msg, const char* file, int line);

#define INTERNAL_ASSERT_OP_(prefix_str, e1, op, e2) \
#define SILO_INTERNAL_ASSERT_OP_(prefix_str, e1, op, e2) \
do { \
auto internal_assert_op__v1 = (e1); \
auto internal_assert_op__v2 = (e2); \
Expand All @@ -79,8 +79,8 @@ namespace silo::common {
} \
} while (0)

#define ASSERT_OP_(partial_prefix, e1, op, e2) \
INTERNAL_ASSERT_OP_("ASSERT_" #partial_prefix, e1, op, e2)
#define SILO_ASSERT_OP_(partial_prefix, e1, op, e2) \
SILO_INTERNAL_ASSERT_OP_("ASSERT_" #partial_prefix, e1, op, e2)

[[noreturn]] void assertOpFailure(
const char* prefix,
Expand All @@ -96,49 +96,49 @@ namespace silo::common {
/// value, compared via `==`. On failure calls `panic` with the stringification of the code and the
/// two values passed through fmt::format, plus file/line information. Always compiled in, if
/// performance overrides safety, use `DEBUG_ASSERT_EQ` instead.
#define ASSERT_EQ(e1, e2) ASSERT_OP_(EQ, e1, ==, e2)
#define SILO_ASSERT_EQ(e1, e2) SILO_ASSERT_OP_(EQ, e1, ==, e2)

/// Like ASSERT_EQ but asserts that `e1 <= e2`.
#define ASSERT_LE(e1, e2) ASSERT_OP_(LE, e1, <=, e2)
#define SILO_ASSERT_LE(e1, e2) SILO_ASSERT_OP_(LE, e1, <=, e2)

/// Like ASSERT_EQ but asserts that `e1 < e2`.
#define ASSERT_LT(e1, e2) ASSERT_OP_(LT, e1, <, e2)
#define SILO_ASSERT_LT(e1, e2) SILO_ASSERT_OP_(LT, e1, <, e2)

/// Like ASSERT_EQ but asserts that `e1 >= e2`.
#define ASSERT_GE(e1, e2) ASSERT_OP_(GE, e1, >=, e2)
#define SILO_ASSERT_GE(e1, e2) SILO_ASSERT_OP_(GE, e1, >=, e2)

/// Like ASSERT_EQ but asserts that `e1 > e2`.
#define ASSERT_GT(e1, e2) ASSERT_OP_(GT, e1, >, e2)
#define SILO_ASSERT_GT(e1, e2) SILO_ASSERT_OP_(GT, e1, >, e2)

/// Like ASSERT_EQ but asserts that `e1 != e2`.
#define ASSERT_NE(e1, e2) ASSERT_OP_(NE, e1, !=, e2)
/// Like SILO_ASSERT_EQ but asserts that `e1 != e2`.
#define SILO_ASSERT_NE(e1, e2) SILO_ASSERT_OP_(NE, e1, !=, e2)

/// `DEBUG_ASSERT` is like `ASSERT`, but for cases where performance
/// `SILO_DEBUG_ASSERT` is like `SILO_ASSERT`, but for cases where performance
/// is more important than verification in production: instantiations
/// are only active when compiling SILO in debug (via
/// `CMakeLists.txt`; concretely, they are compiled to be active when
/// the preprocessor variable `DEBUG_ASSERTIONS` is set to 1, and
/// the preprocessor variable `SILO_DEBUG_ASSERTIONS` is set to 1, and
/// ignored if that variable is set to 0; if the variable is missing,
/// a compilation warning is printed and `DEBUG_ASSERT` is ignored, if
/// a compilation warning is printed and `SILO_DEBUG_ASSERT` is ignored, if
/// present with another value, a compilation error results. Note that
/// `DEBUG_ASSERTIONS` must be set to 1 for debug builds or
/// `DEBUG_ASSERT` won't even check the assertion in debug builds. The
/// `SILO_DEBUG_ASSERTIONS` must be set to 1 for debug builds or
/// `SILO_DEBUG_ASSERT` won't even check the assertion in debug builds. The
/// SILO `CMakeLists.txt` does set it up that way.)
#ifndef DEBUG_ASSERTIONS
#ifndef SILO_DEBUG_ASSERTIONS
#warning \
"DEBUG_ASSERTIONS is not set, should be 0 to ignore DEBUG_ASSERT, 1 to compile it in, assuming 0"
#define DEBUG_ASSERTIONS 0
"SILO_DEBUG_ASSERTIONS is not set, should be 0 to ignore SILO_DEBUG_ASSERT, 1 to compile it in, assuming 0"
#define SILO_DEBUG_ASSERTIONS 0
#else
#if DEBUG_ASSERTIONS == 0 /* never */
#elif DEBUG_ASSERTIONS == 1 /* always */
#if SILO_DEBUG_ASSERTIONS == 0 /* never */
#elif SILO_DEBUG_ASSERTIONS == 1 /* always */
#else
#error "DEBUG_ASSERTIONS should be 0 to ignore DEBUG_ASSERT, 1 to compile it in"
#error "SILO_DEBUG_ASSERTIONS should be 0 to ignore SILO_DEBUG_ASSERT, 1 to compile it in"
#endif
#endif

#define DEBUG_ASSERT(e) \
#define SILO_DEBUG_ASSERT(e) \
do { \
if (DEBUG_ASSERTIONS) { \
if (SILO_DEBUG_ASSERTIONS) { \
if (!(e)) { \
silo::common::debugAssertFailure(#e, __FILE__, __LINE__); \
} \
Expand All @@ -147,30 +147,30 @@ namespace silo::common {

[[noreturn]] void debugAssertFailure(const char* msg, const char* file, int line);

#define DEBUG_ASSERT_OP_(partial_prefix, e1, op, e2) \
do { \
if (DEBUG_ASSERTIONS) { \
INTERNAL_ASSERT_OP_("DEBUG_ASSERT_" #partial_prefix, e1, op, e2); \
} \
#define SILO_DEBUG_ASSERT_OP_(partial_prefix, e1, op, e2) \
do { \
if (SILO_DEBUG_ASSERTIONS) { \
SILO_INTERNAL_ASSERT_OP_("DEBUG_ASSERT_" #partial_prefix, e1, op, e2); \
} \
} while (0)

/// Like `ASSERT_EQ`, but like `DEBUG_ASSERT`, for cases where
/// Like `SILO_ASSERT_EQ`, but like `SILO_DEBUG_ASSERT`, for cases where
/// performance is more important than verification in production.
#define DEBUG_ASSERT_EQ(e1, e2) DEBUG_ASSERT_OP_(EQ, e1, ==, e2)
#define SILO_DEBUG_ASSERT_EQ(e1, e2) SILO_DEBUG_ASSERT_OP_(EQ, e1, ==, e2)

/// Like DEBUG_ASSERT_EQ but asserts that `e1 <= e2`.
#define DEBUG_ASSERT_LE(e1, e2) DEBUG_ASSERT_OP_(LE, e1, <=, e2)
#define SILO_DEBUG_ASSERT_LE(e1, e2) SILO_DEBUG_ASSERT_OP_(LE, e1, <=, e2)

/// Like DEBUG_ASSERT_EQ but asserts that `e1 < e2`.
#define DEBUG_ASSERT_LT(e1, e2) DEBUG_ASSERT_OP_(LT, e1, <, e2)
#define SILO_DEBUG_ASSERT_LT(e1, e2) SILO_DEBUG_ASSERT_OP_(LT, e1, <, e2)

/// Like DEBUG_ASSERT_EQ but asserts that `e1 >= e2`.
#define DEBUG_ASSERT_GE(e1, e2) DEBUG_ASSERT_OP_(GE, e1, >=, e2)
#define SILO_DEBUG_ASSERT_GE(e1, e2) SILO_DEBUG_ASSERT_OP_(GE, e1, >=, e2)

/// Like DEBUG_ASSERT_EQ but asserts that `e1 > e2`.
#define DEBUG_ASSERT_GT(e1, e2) DEBUG_ASSERT_OP_(GT, e1, >, e2)
#define SILO_DEBUG_ASSERT_GT(e1, e2) SILO_DEBUG_ASSERT_OP_(GT, e1, >, e2)

/// Like DEBUG_ASSERT_EQ but asserts that `e1 != e2`.
#define DEBUG_ASSERT_NE(e1, e2) DEBUG_ASSERT_OP_(NE, e1, !=, e2)
#define SILO_DEBUG_ASSERT_NE(e1, e2) SILO_DEBUG_ASSERT_OP_(NE, e1, !=, e2)

} // namespace silo::common
2 changes: 1 addition & 1 deletion src/silo/common/aa_symbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ char AminoAcid::symbolToChar(AminoAcid::Symbol symbol) {
case AminoAcid::Symbol::STOP:
return '*';
}
UNREACHABLE();
SILO_UNREACHABLE();
}

std::optional<AminoAcid::Symbol> AminoAcid::charToSymbol(char character) {
Expand Down
14 changes: 7 additions & 7 deletions src/silo/common/lineage_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ Graph::Graph(size_t number_of_vertices)
adjacency_list(number_of_vertices) {}

void Graph::addEdge(Idx vertex_from, Idx vertex_to) {
ASSERT_LT(vertex_from, number_of_vertices);
ASSERT_LT(vertex_to, number_of_vertices);
SILO_ASSERT_LT(vertex_from, number_of_vertices);
SILO_ASSERT_LT(vertex_to, number_of_vertices);
adjacency_list.at(vertex_from).emplace_back(vertex_to);
}

Expand Down Expand Up @@ -100,11 +100,11 @@ std::optional<std::vector<Idx>> Graph::getCycle() const {
if (witness_lasso.has_value()) {
// We found a witness lasso of the form 1 -> 2 -> 3 -> 4 -> 5 -> 3
// We need to remove leading vertices up until the cycle
ASSERT_GE(witness_lasso.value().size(), 2);
SILO_ASSERT_GE(witness_lasso.value().size(), 2);
const Idx cycle_node = witness_lasso.value().back();
auto cycle_node_first_occurrence =
std::find(witness_lasso.value().begin(), witness_lasso.value().end(), cycle_node);
ASSERT(cycle_node_first_occurrence < witness_lasso.value().end());
SILO_ASSERT(cycle_node_first_occurrence < witness_lasso.value().end());
witness_lasso.value().erase(witness_lasso.value().begin(), cycle_node_first_occurrence);
return witness_lasso;
}
Expand Down Expand Up @@ -201,7 +201,7 @@ std::unordered_map<Idx, Idx> assignAliasIdsAndGetAliasMapping(
std::unordered_map<Idx, Idx> alias_mapping;
for (const auto& lineage : file.lineages) {
const auto lineage_id = lookup.getId(lineage.lineage_name.string);
ASSERT(lineage_id.has_value());
SILO_ASSERT(lineage_id.has_value());
for (const auto& alias : lineage.aliases) {
if (lookup.getId(alias.string).has_value()) {
throw silo::preprocessing::PreprocessingException(fmt::format(
Expand All @@ -225,7 +225,7 @@ std::vector<std::pair<Idx, Idx>> getParentChildEdges(
std::vector<std::pair<Idx, Idx>> edge_list;
for (const auto& lineage : file.lineages) {
const auto child_id = lookup.getId(lineage.lineage_name.string);
ASSERT(child_id.has_value());
SILO_ASSERT(child_id.has_value());

for (const auto& parent_lineage : lineage.parents) {
auto parent_id = lookup.getId(parent_lineage.string);
Expand Down Expand Up @@ -270,4 +270,4 @@ LineageTreeAndIdMap LineageTreeAndIdMap::fromLineageDefinitionFilePath(
return fromLineageDefinitionFile(definition_file);
}

} // namespace silo::common
} // namespace silo::common
2 changes: 1 addition & 1 deletion src/silo/common/nucleotide_symbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ char Nucleotide::symbolToChar(Nucleotide::Symbol symbol) {
case Symbol::N:
return 'N';
}
UNREACHABLE();
SILO_UNREACHABLE();
}

std::optional<Nucleotide::Symbol> Nucleotide::charToSymbol(char character) {
Expand Down
4 changes: 2 additions & 2 deletions src/silo/common/optional_bool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ std::optional<bool> OptionalBool::value() const noexcept {
case Representation::TRUE:
return true;
}
UNREACHABLE();
SILO_UNREACHABLE();
}

std::string_view OptionalBool::asStr() const noexcept {
Expand All @@ -57,7 +57,7 @@ std::string_view OptionalBool::asStr() const noexcept {
case Representation::TRUE:
return "true";
}
UNREACHABLE();
SILO_UNREACHABLE();
}

} // namespace silo::common
38 changes: 17 additions & 21 deletions src/silo/common/panic.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@
#include <cctype>
#include <cstdlib>

/* get rid of gtest's definition, we're going to test our own */
#undef ASSERT_EQ

#include "silo/common/panic.h"

namespace {
// Since we can't use ASSERT_EQ from gtest, make a simple
// replacement. `expected` should be without the file:line
// Fuzzy comparison: `expected` should be without the file:line
// information, whereas `got` should contain it.
void assertMsg(std::string got, std::string expected) {
auto start = got.substr(0, expected.size());
Expand Down Expand Up @@ -47,18 +43,18 @@ void nullCapableSetenv(const char* name, const char* value, int overwrite) {

// NOLINTNEXTLINE(readability-identifier-naming,readability-function-cognitive-complexity)
TEST(panic, assertEqPanicModes) {
ASSERT_EQ(1 + 1, 2);
SILO_ASSERT_EQ(1 + 1, 2);

const char* old_env = getenv("SILO_PANIC");
setenv("SILO_PANIC", "", 1);
try {
ASSERT_EQ(1 + 1, 3);
SILO_ASSERT_EQ(1 + 1, 3);
} catch (const std::exception& ex) {
assertMsg(ex.what(), "ASSERT_EQ failure: 1 + 1 == 3: 2 == 3");
};

setenv("SILO_PANIC", "abort", 1);
ASSERT_DEATH(ASSERT_EQ(1 + 1, 3), "ASSERT_EQ failure: 1 \\+ 1 == 3: 2 == 3");
ASSERT_DEATH(SILO_ASSERT_EQ(1 + 1, 3), "ASSERT_EQ failure: 1 \\+ 1 == 3: 2 == 3");

// revert it back
nullCapableSetenv("SILO_PANIC", old_env, 1);
Expand All @@ -67,49 +63,49 @@ TEST(panic, assertEqPanicModes) {
// NOLINTNEXTLINE(readability-identifier-naming,readability-function-cognitive-complexity)
TEST(panic, debugAssertBehavesAsPerCompilationMode) {
// should never complain
DEBUG_ASSERT(1 + 1 == 2);
SILO_DEBUG_ASSERT(1 + 1 == 2);

// Check that DEBUG_ASSERT is active if DEBUG_ASSERTIONS==1, off
// Check that SILO_DEBUG_ASSERT is active if SILO_DEBUG_ASSERTIONS==1, off
// otherwise; each of those branches is only tested when compiling
// the unit tests in debug or release mode, respectively.

#if DEBUG_ASSERTIONS
#if SILO_DEBUG_ASSERTIONS

const char* old_env = getenv("SILO_PANIC");
setenv("SILO_PANIC", "", 1);
try {
DEBUG_ASSERT(1 + 1 == 3);
SILO_DEBUG_ASSERT(1 + 1 == 3);
} catch (const std::exception& ex) {
assertMsg(ex.what(), "DEBUG_ASSERT failure: 1 + 1 == 3");
};
nullCapableSetenv("SILO_PANIC", old_env, 1);

#else
// check that DEBUG_ASSERT is disabled
DEBUG_ASSERT(1 + 1 == 3);
// check that SILO_DEBUG_ASSERT is disabled
SILO_DEBUG_ASSERT(1 + 1 == 3);
#endif
}

// NOLINTNEXTLINE(readability-identifier-naming,readability-function-cognitive-complexity)
TEST(panic, debugAssertGeWorks) {
// stand-in for all the DEBUG_ASSERT_* variants
// stand-in for all the SILO_DEBUG_ASSERT_* variants

DEBUG_ASSERT_GE(1 + 5, 6);
DEBUG_ASSERT_GE(1 + 5, 5);
SILO_DEBUG_ASSERT_GE(1 + 5, 6);
SILO_DEBUG_ASSERT_GE(1 + 5, 5);

#if DEBUG_ASSERTIONS
#if SILO_DEBUG_ASSERTIONS

const char* old_env = getenv("SILO_PANIC");
setenv("SILO_PANIC", "", 1);
try {
DEBUG_ASSERT_GE(1 + 5, 7);
SILO_DEBUG_ASSERT_GE(1 + 5, 7);
} catch (const std::exception& ex) {
assertMsg(ex.what(), "DEBUG_ASSERT_GE failure: 1 + 5 >= 7: 6 >= 7");
};
nullCapableSetenv("SILO_PANIC", old_env, 1);

#else
// check that DEBUG_ASSERT is disabled
DEBUG_ASSERT_GE(1 + 5, 7);
// check that SILO_DEBUG_ASSERT is disabled
SILO_DEBUG_ASSERT_GE(1 + 5, 7);
#endif
}
2 changes: 1 addition & 1 deletion src/silo/common/string_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ std::vector<std::string> tie(
const std::vector<std::string>& elements2,
std::string_view suffix
) {
ASSERT(elements1.size() == elements2.size());
SILO_ASSERT(elements1.size() == elements2.size());
std::vector<std::string> output;
output.reserve(elements1.size());
for (size_t i = 0; i < elements1.size(); ++i) {
Expand Down
2 changes: 1 addition & 1 deletion src/silo/common/table_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ silo::TableReader::TableReader(

size_t silo::TableReader::read() {
loadTable();
ASSERT(query_result->ColumnCount() == column_functions.size() + 1);
SILO_ASSERT(query_result->ColumnCount() == column_functions.size() + 1);
size_t current_start_of_chunk = 0;
std::unique_ptr<duckdb::DataChunk> current_chunk = query_result->Fetch();
while (current_chunk) {
Expand Down
Loading

0 comments on commit aa50138

Please sign in to comment.