On Thu, Mar 13, 2025 at 9:55 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > On Wed, Mar 12, 2025 at 4:18 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> > >> Elijah Newren <newren@xxxxxxxxx> writes: > >> > >> > But it might be worth mentioning that having side-effects in > >> > assertions is a huge no-no, and I understand that when folks have to > >> > debug one of those (I had to in a separate project years ago which was > >> > kind of nasty), that they sometimes jump to the conclusion that > >> > assertions are bad. > >> > >> Yes, assert() invites such mistakes. Why not avoid them when there > >> are better alternatives like "if (condition) BUG()"? > > > > I mean, I just gave my reasons above which you snipped out. But if > > you feel it is important for folks to move away from assert(), would > > you like to see someone create a better alternative to assert, such as > > BUG_ON(condition), so that they have reason to want to switch? > > You said "BUG_ON()" is better than "if (condition) BUG()", but I do > not see a reason why. It also shares the same problem with assert() > if we make it honor NDEBUG. "if (condition) BUG()" is invalid; it needs more arguments. "if (condition) BUG(something)" requires a separate "something", which requires awkward additional wording and/or is needlessly duplicative. If you don't want to see assert in the codebase because of NDEBUG, then obviously we'd leave NDEBUG out of BUG_ON(). Such a BUG_ON() would be an improvement because it maintains the ergonomics of assert() while avoiding the potential mistake of accidentally including something with side-effects. "If (condition) BUG(something)" doesn't preserve the ergonomics and is thus worse, IMO. (Personally, I still like having cheap checks that are okay to compile out, but if we want a project directive against those, then I think a BUG_ON() that doesn't depend on NDEBUG would be the way to move in that direction.)
