<div dir="ltr">+1 from me. @Hans am I OK to merge this?</div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jan 14, 2017 at 4:53 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"><div><div>Hi Hans,</div><div><br></div><div>can this be merged for 4.0? Eric suggested this in <a href="https://reviews.llvm.org/D22452" target="_blank">https://reviews.llvm.org/<wbr>D22452</a> so I think he should be fine.</div><div><br></div><div>Thanks,</div><div>Jonas</div><div><div class="h5"><div><br></div><div>Am Samstag, den 14.01.2017, 11:35 +0000 schrieb Jonas Hahnfeld via cfe-commits:</div><blockquote type="cite"><pre>Author: hahnfeld
Date: Sat Jan 14 05:35:15 2017
New Revision: 292013

URL: <a href="http://llvm.org/viewvc/llvm-project?rev=292013&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=292013&view=rev</a>
Log:
Fix last_write_time tests for filesystems that don't support negative and very large times

Seems to be the case for NFS.

Original patch by Eric Fiselier!
Differential Revision: <a href="https://reviews.llvm.org/D22452" target="_blank">https://reviews.llvm.org/<wbr>D22452</a>

Modified:
    libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp

Modified: libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp
URL: <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" 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>
==============================<wbr>==============================<wbr>==================
--- libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp (original)
+++ libcxx/trunk/test/std/<wbr>experimental/filesystem/fs.op.<wbr>funcs/fs.op.last_write_time/<wbr>last_write_time.pass.cpp Sat Jan 14 05:35:15 2017
@@ -72,13 +72,60 @@ std::pair<std::time_t, std::time_t> GetS
     return {st.st_atime, st.st_mtime};
 }
 
-inline bool TimeIsRepresentableAsTimeT(<wbr>file_time_type tp) {
+namespace {
+bool TestSupportsNegativeTimes() {
+    using namespace std::chrono;
+    std::error_code ec;
+    std::time_t old_write_time, new_write_time;
+    { // WARNING: Do not assert in this scope.
+        scoped_test_env env;
+        const path file = env.create_file("file", 42);
+        old_write_time = LastWriteTime(file);
+        file_time_type tp(seconds(-5));
+        fs::last_write_time(file, tp, ec);
+        new_write_time = LastWriteTime(file);
+    }
+    return !ec && new_write_time <= -5;
+}
+
+bool TestSupportsMaxTime() {
     using namespace std::chrono;
     using Lim = std::numeric_limits<std::time_<wbr>t>;
-    auto sec = duration_cast<seconds>(tp.<wbr>time_since_epoch()).count();
-    return (sec >= Lim::min() && sec <= Lim::max());
+    auto max_sec = duration_cast<seconds>(file_<wbr>time_type::max().time_since_<wbr>epoch()).count();
+    if (max_sec > Lim::max()) return false;
+    std::error_code ec;
+    std::time_t old_write_time, new_write_time;
+    { // WARNING: Do not assert in this scope.
+        scoped_test_env env;
+        const path file = env.create_file("file", 42);
+        old_write_time = LastWriteTime(file);
+        file_time_type tp = file_time_type::max();
+        fs::last_write_time(file, tp, ec);
+        new_write_time = LastWriteTime(file);
+    }
+    return !ec && new_write_time > max_sec - 1;
 }
 
+static const bool SupportsNegativeTimes = TestSupportsNegativeTimes();
+static const bool SupportsMaxTime = TestSupportsMaxTime();
+
+} // end namespace
+
+// Check if a time point is representable on a given filesystem. Check that:
+// (A) 'tp' is representable as a time_t
+// (B) 'tp' is non-negative or the filesystem supports negative times.
+// (C) 'tp' is not 'file_time_type::max()' or the filesystem supports the max
+//     value.
+inline bool TimeIsRepresentableByFilesyste<wbr>m(file_time_type tp) {
+    using namespace std::chrono;
+    using Lim = std::numeric_limits<std::time_<wbr>t>;
+    auto sec = duration_cast<seconds>(tp.<wbr>time_since_epoch()).count();
+    auto microsec = duration_cast<microseconds>(<wbr>tp.time_since_epoch()).count()<wbr>;
+    if (sec < Lim::min() || sec > Lim::max())   return false;
+    else if (microsec < 0 && !SupportsNegativeTimes) return false;
+    else if (tp == file_time_type::max() && !SupportsMaxTime) return false;
+    return true;
+}
 
 TEST_SUITE(exists_test_suite)
 
@@ -214,15 +261,17 @@ TEST_CASE(set_last_write_time_<wbr>dynamic_en
 
         file_time_type  got_time = last_write_time(TC.p);
 
-        TEST_CHECK(got_time != old_time);
-        if (TC.new_time < epoch_time) {
-            TEST_CHECK(got_time <= TC.new_time);
-            TEST_CHECK(got_time > TC.new_time - Sec(1));
-        } else {
-            TEST_CHECK(got_time <= TC.new_time + Sec(1));
-            TEST_CHECK(got_time >= TC.new_time - Sec(1));
+        if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(TC.new_time)) {
+            TEST_CHECK(got_time != old_time);
+            if (TC.new_time < epoch_time) {
+                TEST_CHECK(got_time <= TC.new_time);
+                TEST_CHECK(got_time > TC.new_time - Sec(1));
+            } else {
+                TEST_CHECK(got_time <= TC.new_time + Sec(1));
+                TEST_CHECK(got_time >= TC.new_time - Sec(1));
+            }
+            TEST_CHECK(LastAccessTime(TC.<wbr>p) == old_times.first);
         }
-        TEST_CHECK(LastAccessTime(TC.<wbr>p) == old_times.first);
     }
 }
 
