On Dec 21, 2016, at 07:55, Jeff King wrote:
On Tue, Dec 20, 2016 at 09:54:15PM -0800, Kyle J. McKay wrote:
I wasn't aware anybody actually built with NDEBUG at all. You'd
have to
explicitly ask for it via CFLAGS, so I assume most people don't.
Not a good assumption. You know what happens when you assume[1],
right? ;)
Kind of. If it's a configuration that nobody[1] in the Git development
community intended to support or test, then isn't the person
triggering
it the one making assumptions?
No, I don't think so. NDEBUG is very clearly specified in POSIX [1].
If NDEBUG is defined then "assert(...)" disappears (and in a nice way
so as not to precipitate "unused variable" warnings). "N" being "No"
or "Not" or "Negated" or "bar over the top" + "DEBUG" meaning Not
DEBUG. So the code that goes away when NDEBUG is defined is clearly
debug code.
Considering the wide deployment and use of Git at this point I think
rather the opposite to be true that "Git does Not require DEBUGging
code to be enabled for everyday use." The alternative that it does
suggests it's not ready for prime time and quite clearly that's not
the case.
I've been defining NDEBUG whenever I make a release build for quite
some
time (not just for Git) in order to squeeze every last possible
drop of
performance out of it.
I think here you are getting into superstition. Is there any single
assert() in Git that will actually have an impact on performance?
You have suggested there is and that Git is enabling NDEBUG for
exactly that reason -- to increase performance:
On Dec 20, 2016, at 08:45, Jeff King wrote:
I do notice that we set NDEBUG for nedmalloc, though if I am
reading the
Makefile right, it is just for compiling those files. It looks like
there are a ton of asserts there that _are_ potentially expensive
Perhaps Git should provide a "verify" macro. Works like "assert"
except
that it doesn't go away when NDEBUG is defined. Being Git-provided
it could
also use Git's die function. Then Git could do a global replace of
assert
with verify and institute a no-assert policy.
What would be the advantage of that over `if(...) die("BUG: ...")`? It
does not require you to write a reason in the die(), but I am not sure
that is a good thing.
You have stated that you believe the current "assert" calls in Git
(excluding nedmalloc) should not magically disappear when NDEBUG is
defined. So precluding a more labor intensive approach where all
currently existing "assert(...)" calls are replaced with an "if (!...)
die(...)" combination, providing a "verify" macro is a quick way to
make that happen. Consider this, was the value that Jonathan provided
for the "die" string immediately obvious to you? It sure wasn't to
me. That means that whoever does the "assert(...)" -> "if(!...)die"
swap out may need to be intimately familiar with that particular piece
of code or the result will be no better than using a "verify" macro.
I'm just trying to find a quick and easy way to accommodate your
wishes without redefining the semantics of NDEBUG. ;)
--Kyle
[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html
