<div dir="ltr">Thanks Eric.  A deeper comment would be welcomed.<div><br></div><div>I don't use LIT to run the tests unfortunately.  My needs are bespoke so I have a custom framework for running them.  Since this issue is particular to me I won't push on the #define issue.  No point polluting the code for one person.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Aug 8, 2015 at 12:15 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">Hi Andrew,<br>
<br>
Please see this patch <a href="http://reviews.llvm.org/D11046" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11046</a>. That patch adds<br>
a more verbose comment that at least explains the point of the test.<br>
It doesn't describe how the expected behavior is achieved. I'll add<br>
that before submitting.<br>
<br>
I prefer not to use '#if _LIBCPP_STD_VER` to wrap C++11/C++14 only<br>
tests. Instead I prefer to use LIT's 'REQUIRES', 'UNSUPPORTED' and<br>
'XFAIL' features. This might be inconvenient if you don't use LIT to<br>
run the test suite.<br>
<br>
Libc++ also tends to back-port support for things like `wait_for` to<br>
older standard dialects. I wouldn't want libc++ to lose test coverage<br>
of these back-ported extensions. Hopefully we can work towards a<br>
solution that allows non-standard tests to be disabled without libc++<br>
losing coverage.<br>
<br>
/Eric<br>
<div><div class="h5"><br>
<br>
<br>
<br>
<br>
On Fri, Aug 7, 2015 at 5:13 AM, Andrew Parker<br>
<<a href="mailto:andrew.j.c.parker@gmail.com">andrew.j.c.parker@gmail.com</a>> wrote:<br>
> I've just spent a lot of time trying to figure out why a particular class of<br>
> tests were failing for me with MSVC and not with GCC.<br>
><br>
> For example:<br>
><br>
> test\std\thread\thread.condition\thread.condition.condvarany\wait_for.exception.pass.cpp<br>
><br>
> These tests are designed to check that std::condition_variable::wait_for<br>
> (and friends) will call std::terminate when an exception is thrown during<br>
> the re-locking of the passed in mutex.<br>
><br>
> Firstly, I think this is C++14 only, at least according to<br>
> <a href="http://en.cppreference.com/w/cpp/thread/condition_variable/wait_for" rel="noreferrer" target="_blank">http://en.cppreference.com/w/cpp/thread/condition_variable/wait_for</a>.  I know<br>
> that's not the single source of truth so please correct me if that's not the<br>
> case.  If it is, then I'd like to comment out these tests in a:<br>
><br>
> #if _LIBCPP_STD_VER > 11<br>
> #else<br>
> #endif<br>
><br>
> block.<br>
><br>
> Secondly, I'd like to add a much more verbose comment as to what these tests<br>
> are doing and why, ultimately, they will work.  The latter point is the more<br>
> important.  To implement the expected behaviour the tests rely on the dtr of<br>
> std::unique_ptr calling a nothrow function and the mutex locking happening<br>
> from within there (this is why MSVC failed the test as it doesn't call<br>
> std::terminate if a throw occurs in a nothrow function).<br>
><br>
> I'll submit a patch (or not) based on comments.<br>
><br>
> Also, on a side not whilst I remember.  The following functions in<br>
> condition_variable.cpp are marked as NOTHROW but actually can throw:<br>
><br>
> void<br>
> condition_variable::wait(unique_lock<mutex>& lk)<br>
><br>
> void<br>
> condition_variable::__do_timed_wait(unique_lock<mutex>& lk,<br>
>      chrono::time_point<chrono::system_clock, chrono::nanoseconds> tp)<br>
><br>
> Seems like a bug to me!  Is it?<br>
><br>
</div></div>> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
><br>
</blockquote></div><br></div>