<div dir="ltr">Hi Jonas,<div><br></div><div>I fixed the filesystem test failures in r294360. You should be able to re-enable</div><div>LIBCXX_ENABLE_ASSERTIONS.</div><div><br></div><div>/Eric</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 6, 2017 at 3:25 AM, Eric Fiselier <span dir="ltr"><<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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!<div>On initial inspection in appears that the new path parser implementation has some issues. I'll work on fixing them in the coming days.</div><div>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. </div><div><br></div><div>Thanks,</div><div><br></div><div>/Eric</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 6, 2017 at 3:10 AM, Hahnfeld, Jonas <span dir="ltr"><<a href="mailto:Hahnfeld@itc.rwth-aachen.de" target="_blank">Hahnfeld@itc.rwth-aachen.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Eric,<br>
<br>
I'm getting quite a few failures with LIBCXX_ENABLE_ASSERTIONS=On (didn't do a<br>
clean build at first):<br>
Failing Tests (32):<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_entry/directory_<wbr>entry.cons.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_entry/directory_<wbr>entry.mods.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_entry/directory_<wbr>entry.obs/comparisons.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_iterator/directo<wbr>ry_iterator.members/ctor.pass.<wbr>cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_iterator/directo<wbr>ry_iterator.members/increment.<wbr>pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.directory_iterator/directo<wbr>ry_iterator.nonmembers/begin_<wbr>end.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.itr/iterator.pas<wbr>s.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.appe<wbr>nd.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.assi<wbr>gn/source.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.comp<wbr>are.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.conc<wbr>at.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.deco<wbr>mpose/path.decompose.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.modi<wbr>fiers/make_preferred.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.modi<wbr>fiers/remove_filename.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.modi<wbr>fiers/replace_extension.pass.<wbr>cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.path/path.member/path.modi<wbr>fiers/swap.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.rec.dir.itr/<a href="http://rec.dir.itr.me">rec.dir.itr.me</a><wbr>mbers/ctor.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.rec.dir.itr/<a href="http://rec.dir.itr.me">rec.dir.itr.me</a><wbr>mbers/depth.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.rec.dir.itr/<a href="http://rec.dir.itr.me">rec.dir.itr.me</a><wbr>mbers/increment.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/cl<wbr>ass.rec.dir.itr/<a href="http://rec.dir.itr.no">rec.dir.itr.no</a><wbr>nmembers/begin_end.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.canonical/cano<wbr>nical.pass.cpp<br>
libc++ :: std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.copy/copy.pass<wbr>.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.is_empty/is_em<wbr>pty.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.permissions/pe<wbr>rmissions.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.read_symlink/<wbr>read_symlink.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.remove/remove.<wbr>pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.remove_all/rem<wbr>ove_all.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.resize_file/re<wbr>size_file.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.space/<a href="http://space.pa">space.pa</a><wbr>ss.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.status/status.<wbr>pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.symlink_status<wbr>/symlink_status.pass.cpp<br>
libc++ ::<br>
std/experimental/filesystem/fs<wbr>.op.funcs/fs.op.temp_dir_path/<wbr>temp_directory_path.pass.cpp<br>
<br>
They go away with LIBCXX_ENABLE_ASSERTIONS=Off which is the new default. Is<br>
this expected to happen?<br>
<br>
Thanks,<br>
Jonas<br>
<div><div class="m_-1459659893500225072h5"><br>
> -----Original Message-----<br>
> From: cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@li<wbr>sts.llvm.org</a>] On Behalf<br>
> Of Eric Fiselier via cfe-commits<br>
> Sent: Sunday, February 05, 2017 12:22 AM<br>
> To: <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> Subject: [libcxx] r294107 - Recommit [libcxx] Never use <cassert> within<br>
> libc++<br>
><br>
> Author: ericwf<br>
> Date: Sat Feb 4 17:22:28 2017<br>
> New Revision: 294107<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294107&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=294107&view=rev</a><br>
> Log:<br>
> Recommit [libcxx] Never use <cassert> within libc++<br>
><br>
> It is my opinion that libc++ should never use `<cassert>`, including in the<br>
> `dylib`.<br>
> This patch remove all uses of `assert` from within libc++ and replaces most<br>
> of<br>
> them with `_LIBCPP_ASSERT` instead.<br>
><br>
> Additionally this patch turn `LIBCXX_ENABLE_ASSERTIONS` off by default,<br>
> because the standard library should not be aborting user programs unless<br>
> explicitly asked to.<br>
><br>
> Modified:<br>
> libcxx/trunk/CMakeLists.txt<br>
> libcxx/trunk/include/__config<br>
> libcxx/trunk/include/__thread<wbr>ing_support<br>
> libcxx/trunk/src/condition_va<wbr>riable.cpp<br>
> libcxx/trunk/src/<wbr>experimental/filesystem/path.<wbr>cpp<br>
> libcxx/trunk/src/mutex.cpp<br>
> libcxx/trunk/src/system_<wbr>error.cpp<br>
><br>
> Modified: libcxx/trunk/CMakeLists.txt<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/CMakeList<wbr>s.txt?rev=294107&r1=294106&r2=<wbr>294107&vie<br>
> w=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/CMakeLists.txt (original)<br>
> +++ libcxx/trunk/CMakeLists.txt Sat Feb 4 17:22:28 2017<br>
> @@ -60,7 +60,7 @@ endif()<br>
> include(CMakeDependentOption)<br>
><br>
> # Basic<br>
> options ------------------------------<wbr>------------------------------<wbr>---<br>
> -option(LIBCXX_ENABLE_ASSERTIO<wbr>NS "Enable assertions independent of<br>
> build mode." ON)<br>
> +option(LIBCXX_ENABLE_ASSERTIO<wbr>NS "Enable assertions independent of<br>
> build<br>
</div></div>> +mode." OFF)<br>
<div><div class="m_-1459659893500225072h5">> option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)<br>
> option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)<br>
> option(LIBCXX_ENABLE_EXPERIMEN<wbr>TAL_LIBRARY "Build<br>
> libc++experimental.a" ON) @@ -501,6 +501,7 @@ endif() # Assertion flags<br>
> ==============================<wbr>============================<br>
> ===<br>
> define_if(LIBCXX_ENABLE_ASSERT<wbr>IONS -UNDEBUG)<br>
> define_if_not(LIBCXX_ENABLE_AS<wbr>SERTIONS -DNDEBUG)<br>
> +define_if(LIBCXX_ENABLE_ASSER<wbr>TIONS -D_LIBCPP_DEBUG=0)<br>
> define_if(LIBCXX_DEBUG_BUILD -D_DEBUG)<br>
> if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_DEBUG_BUILD)<br>
> # MSVC doesn't like _DEBUG on release builds. See PR 4379.<br>
><br>
> Modified: libcxx/trunk/include/__config<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/include/_<wbr>_config?rev=294107&r1=294106&r<wbr>2=294107&v<br>
> iew=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/include/__config (original)<br>
> +++ libcxx/trunk/include/__config Sat Feb 4 17:22:28 2017<br>
> @@ -817,7 +817,9 @@ template <unsigned> struct __static_asse<br>
> # else<br>
> # error Supported values for _LIBCPP_DEBUG are 0 and 1<br>
> # endif<br>
> +# if !defined(_LIBCPP_BUILDING_LIBR<wbr>ARY)<br>
> # define _LIBCPP_EXTERN_TEMPLATE(...)<br>
> +# endif<br>
> #endif<br>
><br>
> #ifndef _LIBCPP_EXTERN_TEMPLATE<br>
><br>
> Modified: libcxx/trunk/include/__threadi<wbr>ng_support<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/include/_<wbr>_threading_support?rev=294107&<wbr>r1=294106&<br>
> r2=294107&view=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/include/__threadi<wbr>ng_support (original)<br>
> +++ libcxx/trunk/include/__threadi<wbr>ng_support Sat Feb 4 17:22:28 2017<br>
> @@ -25,7 +25,6 @@<br>
> # include <pthread.h><br>
> # include <sched.h><br>
> #elif defined(_LIBCPP_HAS_THREAD_API<wbr>_WIN32)<br>
> -#include <assert.h><br>
> #include <Windows.h><br>
> #include <process.h><br>
> #include <fibersapi.h><br>
><br>
> Modified: libcxx/trunk/src/condition_var<wbr>iable.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/src/condi<wbr>tion_variable.cpp?rev=294107&<wbr>r1=294106&r2=<br>
> 294107&view=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/src/condition_var<wbr>iable.cpp (original)<br>
> +++ libcxx/trunk/src/condition_var<wbr>iable.cpp Sat Feb 4 17:22:28 2017<br>
> @@ -14,7 +14,6 @@<br>
> #include "condition_variable"<br>
> #include "thread"<br>
> #include "system_error"<br>
> -#include "cassert"<br>
><br>
> _LIBCPP_BEGIN_NAMESPACE_STD<br>
><br>
><br>
> Modified: libcxx/trunk/src/experimental/<wbr>filesystem/path.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/src/exper<wbr>imental/filesystem/path.cpp?<wbr>rev=294107&r1=<br>
> 294106&r2=294107&view=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/src/experimental/<wbr>filesystem/path.cpp (original)<br>
> +++ libcxx/trunk/src/experimental/<wbr>filesystem/path.cpp Sat Feb 4<br>
</div></div>> +++ 17:22:28 2017<br>
<div><div class="m_-1459659893500225072h5">> @@ -6,11 +6,9 @@<br>
> // Source Licenses. See LICENSE.TXT for details.<br>
> //<br>
><br>
> //===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
> -#undef NDEBUG<br>
> #include "experimental/filesystem"<br>
> #include "string_view"<br>
> #include "utility"<br>
> -#include "cassert"<br>
><br>
> namespace { namespace parser<br>
> {<br>
> @@ -113,7 +111,6 @@ public:<br>
> void decrement() noexcept {<br>
> const PosPtr REnd = &Path.front() - 1;<br>
> const PosPtr RStart = getCurrentTokenStartPos() - 1;<br>
> - assert(RStart != REnd);<br>
><br>
> switch (State) {<br>
> case PS_AtEnd: {<br>
> @@ -322,7 +319,6 @@ string_view_t path::__root_path_raw() co<br>
> auto NextCh = PP.peek();<br>
> if (NextCh && *NextCh == '/') {<br>
> ++PP;<br>
> - assert(PP.State == PathParser::PS_InRootDir);<br>
> return createView(__pn_.data(), &PP.RawEntry.back());<br>
> }<br>
> return PP.RawEntry;<br>
><br>
> Modified: libcxx/trunk/src/mutex.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/src/mutex<wbr>.cpp?rev=294107&r1=294106&r2=<wbr>294107&vie<br>
> w=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/src/mutex.cpp (original)<br>
> +++ libcxx/trunk/src/mutex.cpp Sat Feb 4 17:22:28 2017<br>
> @@ -11,7 +11,6 @@<br>
> #include "mutex"<br>
> #include "limits"<br>
> #include "system_error"<br>
> -#include "cassert"<br>
> #include "include/atomic_support.h"<br>
><br>
> _LIBCPP_BEGIN_NAMESPACE_STD<br>
> @@ -45,7 +44,7 @@ mutex::unlock() _NOEXCEPT {<br>
> int ec = __libcpp_mutex_unlock(&__m_);<br>
> (void)ec;<br>
> - assert(ec == 0);<br>
> + _LIBCPP_ASSERT(ec == 0, "call to mutex::unlock failed");<br>
> }<br>
><br>
> // recursive_mutex<br>
> @@ -61,7 +60,7 @@ recursive_mutex::~recursive_mu<wbr>tex()<br>
> {<br>
> int e = __libcpp_recursive_mutex_destr<wbr>oy(&__m_);<br>
> (void)e;<br>
> - assert(e == 0);<br>
> + _LIBCPP_ASSERT(e == 0, "call to ~recursive_mutex() failed");<br>
> }<br>
><br>
> void<br>
> @@ -77,7 +76,7 @@ recursive_mutex::unlock() _NOEXCEPT {<br>
> int e = __libcpp_recursive_mutex_unloc<wbr>k(&__m_);<br>
> (void)e;<br>
> - assert(e == 0);<br>
> + _LIBCPP_ASSERT(e == 0, "call to recursive_mutex::unlock() failed");<br>
> }<br>
><br>
> bool<br>
><br>
> Modified: libcxx/trunk/src/system_error.<wbr>cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-</a><br>
> project/libcxx/trunk/src/syste<wbr>m_error.cpp?rev=294107&r1=<wbr>294106&r2=2941<br>
> 07&view=diff<br>
> ==============================<wbr>============================<br>
> ====================<br>
> --- libcxx/trunk/src/system_error.<wbr>cpp (original)<br>
> +++ libcxx/trunk/src/system_error.<wbr>cpp Sat Feb 4 17:22:28 2017<br>
> @@ -17,9 +17,9 @@<br>
> #include "cstring"<br>
> #include "cstdio"<br>
> #include "cstdlib"<br>
> -#include "cassert"<br>
> #include "string"<br>
> #include "string.h"<br>
> +#include "__debug"<br>
><br>
> #if defined(__ANDROID__)<br>
> #include <android/api-level.h><br>
> @@ -96,7 +96,7 @@ string do_strerror_r(int ev) {<br>
> std::snprintf(buffer, strerror_buff_size, "Unknown error %d",<br>
> ev);<br>
> return string(buffer);<br>
> } else {<br>
> - assert(new_errno == ERANGE);<br>
> + _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from<br>
</div></div>> + ::strerr_r");<br>
<div class="m_-1459659893500225072HOEnZb"><div class="m_-1459659893500225072h5">> // FIXME maybe? 'strerror_buff_size' is likely to exceed the<br>
> // maximum error size so ERANGE shouldn't be returned.<br>
> std::abort();<br>
><br>
><br>
> ______________________________<wbr>_________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>