Hi,
Jeff King wrote:
> On Wed, Nov 22, 2017 at 02:38:24PM -0800, Stefan Beller wrote:
>> 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
