c++ stdlib assertions mode is weird

[q66]

Describe the bug

The pull request #12683 introduced passing assertion modes when ndebug is not set. IMO this is incorrect behavior as ndebug is supposed to control assertions in the project, not the stdlib. The stdlib assertions have their own default that can be overridden based on the specific toolchain build.

With the new behavior, meson will by default (especially for distributions that use the plain buildtype) override the toolchain's assertions mode, with no way to change this other than forcing ndebug, which in turn breaks intended cases in many projects. This is especially notable for clang libc++ starting from 18, as it forces the extensive assertions which may not be wanted by a lot of people.

It also taints the compiler commandline with a bunch of random new global macros that may or may not apply.

To Reproduce

Compile any C++ with meson after #12683.

Expected behavior

The toolchain assertions mode should be respected by default, and any buildsystem-level control over assertions should be at most a separate parameter.

@thesamesam

[eli-schwartz]

IMO this is incorrect behavior as ndebug is supposed to control assertions in the project, not the stdlib.

???

Feels like you are disagreeing with the stdlib documentation here. The fact that it controls CXX assertions in your project was the entire reason I figured that PR was a good idea to merge.

No, it doesn't control the use of assert()... but it does control whether the stdlib checks your inputs. Your inputs == asserting your project code. I fail to see why it's suddenly bad to have your inputs checked just because you're using stdlib containers, but okay to have your inputs checked if you write your own with asserts and all.

[thesamesam]

I'm fine with it being a separate parameter - just nobody suggested it until now. It's a shame this wasn't reported during the RC period.

IMO this is incorrect behavior as ndebug is supposed to control assertions in the project, not the stdlib.

-Db_ndebug is documented as "Disable asserts". I think you can argue it either way.

as it forces the extensive assertions which may not be wanted by a lot of people.

See below. We can also change which of the libc++ hardening modes is used as well, as another option to proceed here.

I guess my main question is: Why can't these people then disable asserts in general? Is there really a group of people who really want their own asserts, but want absolutely none from anywhere else? They want sanity checking, but not too much sanity checking based on their input? Just their own?

I did also bring this up with a C++ stdlib maintainer who didn't raise any concerns.

The stdlib assertions have their own default that can be overridden based on the specific toolchain build.

I'm not sure what you mean here -- do you mean cpp_debugstl? We differentiated that by ABI breaking-or-not (and for libc++, we chose to put the most expensive ones there, even though they don't break ABI; for libstdc++, they do).

The options we have are at https://libcxx.llvm.org/Hardening.html#using-hardening-modes.

Right now, we do -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE for no-n_debug. I think it'd be reasonable to change it to fast mode if people want that. I went back and forth on this a few times.

random new global macros that may or may not apply

They're not random. We also tried to only apply them when they're likely relevant.

[q66]

I'm fine with it being a separate parameter - just nobody suggested it until now. It's a shame this wasn't reported during the RC period.

IMO this is incorrect behavior as ndebug is supposed to control assertions in the project, not the stdlib.

-Db_ndebug is documented as "Disable asserts". I think you can argue it either way.

as it forces the extensive assertions which may not be wanted by a lot of people.

See below. We can also change which of the libc++ hardening modes is used as well, as another option to proceed here.

I guess my main question is: Why can't these people then disable asserts in general? Is there really a group of people who really want their own asserts, but want absolutely none from anywhere else? They want sanity checking, but not too much sanity checking based on their input? Just their own?

This is because there are many meson projects that rely on ndebug being false (because that's meson's default for release profile), sometimes to work at all, sometimes to pass tests (as they use assert() to do the testing checks). That means for those projects, it becomes unwieldy. There are several distros that have been contemplating the tradeoff between "set ndebug to true by default and deal with the fallout" and "just leave it alone" and generally everyone has leaned towards the latter.

I did also bring this up with a C++ stdlib maintainer who didn't raise any concerns.

The stdlib assertions have their own default that can be overridden based on the specific toolchain build.

I'm not sure what you mean here -- do you mean cpp_debugstl? We differentiated that by ABI breaking-or-not (and for libc++, we chose to put the most expensive ones there, even though they don't break ABI; for libstdc++, they do).

I'm talking about configuring your toolchain with C++ hardening by default (e.g. compiling LLVM with -DLIBCXX_HARDENING_MODE=...). Meson is effectively overriding these defaults now.

The options we have are at https://libcxx.llvm.org/Hardening.html#using-hardening-modes.

Right now, we do -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE for no-n_debug. I think it'd be reasonable to change it to fast mode if people want that. I went back and forth on this a few times.

