[libcxx] r294107 - Recommit [libcxx] Never use <cassert> within libc++

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 7 14:05:40 PST 2017


Hi Jonas,

I fixed the filesystem test failures in r294360. You should be able to
re-enable
LIBCXX_ENABLE_ASSERTIONS.

/Eric

On Mon, Feb 6, 2017 at 3:25 AM, Eric Fiselier <eric at efcs.ca> wrote:

> I should have made it more clear that this change actually caused 100's of
> assertions to be turned on. And good news! They found bugs!
> On initial inspection in appears that the new path parser implementation
> has some issues. I'll work on fixing them in the coming days.
> After I commit the fixes I'll setup bots to test with assertions enabled.
> For now I would suggest disabling assertions if you need the test suite to
> be clean.
>
> Thanks,
>
> /Eric
>
> On Mon, Feb 6, 2017 at 3:10 AM, Hahnfeld, Jonas <
> Hahnfeld at itc.rwth-aachen.de> wrote:
>
>> Hi Eric,
>>
>> I'm getting quite a few failures with LIBCXX_ENABLE_ASSERTIONS=On (didn't
>> do a
>> clean build at first):
>> Failing Tests (32):
>>     libc++ ::
>> std/experimental/filesystem/class.directory_entry/directory_
>> entry.cons.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.directory_entry/directory_
>> entry.mods.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.directory_entry/directory_
>> entry.obs/comparisons.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.directory_iterator/directo
>> ry_iterator.members/ctor.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.directory_iterator/directo
>> ry_iterator.members/increment.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.directory_iterator/directo
>> ry_iterator.nonmembers/begin_end.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.itr/iterator.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.append.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.assi
>> gn/source.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.compare.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.concat.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.deco
>> mpose/path.decompose.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.modi
>> fiers/make_preferred.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.modi
>> fiers/remove_filename.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.modi
>> fiers/replace_extension.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.path/path.member/path.modi
>> fiers/swap.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.me
>> mbers/ctor.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.me
>> mbers/depth.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.me
>> mbers/increment.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.no
>> nmembers/begin_end.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.canonical/cano
>> nical.pass.cpp
>>     libc++ :: std/experimental/filesystem/fs
>> .op.funcs/fs.op.copy/copy.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.is_empty/is_empty.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.permissions/pe
>> rmissions.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.read_symlink/
>> read_symlink.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/rem
>> ove_all.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.resize_file/re
>> size_file.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.space/space.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.status/status.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.symlink_status
>> /symlink_status.pass.cpp
>>     libc++ ::
>> std/experimental/filesystem/fs.op.funcs/fs.op.temp_dir_path/
>> temp_directory_path.pass.cpp
>>
>> They go away with LIBCXX_ENABLE_ASSERTIONS=Off which is the new default.
>> Is
>> this expected to happen?
>>
>> Thanks,
>> Jonas
>>
>> > -----Original Message-----
>> > From: cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org] On Behalf
>> > Of Eric Fiselier via cfe-commits
>> > Sent: Sunday, February 05, 2017 12:22 AM
>> > To: cfe-commits at lists.llvm.org
>> > Subject: [libcxx] r294107 - Recommit [libcxx] Never use <cassert> within
>> > libc++
>> >
>> > Author: ericwf
>> > Date: Sat Feb  4 17:22:28 2017
>> > New Revision: 294107
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=294107&view=rev
>> > Log:
>> > Recommit [libcxx] Never use <cassert> within libc++
>> >
>> > 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.
>> >
>> > Modified:
>> >     libcxx/trunk/CMakeLists.txt
>> >     libcxx/trunk/include/__config
>> >     libcxx/trunk/include/__threading_support
>> >     libcxx/trunk/src/condition_variable.cpp
>> >     libcxx/trunk/src/experimental/filesystem/path.cpp
>> >     libcxx/trunk/src/mutex.cpp
>> >     libcxx/trunk/src/system_error.cpp
>> >
>> > Modified: libcxx/trunk/CMakeLists.txt
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/CMakeLists.txt?rev=294107&r1=294106&r2=294107&vie
>> > w=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/CMakeLists.txt (original)
>> > +++ libcxx/trunk/CMakeLists.txt Sat Feb  4 17:22:28 2017
>> > @@ -60,7 +60,7 @@ endif()
>> >  include(CMakeDependentOption)
>> >
>> >  # Basic
>> > options ---------------------------------------------------------------
>> > -option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of
>> > build mode." ON)
>> > +option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of
>> > build
>> > +mode." OFF)
>> >  option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
>> > option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
>> > option(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY "Build
>> > libc++experimental.a" ON) @@ -501,6 +501,7 @@ endif()  # Assertion flags
>> > ==========================================================
>> > ===
>> >  define_if(LIBCXX_ENABLE_ASSERTIONS -UNDEBUG)
>> > define_if_not(LIBCXX_ENABLE_ASSERTIONS -DNDEBUG)
>> > +define_if(LIBCXX_ENABLE_ASSERTIONS -D_LIBCPP_DEBUG=0)
>> >  define_if(LIBCXX_DEBUG_BUILD -D_DEBUG)
>> >  if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_DEBUG_BUILD)
>> >    # MSVC doesn't like _DEBUG on release builds. See PR 4379.
>> >
>> > Modified: libcxx/trunk/include/__config
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/include/__config?rev=294107&r1=294106&r2=294107&v
>> > iew=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/include/__config (original)
>> > +++ libcxx/trunk/include/__config Sat Feb  4 17:22:28 2017
>> > @@ -817,7 +817,9 @@ template <unsigned> struct __static_asse
>> >  #   else
>> >  #       error Supported values for _LIBCPP_DEBUG are 0 and 1
>> >  #   endif
>> > +# if !defined(_LIBCPP_BUILDING_LIBRARY)
>> >  #   define _LIBCPP_EXTERN_TEMPLATE(...)
>> > +# endif
>> >  #endif
>> >
>> >  #ifndef _LIBCPP_EXTERN_TEMPLATE
>> >
>> > Modified: libcxx/trunk/include/__threading_support
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/include/__threading_support?rev=294107&r1=294106&
>> > r2=294107&view=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/include/__threading_support (original)
>> > +++ libcxx/trunk/include/__threading_support Sat Feb  4 17:22:28 2017
>> > @@ -25,7 +25,6 @@
>> >  # include <pthread.h>
>> >  # include <sched.h>
>> >  #elif defined(_LIBCPP_HAS_THREAD_API_WIN32)
>> > -#include <assert.h>
>> >  #include <Windows.h>
>> >  #include <process.h>
>> >  #include <fibersapi.h>
>> >
>> > Modified: libcxx/trunk/src/condition_variable.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/src/condition_variable.cpp?rev=294107&
>> r1=294106&r2=
>> > 294107&view=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/src/condition_variable.cpp (original)
>> > +++ libcxx/trunk/src/condition_variable.cpp Sat Feb  4 17:22:28 2017
>> > @@ -14,7 +14,6 @@
>> >  #include "condition_variable"
>> >  #include "thread"
>> >  #include "system_error"
>> > -#include "cassert"
>> >
>> >  _LIBCPP_BEGIN_NAMESPACE_STD
>> >
>> >
>> > Modified: libcxx/trunk/src/experimental/filesystem/path.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/src/experimental/filesystem/path.cpp?
>> rev=294107&r1=
>> > 294106&r2=294107&view=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/src/experimental/filesystem/path.cpp (original)
>> > +++ libcxx/trunk/src/experimental/filesystem/path.cpp Sat Feb  4
>> > +++ 17:22:28 2017
>> > @@ -6,11 +6,9 @@
>> >  // Source Licenses. See LICENSE.TXT for details.
>> >  //
>> >
>> > //===-------------------------------------------------------
>> ---------------===//
>> > -#undef NDEBUG
>> >  #include "experimental/filesystem"
>> >  #include "string_view"
>> >  #include "utility"
>> > -#include "cassert"
>> >
>> >  namespace { namespace parser
>> >  {
>> > @@ -113,7 +111,6 @@ public:
>> >    void decrement() noexcept {
>> >      const PosPtr REnd = &Path.front() - 1;
>> >      const PosPtr RStart = getCurrentTokenStartPos() - 1;
>> > -    assert(RStart != REnd);
>> >
>> >      switch (State) {
>> >      case PS_AtEnd: {
>> > @@ -322,7 +319,6 @@ string_view_t path::__root_path_raw() co
>> >        auto NextCh = PP.peek();
>> >        if (NextCh && *NextCh == '/') {
>> >          ++PP;
>> > -        assert(PP.State == PathParser::PS_InRootDir);
>> >          return createView(__pn_.data(), &PP.RawEntry.back());
>> >        }
>> >        return PP.RawEntry;
>> >
>> > Modified: libcxx/trunk/src/mutex.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/src/mutex.cpp?rev=294107&r1=294106&r2=294107&vie
>> > w=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/src/mutex.cpp (original)
>> > +++ libcxx/trunk/src/mutex.cpp Sat Feb  4 17:22:28 2017
>> > @@ -11,7 +11,6 @@
>> >  #include "mutex"
>> >  #include "limits"
>> >  #include "system_error"
>> > -#include "cassert"
>> >  #include "include/atomic_support.h"
>> >
>> >  _LIBCPP_BEGIN_NAMESPACE_STD
>> > @@ -45,7 +44,7 @@ mutex::unlock() _NOEXCEPT  {
>> >      int ec = __libcpp_mutex_unlock(&__m_);
>> >      (void)ec;
>> > -    assert(ec == 0);
>> > +    _LIBCPP_ASSERT(ec == 0, "call to mutex::unlock failed");
>> >  }
>> >
>> >  // recursive_mutex
>> > @@ -61,7 +60,7 @@ recursive_mutex::~recursive_mutex()
>> >  {
>> >      int e = __libcpp_recursive_mutex_destroy(&__m_);
>> >      (void)e;
>> > -    assert(e == 0);
>> > +    _LIBCPP_ASSERT(e == 0, "call to ~recursive_mutex() failed");
>> >  }
>> >
>> >  void
>> > @@ -77,7 +76,7 @@ recursive_mutex::unlock() _NOEXCEPT  {
>> >      int e = __libcpp_recursive_mutex_unlock(&__m_);
>> >      (void)e;
>> > -    assert(e == 0);
>> > +    _LIBCPP_ASSERT(e == 0, "call to recursive_mutex::unlock() failed");
>> >  }
>> >
>> >  bool
>> >
>> > Modified: libcxx/trunk/src/system_error.cpp
>> > URL: http://llvm.org/viewvc/llvm-
>> > project/libcxx/trunk/src/system_error.cpp?rev=294107&r1=294106&r2=2941
>> > 07&view=diff
>> > ==========================================================
>> > ====================
>> > --- libcxx/trunk/src/system_error.cpp (original)
>> > +++ libcxx/trunk/src/system_error.cpp Sat Feb  4 17:22:28 2017
>> > @@ -17,9 +17,9 @@
>> >  #include "cstring"
>> >  #include "cstdio"
>> >  #include "cstdlib"
>> > -#include "cassert"
>> >  #include "string"
>> >  #include "string.h"
>> > +#include "__debug"
>> >
>> >  #if defined(__ANDROID__)
>> >  #include <android/api-level.h>
>> > @@ -96,7 +96,7 @@ string do_strerror_r(int ev) {
>> >              std::snprintf(buffer, strerror_buff_size, "Unknown error
>> %d",
>> > ev);
>> >              return string(buffer);
>> >          } else {
>> > -            assert(new_errno == ERANGE);
>> > +            _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from
>> > + ::strerr_r");
>> >              // FIXME maybe? 'strerror_buff_size' is likely to exceed
>> the
>> >              // maximum error size so ERANGE shouldn't be returned.
>> >              std::abort();
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170207/144a6d62/attachment-0001.html>


More information about the cfe-commits mailing list