From ef5f4061012078a9d24099967bb35b634c88fccc Mon Sep 17 00:00:00 2001 From: Xeom Date: Mon, 4 Nov 2024 19:01:52 +0000 Subject: [PATCH] Fix: Print correct file/line when a subroutine contains an assert. Tests such as asserts were implemented as macros containing two parts: snow_fail_update(); // Update the globally stored file and line number _asserteq(thing 1, thing 2); // Perform a check This falls down if "thing 1" or "thing 2" contain assertions themselves, which themselves update the globally stored file and line number. If the outer assertion then fails, it reports the wrong file and line number. To fix this, add a struct _snow_explanation that is created in assertion macros, and is used to update the global file and line number only once the actual failiure is triggered. --- snow/snow.h | 75 +++++++++++++++------------------ test/.gitignore | 2 + test/cases/misc.c | 22 ++++++++++ test/expected/misc-fail-in-call | 14 ++++++ test/test.c | 6 +++ 5 files changed, 78 insertions(+), 41 deletions(-) create mode 100644 test/cases/misc.c create mode 100644 test/expected/misc-fail-in-call diff --git a/snow/snow.h b/snow/snow.h index 90436d3..30a47c5 100644 --- a/snow/snow.h +++ b/snow/snow.h @@ -257,6 +257,12 @@ struct _snow_desc_func { void (*func)(void); }; +struct _snow_explanation { + const char *description; + const char *filename; + int linenum; +}; + struct _snow { int exit_code; const char *filename; @@ -535,12 +541,17 @@ static void _snow_print_desc_end(void) { _snow_case_end(0); \ } while (0) +#define _snow_explain(...) \ + (struct _snow_explanation){ "" __VA_ARGS__, __FILE__, __LINE__ } + #define _snow_fail_expl(expl, fmt, ...) \ do { \ - if (expl[0] == '\0') { \ + _snow.filename = expl.filename; \ + _snow.linenum = expl.linenum; \ + if (expl.description[0] == '\0') { \ snow_fail(fmt ".", __VA_ARGS__); \ } else { \ - snow_fail(fmt ": %s", __VA_ARGS__, expl); \ + snow_fail(fmt ": %s", __VA_ARGS__, expl.description); \ } \ } while (0) @@ -1228,10 +1239,8 @@ static int snow_main_function(int argc, char **argv) { #define assert(x, expl...) \ do { \ - snow_fail_update(); \ - const char *_snow_explanation = "" expl; \ if (!(x)) \ - _snow_fail_expl(_snow_explanation, "Assertion failed: %s", #x); \ + _snow_fail_expl(_snow_explain(expl), "Assertion failed: %s", #x); \ } while (0) /* @@ -1241,7 +1250,7 @@ static int snow_main_function(int argc, char **argv) { #define _snow_define_assertfunc(name, type, pattern) \ __attribute__((unused)) \ static int _snow_assert_##name( \ - int invert, const char *explanation, \ + int invert, struct _snow_explanation explanation, \ const type a, const char *astr, const type b, const char *bstr) { \ int eq = (a) == (b); \ if (!eq && !invert) \ @@ -1261,7 +1270,7 @@ _snow_define_assertfunc(ptr, void *, "%p") __attribute__((unused)) static int _snow_assert_str( - int invert, const char *explanation, + int invert, struct _snow_explanation explanation, const char *a, const char *astr, const char *b, const char *bstr) { int eq = strcmp(a, b) == 0; if (!eq && !invert) @@ -1277,7 +1286,7 @@ static int _snow_assert_str( __attribute__((unused)) static int _snow_assert_buf( - int invert, const char *explanation, + int invert, struct _snow_explanation explanation, const void *a, const char *astr, const void *b, const char *bstr, size_t size) { int eq = memcmp(a, b, size) == 0; @@ -1335,44 +1344,37 @@ static int _snow_assert_fake(int invert, ...) { #define asserteq_dbl(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_dbl( \ - 0, "" expl, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define asserteq_ptr(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_ptr( \ - 0, "" expl, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define asserteq_str(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_str( \ - 0, "" expl, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define asserteq_int(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_int( \ - 0, "" expl, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define asserteq_uint(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_uint( \ - 0, "" expl, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define asserteq_buf(a, b, size, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_buf( \ - 0, "" expl, (a), #a, (b), #b, size); \ + 0, _snow_explain(expl), (a), #a, (b), #b, size); \ } while (0) #define asserteq_any(a, b, expl...) \ do { \ snow_fail_update(); \ - const char *_snow_explanation = "" expl; \ _Pragma("GCC diagnostic push") \ _Pragma("GCC diagnostic ignored \"-Wpragmas\"") \ _Pragma("GCC diagnostic ignored \"-Wpointer-arith\"") \ @@ -1381,12 +1383,12 @@ static int _snow_assert_fake(int invert, ...) { typeof ((b)+0) _b = b; \ _Pragma("GCC diagnostic pop") \ if (sizeof(_a) != sizeof(_b)) { /* NOLINT */ \ - _snow_fail_expl(_snow_explanation, \ + _snow_fail_expl(_snow_explain(expl), \ "Expected %s to equal %s, but their lengths don't match", \ #a, #b); \ } else { \ _snow_assert_buf( \ - 0, _snow_explanation, &_a, #a, &_b, #b, sizeof(_a)); \ + 0, _snow_explain(expl), &_a, #a, &_b, #b, sizeof(_a)); \ } \ } while (0) @@ -1396,44 +1398,37 @@ static int _snow_assert_fake(int invert, ...) { #define assertneq_dbl(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_dbl( \ - 1, "" expl, (a), #a, (b), #b); \ + 1, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define assertneq_ptr(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_ptr( \ - 1, "" expl, (a), #a, (b), #b); \ + 1, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define assertneq_str(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_str( \ - 1, "" expl, (a), #a, (b), #b); \ + 1, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define assertneq_int(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_int( \ - 1, "" expl, (a), #a, (b), #b); \ + 1, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define assertneq_uint(a, b, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_uint( \ - 2, "" expl, (a), #a, (b), #b); \ + 2, _snow_explain(expl), (a), #a, (b), #b); \ } while (0) #define assertneq_buf(a, b, size, expl...) \ do { \ - snow_fail_update(); \ _snow_assert_buf( \ - 1, "" expl, (a), #a, (b), #b, (size)); \ + 1, _snow_explain(expl), (a), #a, (b), #b, (size)); \ } while (0) #define assertneq_any(a, b, expl...) \ do { \ snow_fail_update(); \ - const char *_snow_explanation = "" expl; \ _Pragma("GCC diagnostic push") \ _Pragma("GCC diagnostic ignored \"-Wpragmas\"") \ _Pragma("GCC diagnostic ignored \"-Wpointer-arith\"") \ @@ -1445,7 +1440,7 @@ static int _snow_assert_fake(int invert, ...) { break; \ } else { \ _snow_assert_buf( \ - 1, _snow_explanation, &_a, #a, &_b, #b, sizeof(_a)); \ + 1, _snow_explain(expl), &_a, #a, &_b, #b, sizeof(_a)); \ } \ } while (0) @@ -1456,9 +1451,8 @@ static int _snow_assert_fake(int invert, ...) { #define asserteq(a, b, expl...) \ do { \ snow_fail_update(); \ - const char *_snow_explanation = "" expl; \ int _snow_ret = _snow_generic_assert(b)( \ - 0, _snow_explanation, (a), #a, (b), #b); \ + 0, _snow_explain(expl), (a), #a, (b), #b); \ if (_snow_ret < 0) { \ asserteq_any(a, b, expl); \ } \ @@ -1471,9 +1465,8 @@ static int _snow_assert_fake(int invert, ...) { #define assertneq(a, b, expl...) \ do { \ snow_fail_update(); \ - const char *_snow_explanation = "" expl; \ int _snow_ret = _snow_generic_assert(b)( \ - 1, _snow_explanation, (a), #a, (b), #b); \ + 1, _snow_explain(expl), (a), #a, (b), #b); \ if (_snow_ret < 0) { \ assertneq_any(a, b, expl); \ } \ diff --git a/test/.gitignore b/test/.gitignore index c8610db..0f1e036 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -8,4 +8,6 @@ cases/tests cases/tests.exe cases/around cases/around.exe +cases/misc +cases/misc.exe snow diff --git a/test/cases/misc.c b/test/cases/misc.c new file mode 100644 index 0000000..a4f8516 --- /dev/null +++ b/test/cases/misc.c @@ -0,0 +1,22 @@ +#include + +static inline int asserteq_call(int a, int b) +{ + asserteq(a, b); + return a + b; +} + +static inline int fail_call() +{ + fail("It has gone wrong :("); + return 0; +} + +describe(fail_in_call) { + test("asserteq pass") { asserteq(asserteq_call(1, 1), 2); } + test("asserteq fail outer") { asserteq(asserteq_call(1, 1), 3); } + test("asserteq fail inner") { asserteq(asserteq_call(1, 2), 3); } + test("fail inner") { asserteq(fail_call(), 0); } +} + +snow_main(); diff --git a/test/expected/misc-fail-in-call b/test/expected/misc-fail-in-call new file mode 100644 index 0000000..3280ea6 --- /dev/null +++ b/test/expected/misc-fail-in-call @@ -0,0 +1,14 @@ + +Testing fail_in_call: +✓ Success: asserteq pass (1.00s) +✕ Failed: asserteq fail outer: + (int) Expected asserteq_call(1, 1) to equal 3, but got 2. + in cases/misc.c:17(fail_in_call) +✕ Failed: asserteq fail inner: + (int) Expected a to equal b, but got 1. + in cases/misc.c:5(fail_in_call) +✕ Failed: fail inner: + It has gone wrong :( + in cases/misc.c:11(fail_in_call) +fail_in_call: Passed 1/4 tests. (6.00s) + diff --git a/test/test.c b/test/test.c index 19ec788..8905fab 100644 --- a/test/test.c +++ b/test/test.c @@ -323,5 +323,11 @@ describe(around) { } } +describe(misc) { + it("can fail in a subroutine call") { + assert(compareOutput("./cases/misc fail_in_call", "misc-fail-in-call")); + } +} + snow_main(); #endif