Re: [PATCH 0/3] Introduce BUG_ON(cond, msg) MACRO

Hi,

Jeff King wrote:

On Wed, Nov 22, 2017 at 02:38:24PM -0800, Stefan Beller wrote:

quoted

On reviewing [1] I wondered why there are so many asserts and wondered
if these asserts could have been prevented by a better functionality around
bug reporting in our code.

Introduce a BUG_ON macro, which is superior to assert() by
 * being always there, even when compiled with NDEBUG and
 * providind an additional human readable error message, like BUG()

I'm not sure I agree with the aim of the series.

If people want to compile with NDEBUG, that's their business, I guess.
I don't see much _point_ in it for Git, since most of our assertions do
not respect NDEBUG, and I don't think we tend to assert in expensive
ways anyway.

I do like human readable messages. But sometimes such a message just
makes the code harder to read (and to write). E.g., is there any real
value in:

  BUG_ON(!foo, "called bar() with a foo!");

over:

  assert(foo);

I think you're hinting at wanting

	BUG_ON(!foo);

which is something that the Linux kernel has (and which is not done in
this series).

[...]

I also find (as your third patch switches):

  if (!foo)
	BUG("foo has not been setup");

more readable than the BUG_ON() version, if only because it uses
traditional control flow.

Yes, I think you're right.

Thanks,
Jonathan