Definition of UNDEBUG info in RelWithDebInfo breaks principle of least astonishment?

[traversaro]

(sorry for ignoring the template, but this discussion did not fit really the template)

I recently discovered (see conda-forge/gz-physics-feedstock#26 (comment)) that gz-cmake defined UNDEBUG (i.e. undefined -DNDEBUG) for RelWithDebInfo.

The motivation for this reported in the code (see

gz-cmake/cmake/GzSetCompilerFlags.cmake

Lines 137 to 142 in 459ea34

	# -UNDEBUG: Undefine the NDEBUG symbol so that assertions get triggered in
	# RelWithDebInfo mode
	# NOTE: Always make -UNDEBUG the first flag in this list so that it appears
	# immiediately after cmake's automatically provided -DNDEBUG flag.
	# Keeping them next to each other should make it more clear that the
	# -DNDEBUG flag is being canceled out.

) are the following:

  # -UNDEBUG: Undefine the NDEBUG symbol so that assertions get triggered in
  #           RelWithDebInfo mode
  # NOTE: Always make -UNDEBUG the first flag in this list so that it appears
  #       immiediately after cmake's automatically provided -DNDEBUG flag.
  #       Keeping them next to each other should make it more clear that the
  #       -DNDEBUG flag is being canceled out.

However, this seems to be quite error prone. RelWithDebInfo has a specific meaning with which CMake users are familiar with, that is basically "Release builds with debug symbols enabled" (that is not entirely true, as Release use -O3 while RelWithDebInfo -O2, but that is another story. The expectation of most users is that in RelWithDebInfo builds, NDEBUG is defined, and so no assert are enabled.

To make you an example, when compiling in RelWithDebInfo users do not expect particular slowdown w.r.t. to Release, and sometimes NDEBUG is used also to disable expensive checks (see for example for Eigen http://eigen.tuxfamily.org/index.php?title=FAQ#How_do_I_get_good_performance.3F).

Perhaps we can look into removing the -DUNDEBUG definition in gz-cmake4?

[mjcarroll]

I agree that even as someone who has spent time in this codebase, this is surprising. I'm okay with going back to the standard set of flags here.

[j-rivero]

Perhaps we can look into removing the -DUNDEBUG definition in gz-cmake4?

Totally in favor in the change yes.