[llvm-dev] [RFC] Usage of NDEBUG as a guard for non-assert debug code

[Philip Reames via llvm-dev]

On 4/9/20 9:59 AM, Chris Tetreault via llvm-dev wrote:
>
> David,
>
>    In my opinion, NDEBUG is one of those gross old C things that 
> everybody complains about. It’s called “Not Debug”, but really it 
> means “Assert Disabled”. I think one could be forgiven for actually 
> using it as a heuristic of whether or not a build is a debug build, 
> especially since no other options are provided. I appreciate your 
> desire, but I think it’d be unfortunate if the build system grew yet 
> another flag to control debugness.
>
>    As far as I can tell, as it currently works, LLVM_ENABLE_ASSERTIONS 
> just makes sure that NDEBUG is not defined, even in release builds. So 
> if I do -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=TRUE, I’ll 
> get an optimized build with no debug symbols but with asserts enabled, 
> which in my mind isn’t a terribly useful thing to have.
>
(Completely ignoring rest of thread context.  Replying to previous 
sentence in isolation.)

A Release+Asserts build is an *incredibly* useful thing to have. It's 
generally the only configuration I build.  It's fast enough to not be 
obviously painfully slow, several times faster to build than a debug 
build, takes *much* less space on disk, and yet reports assertion failures.

> Furthermore, none of this works on Visual Studio because it has a UI 
> menu to control the build type. I personally would be very 
> disappointed to see Visual Studio’s build type dropdown break.
>
>    Since we C’s assert, it is intrinsically tied to NDEBUG. What we 
> need is proper custom asserts. In codebases I’ve seen in my travels 
> that have this it usually looks like:
>
> // If asserts are enabled, evaluate and assert that expr is truthy. If 
> it is not, complain with msg.
>
> LLVM_ASSERT(expr, msg)
>
> // If asserts are enabled, evaluate and assert that expr is truthy. If 
> it is not, complain with msg.
>
> // If asserts are disabled, evaluate expr, do not assert.
>
> // either way, return expr
>
> LLVM_VERIFY(expr, msg)
>
>    The first one is useful as a traditional assert. The second one is 
> useful if you are calling a function, and want to assert that it 
> succeeds, but still need it to be evaluated in release builds:
>
> auto *Foo = 
> LLVM_VERIFY(ReturnsAPointerThatShouldNeverActuallyBeNull(), “this 
> should never return null”);
>
>    If we have custom asserts, then we can have custom assert guard macros:
>
> // true if this is any sort of debug build
>
> LLVM_DEBUG_BUILD
>
> // true if asserts are turned on (Debug build on Windows,
>
> // Debug build or -DLLVM_ASSERTIONS_ENABLED=TRUE on other platforms)
>
> LLVM_ASSERTS_ENABLED
>
>    These flags could be derived from just CMAKE_BUILD_TYPE, and 
> LLVM_ENABLE_ASSERTIONS can go away (assuming we agree that an 
> asserting build with optimizations and no debug info is worse than 
> useless). Custom asserts also have the advantage of having a proper 
> message parameter and not needing to rely on the truthiness of string 
> literals. Obviously this is a much more invasive change than what you 
> are proposing, but in my opinion it’s the correct thing to do.
>
> Thanks,
>
>    Christopher Tetreault
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of 
> *David Truby via llvm-dev
> *Sent:* Thursday, April 9, 2020 7:26 AM
> *To:* llvm-dev at lists.llvm.org
> *Subject:* [EXT] [llvm-dev] [RFC] Usage of NDEBUG as a guard for 
> non-assert debug code
>
> Hi all,
>
> During discussions about assertions in the Flang project, we noticed 
> that there are a lot of cases in LLVM that #ifndef NDEBUG is used as a 
> guard for non-assert code that we want enabled in debug builds.
>
> This works fine on its own, however it affects the behaviour of 
> LLVM_ENABLE_ASSERTIONS;  since NDEBUG controls whether assertions are 
> enabled or not, a lot of debug code gets enabled in addition to 
> asserts if you specify this flag. This goes contrary to the name of 
> the flag I believe also its intention. Specifically in Flang we have a 
> case where someone wants to ship a build with assertions enabled, but 
> doesn't want to drag in all the extra things that are controlled by 
> NDEBUG in LLVM.
>
> In my opinion we ideally want LLVM_ENABLE_ASSERTIONS to _only_ enable 
> assertions and do nothing else. I don't think this is possible without 
> changing the use of NDEBUG elsewhere as NDEBUG controls whether assert 
> is enabled.
>
> I propose we should be using another macro (something like 
> LLVM_DEBUG_CHECKS ?) that is enabled in Debug builds, and possibly 
> controlled by another cmake flag (LLVM_ENABLE_DEBUG_CHECKS ?) for code 
> that we want enabled for debugging but not in releases. This would 
> allow LLVM_ENABLE_ASSERTIONS to do what it says on the tin and 
> actually enable assertions only.
>
> Does anyone else have any thoughts on this?
>
> Thanks
>
> David Truby
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200409/9df08bd9/attachment.html>