[libcxx] r294107 - Recommit [libcxx] Never use <cassert> within libc++
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 6 02:25:11 PST 2017
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/
> directory_iterator.members/ctor.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.directory_iterator/
> directory_iterator.members/increment.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.directory_iterator/
> directory_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.
> assign/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.
> decompose/path.decompose.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.path/path.member/path.
> modifiers/make_preferred.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.path/path.member/path.
> modifiers/remove_filename.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.path/path.member/path.
> modifiers/replace_extension.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.path/path.member/path.
> modifiers/swap.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.
> members/ctor.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.
> members/depth.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.
> members/increment.pass.cpp
> libc++ ::
> std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.
> nonmembers/begin_end.pass.cpp
> libc++ ::
> std/experimental/filesystem/fs.op.funcs/fs.op.canonical/canonical.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/
> permissions.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/
> remove_all.pass.cpp
> libc++ ::
> std/experimental/filesystem/fs.op.funcs/fs.op.resize_file/
> resize_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/20170206/d31ce802/attachment-0001.html>
More information about the cfe-commits
mailing list