Elijah Newren [off-list ref] writes:
... would you two prefer
that I add a new affirm() function that is ALSO compiled out in
production?
The reason I do not think affirm would fly is because it shares the
most problematic trait with assert(): it can be comipled out.
There was at least one bug we found and fixed in a piece of code
that came later in a codepath that had an assert() in it. The buggy
code (IIRC, it was added way after the assert() was written in an
unrelated patch) was unknowingly depending on a side-effect of an
innocuous function call (i.e. assert(func_tion(args) == expected))
made in the assert condition, and the bug did not reproduce in the
developer's environment.
On Wed, Mar 31, 2021 at 11:01 AM Junio C Hamano [off-list ref] wrote:
Elijah Newren [off-list ref] writes:
quoted
... would you two prefer
that I add a new affirm() function that is ALSO compiled out in
production?
The reason I do not think affirm would fly is because it shares the
most problematic trait with assert(): it can be comipled out.
There was at least one bug we found and fixed in a piece of code
that came later in a codepath that had an assert() in it. The buggy
code (IIRC, it was added way after the assert() was written in an
unrelated patch) was unknowingly depending on a side-effect of an
innocuous function call (i.e. assert(func_tion(args) == expected))
made in the assert condition, and the bug did not reproduce in the
developer's environment.
Ah, now that's a much different argument than claiming assert is
useless. I had to debug one of those once about a decade ago, in a
different project, put in by some other developer, and yeah it's
painful.
If that causes assert() to be frowned upon, then I can understand
based on this reason. If so, perhaps a BUG_ON() function would be
useful? The "if (...) BUG()" construct causes line coverage problems
and I find it an onerous way to signal to future code readers "hey,
just as a reminder, these things should be true at this point". If
you need to signal something more than that or explain why the things
being false is a problem or what might cause it, then the "if (...)
BUG()" construct is useful and there are certainly many places for
that, but it just doesn't fit all the cases.