random new global macros that may or may not apply

They're not random. We also tried to only apply them when they're likely relevant.

For clang it's applying both libc++ and libstdc++ macros at the same time, regardless of which library is in use, as far as i can tell. Funnily, for GCC it does not, even though GCC also supports both.

[q66]

IMO this is incorrect behavior as ndebug is supposed to control assertions in the project, not the stdlib.

???

Feels like you are disagreeing with the stdlib documentation here. The fact that it controls CXX assertions in your project was the entire reason I figured that PR was a good idea to merge.

No, it doesn't control the use of assert()... but it does control whether the stdlib checks your inputs. Your inputs == asserting your project code. I fail to see why it's suddenly bad to have your inputs checked just because you're using stdlib containers, but okay to have your inputs checked if you write your own with asserts and all.

What? -DNDEBUG very explicitly controls whether assert() becomes noop, and until now that was the entire purpose of b_ndebug in meson (https://pubs.opengroup.org/onlinepubs/009696799/basedefs/assert.h.html : "If NDEBUG is defined as a macro name before the inclusion of this header, the assert() macro shall be defined simply as:"). Now it's being conflated with extra C++ stdlib checks, which people may or may not want. Meanwhile, setting b_ndebug for meson is nearly always a bad idea because of what it defaults to for release mode, which has resulted in many projects having specific expecations.

[eli-schwartz]

This is because there are many meson projects that rely on ndebug being false (because that's meson's default for release profile),

Can you show me where meson makes a decision about the default value returned by get_option('b_ndebug') based on a release profile?

If you want to distinguish asserts based on buildtype you can use project(default_options: ['b_ndebug=if-release']). That's not meson's default for release profile or any profile but I think you actually want it to be without realizing it -- since that would of course mean ndebug is true, for the release profile.

There are several distros that have been contemplating the tradeoff between "set ndebug to true by default and deal with the fallout" and "just leave it alone" and generally everyone has leaned towards the latter.

Right, the correct solution is to check whether upstream does in fact rely on asserts being always-active, and if not, ask them to set b_ndebug=if-release so that building for release disables asserts.

No, it doesn't control the use of assert()... but it does control whether the stdlib checks your inputs. Your inputs == asserting your project code. I fail to see why it's suddenly bad to have your inputs checked just because you're using stdlib containers, but okay to have your inputs checked if you write your own with asserts and all.

What? -DNDEBUG very explicitly controls whether assert() becomes noop, and until now that was the entire purpose of b_ndebug in meson

Disregarding the incidental quotation of the POSIX manual, I would like to reiterate that I understand -D_GLIBCXX_ASSERTIONS doesn't control the literal function assert() but it does control "assertions" which are "in your code" and not "in the stdlib" because they are, in fact, asserting things about your inputs.

Given the meson option "b_ndebug" isn't documented to "pass the -DNDEBUG flag to the compiler" but rather is documented to "disable assertions" I don't think it's quite reasonable to say that this change has somehow violated the meson contract. The flag is controlling whether assertions are used. Whether they are the assert() function or merely the moral equivalent feels like splitting hairs.

[eli-schwartz]

I'm talking about configuring your toolchain with C++ hardening by default (e.g. compiling LLVM with -DLIBCXX_HARDENING_MODE=...). Meson is effectively overriding these defaults now.

Are you configuring your toolchain by default with the debug setting? If so, why? If not, how is your default being overridden -- any more than someone configuring with no hardening at all, is having their default overridden.

If you just want MODE_FAST to be used instead of MODE_EXTENSIVE, then @thesamesam has already agreed it may make sense to switch to that.

[eli-schwartz]

For clang it's applying both libc++ and libstdc++ macros at the same time, regardless of which library is in use, as far as i can tell. Funnily, for GCC it does not, even though GCC also supports both.

There is an inline code comment about that:

        # XXX: This needs updating if/when GCC starts to support libc++.
        # It currently only does so via an experimental configure arg.

[q66]

I'm talking about configuring your toolchain with C++ hardening by default (e.g. compiling LLVM with -DLIBCXX_HARDENING_MODE=...). Meson is effectively overriding these defaults now.

Are you configuring your toolchain by default with the debug setting? If so, why? If not, how is your default being overridden -- any more than someone configuring with no hardening at all, is having their default overridden.

If you just want MODE_FAST to be used instead of MODE_EXTENSIVE, then @thesamesam has already agreed it may make sense to switch to that.

That's not a debug option, it's a legitimate default that can be set and generally should be set. Your stdlib will then default to assertions on without requiring any explicit macros. You can still specify those macros to change it for the current build though.

[q66]

For clang it's applying both libc++ and libstdc++ macros at the same time, regardless of which library is in use, as far as i can tell. Funnily, for GCC it does not, even though GCC also supports both.

There is an inline code comment about that:

        # XXX: This needs updating if/when GCC starts to support libc++.
        # It currently only does so via an experimental configure arg.

GCC has supported -stdlib=libc++ for a couple releases now. It has nothing to do with configure args, it can simply choose another toolchain's libc++ just like Clang can do the otherwise.

[thesamesam]

For clang it's applying both libc++ and libstdc++ macros at the same time, regardless of which library is in use, as far as i can tell. Funnily, for GCC it does not, even though GCC also supports both.

There is an inline code comment about that:

        # XXX: This needs updating if/when GCC starts to support libc++.
        # It currently only does so via an experimental configure arg.

GCC has supported -stdlib=libc++ for a couple releases now. It has nothing to do with configure args, it can simply choose another toolchain's libc++ just like Clang can do the otherwise.

That's not right:

$ gcc -stdlib=
gcc: error: command-line option ‘-stdlib=’ is not supported by this configuration
gcc: fatal error: no input files
compilation terminated.

gcc_gxx_libcxx_include_dir_add_sysroot=0
gcc_enable_stdlib_opt=0
if test x${gcc_gxx_libcxx_include_dir} != x; then
  if test x${gcc_gxx_libcxx_include_dir} = xno; then
    # set defaults for the dir, but the option is disabled anyway.
    gcc_gxx_libcxx_include_dir=
  else
    gcc_enable_stdlib_opt=1
  fi
else
  case $target in
    *-darwin1[[1-9]]* | *-darwin2*)
       # Default this on for Darwin versions which default to libcxx,
       # and embed the path in the compiler install so that we get a
       # self-contained toolchain.
       gcc_enable_stdlib_opt=1
       ;;
    *) ;;
  esac
