llvm dev - Apr 2020 - [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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200409/5f7af74c/attachment.html>

Hi David,

It's true, there are many development-only features guarded by ifndef
NDEBUG in LLVM and Clang. Generally, I think LLVM prefers to have as few
configure-time compilation modes as possible. The existing alternative
build modes of "expensive checks" and "ABI breaking checks"
are already
expensive enough to maintain.

I would caution you that, in our experience in Chromium, we found that
assertions add ~25% to compile time, so shipping with assertions enabled is
not a small performance hit. Hans Wennborg did some experiments, and he
found that the highest overhead asserts also have the highest value (mostly
type casting asserts). See the details in https://crbug.com/896306#c27.

Given how expensive the assertions are by themselves, I suggest that you
either accept the cost of the additional features enabled with assertions
(dbgs etc), or ship without assertions.

Reid

On Thu, Apr 9, 2020 at 7:26 AM David Truby via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> 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/3c36f0f1/attachment.html>

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. 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200409/51028d13/attachment.html>

I almost always build with  -DCMAKE_BUILD_TYPE=Release
-DLLVM_ENABLE_ASSERTIONS=TRUE  because in addition to assertions,
-DLLVM_ENABLE_ASSERTIONS=TRUE enables all of the debug printing controlled
by -debug/-debug-only. So you get a compiler that runs faster than
-DCMAKE_BUILD_TYPE=Debug, takes less disk space and is still somewhat
debuggable. You can always throw in more print messages if you need more
info without needing to use gdb.

~Craig


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. 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/ecd63cf4/attachment-0001.html>

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>

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>

#ifndef NDEBUG in the LLVM source and assert() are at least somewhat
linked. For instance, consider
https://github.com/llvm/llvm-project/blob/8423a6f36386aabced2a367c0ea53487901d31ca/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp#L2668

The #ifndef NDEBUG is used to guard the value that checked in an
assert() later. Only enabling assert() will cause the build to fail
because the value is not defined. Another example is a loop only for
the assert, eg.

    #ifndef NDEBUG
    for (auto &Val : Container)
      assert(Val && "all");
    #endif

where the loop would loop over nothing with assertions disable. We
cannot have a 'change all 'NDEBUG to LLVM_NO_DEBUG_CHECKS'. Even if
we
manage to leave all NDEBUG that are linked to some use of assert(), it
doubles the number of configurations that might break in some
configuration such as IndVarSimplify. I understand NDEBUG as `remove
all the code only useful for developers`, independent of whether we
also want debug symbols.

I'd find it more useful to discuss what should NOT be covered under
the blanket term LLVM_ENABLE_ASSERTIONS. Example from the past are
LLVM_ENABLE_STATS and LLVM_ENABLE_DUMP that once was also using
NDEBUG. As was mentioned, Support/Debug.h might be another candidate.
But IMHO the compile-time/execution-time/code-size impact of these are
too small to justify the increase combinatorial complexity of
configuration. At the last LLVM DevMtg we actually discussed how to
*reduce* the number of independent configuration parameters.

Michael


Am Do., 9. Apr. 2020 um 09:26 Uhr schrieb David Truby via llvm-dev
<llvm-dev at lists.llvm.org>:
>
> 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

On 2020-04-10, Michael Kruse via llvm-dev wrote:
>#ifndef NDEBUG in the LLVM source and assert() are at least somewhat
>linked. For instance, consider
>https://github.com/llvm/llvm-project/blob/8423a6f36386aabced2a367c0ea53487901d31ca/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp#L2668
>
>The #ifndef NDEBUG is used to guard the value that checked in an
>assert() later. Only enabling assert() will cause the build to fail
>because the value is not defined. Another example is a loop only for
>the assert, eg.
>
>    #ifndef NDEBUG
>    for (auto &Val : Container)
>      assert(Val && "all");
>    #endif
>
>where the loop would loop over nothing with assertions disable. We
>cannot have a 'change all 'NDEBUG to LLVM_NO_DEBUG_CHECKS'. Even
if we
>manage to leave all NDEBUG that are linked to some use of assert(), it
>doubles the number of configurations that might break in some
>configuration such as IndVarSimplify. I understand NDEBUG as `remove
>all the code only useful for developers`, independent of whether we
>also want debug symbols.
>
>I'd find it more useful to discuss what should NOT be covered under
>the blanket term LLVM_ENABLE_ASSERTIONS. Example from the past are
>LLVM_ENABLE_STATS and LLVM_ENABLE_DUMP that once was also using
>NDEBUG. 
+1 for bring up this topic.


The code base sometimes uses #ifdef NDEBUG to guard the definitions of
some struct/class members and functions.

This means there are inherent ABI incompatibility between non-NDEBUG and
NDEBUG object files. I hoped that mixing object files could work(at
least in some configurations.  Also note that having different
definitions of inline functions leads to ODR violation) but I think this
cannot be changed any more.
>As was mentioned, Support/Debug.h might be another candidate.
>But IMHO the compile-time/execution-time/code-size impact of these are
>too small to justify the increase combinatorial complexity of
>configuration. At the last LLVM DevMtg we actually discussed how to
>*reduce* the number of independent configuration parameters.
>
>Michael