Export to GitHub

lilypond - issue #2787

Sanitize usage of -DDEBUG, -DNDEBUG, assert, programming_error and so on


Posted on Aug 29, 2012 by Quick Lion

At the current point of time, -enable-optimising implies -DNDEBUG which is nonsensical.

-DNDEBUG has the effect of disabling "assert" in the standard library. The normal use of assert is to spell out an underlying assumption that the following code depends on. A failed assertion implies broken program logic, and an inability to provide useful results. Disabling assertions saves a minimal amount of execution time (one should not use expensive function calls in assertions) and is mostly there for mission-critical code where graceful failure is not an option, or for end-user code without a useful interface for relaying the assertion message. Basically, when a program crashing silently or continuing with wrong results is just as bad as a controlled exit.

LilyPond is a console program. It will always report failed assertions to stderr where they can be reported. Compiling it with disabled assertions makes no sense. Even if it is running as a software service hidden to the end user, the application using it can make use of stderr.

It would be conceivable to add an "-disable-checking" option to configure, but if there ever is a valid use case, the requirements will be so special that having to add -DNDEBUG manually to CXXFLAGS is quite acceptable.

Programming errors are things that should not occur with regular use, but may be due to user error and where a strategy for continuing is feasible. It does not make sense to enable checks for programming errors (there is no way of disabling them currently) while disabling assertions.

So my feeling with regard to NDEBUG is: hands off.

Now LilyPond has several more expensive checks and/or gathering of runtime profiles (the latter might warrant a separate configuration option, in particular since their presence might affect normal operations, to the degree where garbage collection problems, to be diagnosed with expensive checks, disappear). If we want to enable/disable them selectively, -DDEBUG is sort of customary for that. A configure option -denable-checks or -denable-debug seems useful for that (Patchy should use that).

All of the debugging options should be orthogonal to -disable-optimising. In fact, the debugging options are very important when optimising is enabled since a number of garbage collection problems are more likely to appear when the optimiser is being used. While it is true that optimisation can complicate debugging, there is no larger obstacle to debugging than not being able to trigger the bug.

Comment #1

Posted on May 12, 2015 by Quick Lion

Issue 2787: Sanitize usage of -DDEBUG, -DNDEBUG and assert

The compiler option -DNDEBUG is no longer being used: -DNDEBUG disables the assert function, and assert is essentially stating that the program cannot useful continue if the assertion is not met. -DNDEBUG is basically an option for compiling an application to a limited amount of ROM when aborting with a diagnostic is not preferable to crashing.

This is not the case for LilyPond. So expensive debugging options now are enabled with -DDEBUG instead. At the current point of time, setting this is still tied to the configure option --disable-optimising. It might make sense to move this to a separate option --enable-testing or similar, but that would require simultaneous changes to the Patchy testing framework.

http://codereview.appspot.com/236190043

Comment #2

Posted on May 12, 2015 by Quick Lion

Add --enable-checking option, let it be implied by --disable-optimising for now

http://codereview.appspot.com/236190043

Comment #3

Posted on May 12, 2015 by Quick Lion

Patchy the autobot says: passes tests.

Comment #4

Posted on May 12, 2015 by Quick Kangaroo

Patchy the autobot says: passes tests. Includes a full make doc

Comment #5

Posted on May 16, 2015 by Quick Kangaroo

Patch on countdown for May 19th

Comment #6

Posted on May 19, 2015 by Quick Kangaroo

Patch counted down - please push

Comment #7

Posted on May 19, 2015 by Quick Kangaroo

Patch counted down - please push

Comment #8

Posted on May 19, 2015 by Quick Lion

Pushed to staging as commit a4cc910a3401d25bb94ff0ecb4dc18f681c71004 Author: David Kastrup Date: Tue May 12 19:01:57 2015 +0200

Issue 2787: Sanitize usage of -DDEBUG, -DNDEBUG and assert

The compiler option -DNDEBUG is no longer being used: -DNDEBUG disables
the assert function, and assert is essentially stating that the program
cannot useful continue if the assertion is not met.  -DNDEBUG is
basically an option for compiling an application to a limited amount of
ROM when aborting with a diagnostic is not preferable to crashing.

This is not the case for LilyPond.  So expensive debugging options now
are enabled with -DDEBUG instead.  There is a new configure option
--enable-checking defaulting to "off" for this now.

At the current point of time, setting --disable-optimising also has the
effect of enabling the checks: this will be retained until Patchy has
been adapted to using --enable-checking.

Comment #9

Posted on May 25, 2015 by Happy Wombat

(No comment was entered for this change.)

Status: Verified

Labels:
Fixed_2_19_21 Type-Build