fi
AC_DEFINE_UNQUOTED(ENABLE_STDLIB_OPTION, $gcc_enable_stdlib_opt,
          [Define if the -stdlib= option should be enabled.])
[...]
AC_DEFINE_UNQUOTED(ENABLE_STDLIB_OPTION, $gcc_enable_stdlib_opt,

And then:

gcc/c-family/c.opt:2661:Driver C++ ObjC++ Common Condition(ENABLE_STDLIB_OPTION) Var(flag_stdlib_kind) Joined Enum(stdlib_kind) RejectNegative Init(1)

[q66]

This is because there are many meson projects that rely on ndebug being false (because that's meson's default for release profile),

Can you show me where meson makes a decision about the default value returned by get_option('b_ndebug') based on a release profile?

You are being intentionally obtuse here and I do not appreciate that. Meson's b_ndebug is false by default, regardless of the profile. That means projects end up relying on assertions not being no-op. That means setting b_ndebug to true to eliminate meson's newly-changed stuff also has the side effect on making asserts no-op, which breaks the projects, so it's not an option.

If you want to distinguish asserts based on buildtype you can use project(default_options: ['b_ndebug=if-release']). That's not meson's default for release profile or any profile but I think you actually want it to be without realizing it -- since that would of course mean ndebug is true, for the release profile.

There are several distros that have been contemplating the tradeoff between "set ndebug to true by default and deal with the fallout" and "just leave it alone" and generally everyone has leaned towards the latter.

Right, the correct solution is to check whether upstream does in fact rely on asserts being always-active, and if not, ask them to set b_ndebug=if-release so that building for release disables asserts.

That's entirely orthogonal to what is being discussed here.

No, it doesn't control the use of assert()... but it does control whether the stdlib checks your inputs. Your inputs == asserting your project code. I fail to see why it's suddenly bad to have your inputs checked just because you're using stdlib containers, but okay to have your inputs checked if you write your own with asserts and all.

What? -DNDEBUG very explicitly controls whether assert() becomes noop, and until now that was the entire purpose of b_ndebug in meson

Disregarding the incidental quotation of the POSIX manual, I would like to reiterate that I understand -D_GLIBCXX_ASSERTIONS doesn't control the literal function assert() but it does control "assertions" which are "in your code" and not "in the stdlib" because they are, in fact, asserting things about your inputs.

Given the meson option "b_ndebug" isn't documented to "pass the -DNDEBUG flag to the compiler" but rather is documented to "disable assertions" I don't think it's quite reasonable to say that this change has somehow violated the meson contract. The flag is controlling whether assertions are used. Whether they are the assert() function or merely the moral equivalent feels like splitting hairs.

Regardless of how it's documented, the practical semantics changed with the new release, in a bad way.

[q66]

For clang it's applying both libc++ and libstdc++ macros at the same time, regardless of which library is in use, as far as i can tell. Funnily, for GCC it does not, even though GCC also supports both.

There is an inline code comment about that:

        # XXX: This needs updating if/when GCC starts to support libc++.
        # It currently only does so via an experimental configure arg.

GCC has supported -stdlib=libc++ for a couple releases now. It has nothing to do with configure args, it can simply choose another toolchain's libc++ just like Clang can do the otherwise.

That's not right:

Okay, noted. That said, it does not change my point, meson could easily emit the same macros regardless, especially considering you're emitting the libstdc++ one for clang without checking anything.

[nekopsykose]

assuming you want meson to be helpful in setting the cpp stdlib assert macros, this is best done in a separate variable. but then the issue is that a project can have a legit reason to (for libc++) pick any of the 4 values. for glibcxx, there are also technically 4 values (not set, _GLIBCXX_ASSERTIONS, _GLIBCXX_DEBUG, _GLIBCXX_DEBUG_PEDANTIC).

the crux of the issue is summarised by q66:

That means projects end up relying on assertions not being no-op. That means setting b_ndebug to true to eliminate meson's newly-changed stuff also has the side effect on making asserts no-op, which breaks the projects

effectively, this is overloading the previous value to change multiple distinct things at once. for a project that wants any combination of values here, except for the specific one b_ndebug=false or =true gives them now, this attribute is doing something they don't want and has no usable option to choose. you either get overriden stdlib assertions you might not want (what if the project wants =debug specifically for tests?), or a forced DNDEBUG they don't want. there's no opt-out.

[nekopsykose]

there's no opt-out.

for our downstream distro side, we will default libc++18 to =fast. so in the case where (for some projects) we were setting b_ndebug=true, there is no change, but for the remainder of projects, this is now forcibly changed to =extensive. the only thing we can do is carry a revert (assuming this isn't changed in some way).

[nekopsykose]

for our downstream distro side, we will default libc++18 to =fast.

and since it was mentioned above this might be changed to be =fast by default, the same issue would then happen to a downstream that instead wanted to default to =extensive. there's no way to map a simple boolean for the options here, as long as everyone agrees this sort of granular configuration is wanted (which for us it is).

if it's not wanted to be granular, then i guess everything i said is moot

[eli-schwartz]

You are being intentionally obtuse here and I do not appreciate that.

Consider the feeling mutual, then.

Meson's b_ndebug is false by default, regardless of the profile. That means projects end up relying on assertions not being no-op.

Some projects do, yes.

It's not because it's the meson default, it's because it's the compiler default. Many of those same projects originally used autotools, and still relied on asserts at runtime.

Again, the answer here is to teach best practices and/or review project usage and encourage the use of b_ndebug=if-release. But mostly, the answer here is to teach projects that there is something to be knowledgeable about at all.

It's important because meson's default profile is debug and you have no way of knowing whether the project was ever tested with -D buildtype=release at all.

So even if meson changed it to default to if-release, you still cannot really rely on it!

Regardless of how it's documented, the practical semantics changed with the new release, in a bad way.

It is certainly true that the practical semantics changed. It is less definite that it did so in a bad way.

The change results in the default experience for users being more consistency and code-correctness checks, in ways that you are already doing -- you are merely debating the intensity.

Regarding intensity, it has already been agreed that it's reasonable to change that to =fast if people would prefer that.

You've also (both) spent a lot of time arguing about how it's important to not override the libcxx configuration / CXXFLAGS / whatnot but mostly managed to drown yourselves out by carrying on as though defaulting to check code to make sure it actually works is the end of the world (despite you yourself doing it too). This is a pity, because the actual core point is indeed a good one and @thesamesam already started work on checking the default mode to avoid trampling on preexisting intensities.

We can have our cake and eat it too. You don't need to pretend that the PR has to be reverted and forgotten about forever, to the detriment of all users everywhere.

[q66]

i'm not asking to switch the default to fast (that has the same problem, just in a different way - somebody that has their setup configured to default to stricter checks will suddenly find theirs being dehardened), i'm saying that there should be a way to avoid having meson mess with these while still providing the same NDEBUG semantics as before; i'm not even asking for the PR to be reverted, i'm asking for an option (i really don't care how it's done, if it's to be a separate meson option to disable this, so be it, right now there is none except -Db_ndebug=true which, as i've already explained, has problems)

otherwise, i will carry a downstream patch reverting the behavior for as long as it's needed