@@ -269,17 +318,12 @@ TEST_CASE(test_write_min_time)
     const path p = env.create_file("file", 42);
 
     std::error_code ec = GetTestEC();
-    file_time_type last_time = last_write_time(p);
     file_time_type new_time = file_time_type::min();
 
     last_write_time(p, new_time, ec);
     file_time_type tt = last_write_time(p);
 
-    if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {
-        TEST_CHECK(ec);
-        TEST_CHECK(ec != GetTestEC());
-        TEST_CHECK(tt == last_time);
-    } else {
+    if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {
         TEST_CHECK(!ec);
         TEST_CHECK(tt >= new_time);
         TEST_CHECK(tt < new_time + Sec(1));
@@ -287,18 +331,13 @@ TEST_CASE(test_write_min_time)
 
     ec = GetTestEC();
     last_write_time(p, Clock::now());
-    last_time = last_write_time(p);
 
     new_time = file_time_type::min() + MicroSec(1);
 
     last_write_time(p, new_time, ec);
     tt = last_write_time(p);
 
-    if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {
-        TEST_CHECK(ec);
-        TEST_CHECK(ec != GetTestEC());
-        TEST_CHECK(tt == last_time);
-    } else {
+    if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {
         TEST_CHECK(!ec);
         TEST_CHECK(tt >= new_time);
         TEST_CHECK(tt < new_time + Sec(1));
@@ -317,18 +356,13 @@ TEST_CASE(test_write_min_max_<wbr>time)
     const path p = env.create_file("file", 42);
 
     std::error_code ec = GetTestEC();
-    file_time_type last_time = last_write_time(p);
     file_time_type new_time = file_time_type::max();
 
     ec = GetTestEC();
     last_write_time(p, new_time, ec);
     file_time_type tt = last_write_time(p);
 
-    if (!TimeIsRepresentableAsTimeT(<wbr>new_time)) {
-        TEST_CHECK(ec);
-        TEST_CHECK(ec != GetTestEC());
-        TEST_CHECK(tt == last_time);
-    } else {
+    if (<wbr>TimeIsRepresentableByFilesyste<wbr>m(new_time)) {
         TEST_CHECK(!ec);
         TEST_CHECK(tt > new_time - Sec(1));
         TEST_CHECK(tt <= new_time);


______________________________<wbr>_________________
cfe-commits mailing list
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a>
</pre></blockquote></div></div></div></blockquote></div><br></div>