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

Thu Apr 9 12:03:29 PDT 2020

On 4/9/20 12:16 PM, Philip Reames via llvm-dev wrote:
>
> 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.
>

+1

Same for me on my laptop for years.

>> 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
>
>
> _______________________________________________
> 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/1142be85/attachment.html>