<div dir="ltr">@Jonas please go ahead and merge this patch.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 17, 2017 at 5:24 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, go ahead.<br>
<br>
Apologies for the delay.<br>
<span class="HOEnZb"><font color="#888888"><br>
- Hans<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Sat, Jan 14, 2017 at 5:54 AM, Eric Fiselier <<a href="mailto:eric@efcs.ca">eric@efcs.ca</a>> wrote:<br>
> +1 from me. @Hans am I OK to merge this?<br>
><br>
> On Sat, Jan 14, 2017 at 4:53 AM, Hahnfeld, Jonas<br>
> <<a href="mailto:Hahnfeld@itc.rwth-aachen.de">Hahnfeld@itc.rwth-aachen.de</a>> wrote:<br>
>><br>
>> Hi Hans,<br>
>><br>
>> can this be merged for 4.0? Eric suggested this in<br>
>> <a href="https://reviews.llvm.org/D22452" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D22452</a> so I think he should be fine.<br>
>><br>
>> Thanks,<br>
>> Jonas<br>
>><br>
>> Am Samstag, den 14.01.2017, 11:35 +0000 schrieb Jonas Hahnfeld via<br>
>> cfe-commits:<br>
>><br>
>> Author: hahnfeld<br>
>> Date: Sat Jan 14 05:35:15 2017<br>
>> New Revision: 292013<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=292013&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=292013&view=rev</a><br>
>> Log:<br>
>> Fix last_write_time tests for filesystems that don't support negative and<br>
>> very large times<br>
>><br>
>> Seems to be the case for NFS.<br>
>><br>
>> Original patch by Eric Fiselier!<br>
>> Differential Revision: <a href="https://reviews.llvm.org/D22452" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D22452</a><br>
>><br>
>> Modified:<br>
>><br>
>> libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp<br>
>><br>
>> Modified:<br>
>> libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp?rev=292013&r1=292012&r2=292013&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp?rev=<wbr>292013&r1=292012&r2=292013&<wbr>view=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> ---<br>
>> libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp<br>
>> (original)<br>
>> +++<br>
>> libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp<br>
>> Sat Jan 14 05:35:15 2017<br>
>> @@ -72,13 +72,60 @@ std::pair<std::time_t, std::time_t> GetS<br>
>> return {st.st_atime, st.st_mtime};<br>
>> }<br>
>><br>
>> -inline bool TimeIsRepresentableAsTimeT(<wbr>file_time_type tp) {<br>
>> +namespace {<br>
>> +bool TestSupportsNegativeTimes() {<br>
>> + using namespace std::chrono;<br>
>> + std::error_code ec;<br>
>> + std::time_t old_write_time, new_write_time;<br>
>> + { // WARNING: Do not assert in this scope.<br>
>> + scoped_test_env env;<br>
>> + const path file = env.create_file("file", 42);<br>
>> + old_write_time = LastWriteTime(file);<br>
>> + file_time_type tp(seconds(-5));<br>
>> + fs::last_write_time(file, tp, ec);<br>
>> + new_write_time = LastWriteTime(file);<br>
>> + }<br>
>> + return !ec && new_write_time <= -5;<br>
>> +}<br>
>> +<br>
>> +bool TestSupportsMaxTime() {<br>
>> using namespace std::chrono;<br>
>> using Lim = std::numeric_limits<std::time_<wbr>t>;<br>
>> - auto sec = duration_cast<seconds>(tp.<wbr>time_since_epoch()).count();<br>
>> - return (sec >= Lim::min() && sec <= Lim::max());<br>
>> + auto max_sec =<br>
>> duration_cast<seconds>(file_<wbr>time_type::max().time_since_<wbr>epoch()).count();<br>
>> + if (max_sec > Lim::max()) return false;<br>
>> + std::error_code ec;<br>
>> + std::time_t old_write_time, new_write_time;<br>
>> + { // WARNING: Do not assert in this scope.<br>
>> + scoped_test_env env;<br>
>> + const path file = env.create_file("file", 42);<br>
>> + old_write_time = LastWriteTime(file);<br>
>> + file_time_type tp = file_time_type::max();<br>
>> + fs::last_write_time(file, tp, ec);<br>
>> + new_write_time = LastWriteTime(file);<br>
>> + }<br>
>> + return !ec && new_write_time > max_sec - 1;<br>
>> }<br>
>><br>
>> +static const bool SupportsNegativeTimes = TestSupportsNegativeTimes();<br>
>> +static const bool SupportsMaxTime = TestSupportsMaxTime();<br>
>> +<br>
>> +} // end namespace<br>
>> +<br>
>> +// Check if a time point is representable on a given filesystem. Check<br>
>> that:<br>
>> +// (A) 'tp' is representable as a time_t<br>
>> +// (B) 'tp' is non-negative or the filesystem supports negative times.<br>
>> +// (C) 'tp' is not 'file_time_type::max()' or the filesystem supports the<br>
>> max<br>
>> +// value.<br>
>> +inline bool TimeIsRepresentableByFilesyste<wbr>m(file_time_type tp) {<br>
>> + using namespace std::chrono;<br>
>> + using Lim = std::numeric_limits<std::time_<wbr>t>;<br>
>> + auto sec = duration_cast<seconds>(tp.<wbr>time_since_epoch()).count();<br>
>> + auto microsec =<br>
>> duration_cast<microseconds>(<wbr>tp.time_since_epoch()).count()<wbr>;<br>
>> + if (sec < Lim::min() || sec > Lim::max()) return false;<br>
>> + else if (microsec < 0 && !SupportsNegativeTimes) return false;<br>
>> + else if (tp == file_time_type::max() && !SupportsMaxTime) return<br>
>> false;<br>
>> + return true;<br>
>> +}<br>
>><br>
>> TEST_SUITE(exists_test_suite)<br>
>><br>
>> @@ -214,15 +261,17 @@ TEST_CASE(set_last_write_time_<wbr>dynamic_en<br>
>><br>
>> file_time_type got_time = last_write_time(TC.p);<br>
>><br>
>> - TEST_CHECK(got_time != old_time);<br>
>> - if (TC.new_time < epoch_time) {<br>
>> - TEST_CHECK(got_time <= TC.new_time);<br>
>> - TEST_CHECK(got_time > TC.new_time - Sec(1));<br>
>> - } else {<br>
>> - TEST_CHECK(got_time <= TC.new_time + Sec(1));<br>
>> - TEST_CHECK(got_time >= TC.new_time - Sec(1));<br>
>> + if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(TC.new_time)) {<br>
>> + TEST_CHECK(got_time != old_time);<br>
>> + if (TC.new_time < epoch_time) {<br>
>> + TEST_CHECK(got_time <= TC.new_time);<br>
>> + TEST_CHECK(got_time > TC.new_time - Sec(1));<br>
>> + } else {<br>
>> + TEST_CHECK(got_time <= TC.new_time + Sec(1));<br>
>> + TEST_CHECK(got_time >= TC.new_time - Sec(1));<br>
>> + }<br>
>> + TEST_CHECK(LastAccessTime(TC.<wbr>p) == old_times.first);<br>
>> }<br>
>> - TEST_CHECK(LastAccessTime(TC.<wbr>p) == old_times.first);<br>
>> }<br>
>> }<br>
>><br>
>> @@ -269,17 +318,12 @@ TEST_CASE(test_write_min_time)<br>
>> const path p = env.create_file("file", 42);<br>
>><br>
>> std::error_code ec = GetTestEC();<br>
>> - file_time_type last_time = last_write_time(p);<br>
>> file_time_type new_time = file_time_type::min();<br>
>><br>
>> last_write_time(p, new_time, ec);<br>
>> file_time_type tt = last_write_time(p);<br>
>><br>
>> - if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {<br>
>> - TEST_CHECK(ec);<br>
>> - TEST_CHECK(ec != GetTestEC());<br>
>> - TEST_CHECK(tt == last_time);<br>
>> - } else {<br>
>> + if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {<br>
>> TEST_CHECK(!ec);<br>
>> TEST_CHECK(tt >= new_time);<br>
>> TEST_CHECK(tt < new_time + Sec(1));<br>
>> @@ -287,18 +331,13 @@ TEST_CASE(test_write_min_time)<br>
>><br>
>> ec = GetTestEC();<br>
>> last_write_time(p, Clock::now());<br>
>> - last_time = last_write_time(p);<br>
>><br>
>> new_time = file_time_type::min() + MicroSec(1);<br>
>><br>
>> last_write_time(p, new_time, ec);<br>
>> tt = last_write_time(p);<br>
>><br>
>> - if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {<br>
>> - TEST_CHECK(ec);<br>
>> - TEST_CHECK(ec != GetTestEC());<br>
>> - TEST_CHECK(tt == last_time);<br>
>> - } else {<br>
>> + if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {<br>
>> TEST_CHECK(!ec);<br>
>> TEST_CHECK(tt >= new_time);<br>
>> TEST_CHECK(tt < new_time + Sec(1));<br>
>> @@ -317,18 +356,13 @@ TEST_CASE(test_write_min_max_<wbr>time)<br>
>> const path p = env.create_file("file", 42);<br>
>><br>
>> std::error_code ec = GetTestEC();<br>
>> - file_time_type last_time = last_write_time(p);<br>
>> file_time_type new_time = file_time_type::max();<br>
>><br>
>> ec = GetTestEC();<br>
>> last_write_time(p, new_time, ec);<br>
>> file_time_type tt = last_write_time(p);<br>
>><br>
>> - if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {<br>
>> - TEST_CHECK(ec);<br>
>> - TEST_CHECK(ec != GetTestEC());<br>
>> - TEST_CHECK(tt == last_time);<br>
>> - } else {<br>
>> + if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {<br>
>> TEST_CHECK(!ec);<br>
>> TEST_CHECK(tt > new_time - Sec(1));<br>
>> TEST_CHECK(tt <= new_time);<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org">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>
><br>
><br>
</div></div></blockquote></div><br></div>