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

Thu Apr 9 10:09:30 PDT 2020

On Thu, Apr 9, 2020 at 9:59 AM Chris Tetreault via llvm-dev <
llvm-dev at lists.llvm.org> 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.
>

FWIW, I believe quite a few people use that mode & don't use debuggers much
- faster link/compile times, etc.

> 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”);
>

This seems fairly orthogonal to the rest of this discussion, to me at least
- it's an improvement over the existing/common:

auto *Foo = ReturnsAPointer...;
assert(Foo && "this should never be null");

But not a fundamentally new thing, etc. (& could be proposed independent of
renaming 'assert' to LLVM_ASSERT)

>
>
>    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).
>

I don't think people would agree to that - I believe at least a few regular
LLVM developers use that mode regularly.

> 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/8e8c5f3c/attachment.html>