Page MenuHomePhabricator

[libcxx] Never use <cassert> within libc++

Authored by EricWF on Jan 23 2017, 5:39 PM.



It is my opinion that libc++ should never use <cassert>, including in the dylib. This patch remove all uses of assert from within libc++ and replaces most of them with _LIBCPP_ASSERT instead.

Additionally this patch turn LIBCXX_ENABLE_ASSERTIONS off by default, because the standard library should not be aborting user programs unless explicitly asked to.

Diff Detail

Event Timeline

EricWF created this revision.Jan 23 2017, 5:39 PM
mclow.lists accepted this revision.Jan 23 2017, 8:33 PM

this LGTM. assert is not something we should have in the dylib.


Does this belong here?

This revision is now accepted and ready to land.Jan 23 2017, 8:33 PM
EricWF added inline comments.Jan 23 2017, 8:36 PM

Yeah, so -DLIBCXX_ENABLE_ASSERTIONS=ON now define -D_LIBCPP_DEBUG=0 during the build. This suppresses std::strings extern template declarations and changes the export lists of libc++.dylib. This change ensures that enabling debug mode during the build doesn't do that.

EricWF closed this revision.Jan 23 2017, 9:08 PM

@EricWF: My downstream builds have -DLLVM_ENABLE_ASSERTIONS=ON by default and they are showing some failures after this commit. Log attached:

It should be pretty easy to reproduce these (I'm on Ubuntu 16.04) if you enable assertions. Since the default has changed, the public builders won't be seeing these.

Are these failures expected?


/ Asiri


It seems like weird usages of _LIBCPP_ASSERT in <string_view> are causing this problem.
Specifically the usages in <string_view> attempt to use _LIBCPP_ASSERT in C++11 constexpr functions and this currently does not work.

For now I've reverted this change in r292923.