On Wed, Mar 31, 2021 at 11:01 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> 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. 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.
