[libcxx] r307117 - Implement LWG 2937 - equivalent("dne", "exists") is not an error

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 4 20:37:05 PDT 2017


Author: ericwf
Date: Tue Jul  4 20:37:05 2017
New Revision: 307117

URL: http://llvm.org/viewvc/llvm-project?rev=307117&view=rev
Log:
Implement LWG 2937 - equivalent("dne", "exists") is not an error

This patch speculatively implements the PR for LWG 2937, which fixes
two issues with equivalent.

(1) It makes equivalent("dne", "exists") an error. Previously only
    equivalent("dne", "dne") was an error and the former case was not (it returned false).
    Now equivalent reports an error when either input doesn't exist.

(2) It makes equivalent(p1, p2) well-formed when `is_other(p1) && is_other(p2)`.
    Previously this was an error, but there is seemingly no reason why it should be on POSIX system.

Modified:
    libcxx/trunk/src/experimental/filesystem/operations.cpp
    libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.equivalent/equivalent.pass.cpp
    libcxx/trunk/www/upcoming_meeting.html

Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=307117&r1=307116&r2=307117&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/operations.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp Tue Jul  4 20:37:05 2017
@@ -426,17 +426,20 @@ void __current_path(const path& p, std::
 
 bool __equivalent(const path& p1, const path& p2, std::error_code *ec)
 {
+    auto make_unsupported_error = [&]() {
+      set_or_throw(make_error_code(errc::not_supported), ec,
+                     "equivalent", p1, p2);
+      return false;
+    };
     std::error_code ec1, ec2;
     struct ::stat st1 = {};
     struct ::stat st2 = {};
     auto s1 = detail::posix_stat(p1.native(), st1, &ec1);
+    if (!exists(s1))
+      return make_unsupported_error();
     auto s2 = detail::posix_stat(p2.native(), st2, &ec2);
-
-    if ((!exists(s1) && !exists(s2)) || (is_other(s1) && is_other(s2))) {
-        set_or_throw(make_error_code(errc::not_supported), ec,
-                     "equivalent", p1, p2);
-        return false;
-    }
+    if (!exists(s2))
+      return make_unsupported_error();
     if (ec) ec->clear();
     return (st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino);
 }

Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.equivalent/equivalent.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.equivalent/equivalent.pass.cpp?rev=307117&r1=307116&r2=307117&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.equivalent/equivalent.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.equivalent/equivalent.pass.cpp Tue Jul  4 20:37:05 2017
@@ -26,63 +26,87 @@ using namespace std::experimental::files
 
 TEST_SUITE(equivalent_test_suite)
 
-TEST_CASE(signature_test)
-{
-    const path p; ((void)p);
-    std::error_code ec; ((void)ec);
-    ASSERT_NOEXCEPT(equivalent(p, p, ec));
-    ASSERT_NOT_NOEXCEPT(equivalent(p, p));
+TEST_CASE(signature_test) {
+  const path p;
+  ((void)p);
+  std::error_code ec;
+  ((void)ec);
+  ASSERT_NOEXCEPT(equivalent(p, p, ec));
+  ASSERT_NOT_NOEXCEPT(equivalent(p, p));
 }
 
-TEST_CASE(equivalent_test)
-{
-    struct TestCase {
-        path lhs;
-        path rhs;
-        bool expect;
-    };
-    const TestCase testCases[] = {
-        {StaticEnv::Dir, StaticEnv::Dir, true},
-        {StaticEnv::File, StaticEnv::Dir, false},
-        {StaticEnv::Dir, StaticEnv::SymlinkToDir, true},
-        {StaticEnv::Dir, StaticEnv::SymlinkToFile, false},
-        {StaticEnv::File, StaticEnv::File, true},
-        {StaticEnv::File, StaticEnv::SymlinkToFile, true},
-    };
-    for (auto& TC : testCases) {
-        std::error_code ec;
-        TEST_CHECK(equivalent(TC.lhs, TC.rhs, ec) == TC.expect);
-        TEST_CHECK(!ec);
-    }
+TEST_CASE(equivalent_test) {
+  struct TestCase {
+    path lhs;
+    path rhs;
+    bool expect;
+  };
+  const TestCase testCases[] = {
+      {StaticEnv::Dir, StaticEnv::Dir, true},
+      {StaticEnv::File, StaticEnv::Dir, false},
+      {StaticEnv::Dir, StaticEnv::SymlinkToDir, true},
+      {StaticEnv::Dir, StaticEnv::SymlinkToFile, false},
+      {StaticEnv::File, StaticEnv::File, true},
+      {StaticEnv::File, StaticEnv::SymlinkToFile, true},
+  };
+  for (auto& TC : testCases) {
+    std::error_code ec;
+    TEST_CHECK(equivalent(TC.lhs, TC.rhs, ec) == TC.expect);
+    TEST_CHECK(!ec);
+  }
 }
 
-TEST_CASE(equivalent_reports_double_dne)
-{
-    const path E = StaticEnv::File;
-    const path DNE = StaticEnv::DNE;
-    { // Test that no exception is thrown if one of the paths exists
-        TEST_CHECK(equivalent(E, DNE) == false);
-        TEST_CHECK(equivalent(DNE, E) == false);
-    }
-    { // Test that an exception is thrown if both paths do not exist.
-        TEST_CHECK_THROW(filesystem_error, equivalent(DNE, DNE));
-    }
-    {
-        std::error_code ec;
-        TEST_CHECK(equivalent(DNE, DNE, ec) == false);
-        TEST_CHECK(ec);
-    }
+TEST_CASE(equivalent_reports_error_if_input_dne) {
+  const path E = StaticEnv::File;
+  const path DNE = StaticEnv::DNE;
+  { // Test that an error is reported when either of the paths don't exist
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(equivalent(E, DNE, ec) == false);
+    TEST_CHECK(ec);
+    TEST_CHECK(ec != GetTestEC());
+  }
+  {
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(equivalent(DNE, E, ec) == false);
+    TEST_CHECK(ec);
+    TEST_CHECK(ec != GetTestEC());
+  }
+  {
+    TEST_CHECK_THROW(filesystem_error, equivalent(DNE, E));
+    TEST_CHECK_THROW(filesystem_error, equivalent(E, DNE));
+  }
+  { // Test that an exception is thrown if both paths do not exist.
+    TEST_CHECK_THROW(filesystem_error, equivalent(DNE, DNE));
+  }
+  {
+    std::error_code ec = GetTestEC();
+    TEST_CHECK(equivalent(DNE, DNE, ec) == false);
+    TEST_CHECK(ec);
+    TEST_CHECK(ec != GetTestEC());
+  }
 }
 
-TEST_CASE(equivalent_is_other_succeeds)
-{
-    scoped_test_env env;
-    path const file = env.create_file("file", 42);
-    const path hl1 = env.create_hardlink(file, "hl1");
-    const path hl2 = env.create_hardlink(file, "hl2");
-    TEST_CHECK(equivalent(file, hl1));
-    TEST_CHECK(equivalent(file, hl2));
-    TEST_CHECK(equivalent(hl1, hl2));
+TEST_CASE(equivalent_hardlink_succeeds) {
+  scoped_test_env env;
+  path const file = env.create_file("file", 42);
+  const path hl1 = env.create_hardlink(file, "hl1");
+  const path hl2 = env.create_hardlink(file, "hl2");
+  TEST_CHECK(equivalent(file, hl1));
+  TEST_CHECK(equivalent(file, hl2));
+  TEST_CHECK(equivalent(hl1, hl2));
+}
+
+TEST_CASE(equivalent_is_other_succeeds) {
+  scoped_test_env env;
+  path const file = env.create_file("file", 42);
+  const path sock1 = env.create_socket("sock1");
+  const path sock2 = env.create_socket("sock2");
+  // Required to test behavior for inputs where is_other(p) is true.
+  TEST_REQUIRE(is_other(sock1));
+  TEST_CHECK(!equivalent(file, sock1));
+  TEST_CHECK(!equivalent(sock2, file));
+  TEST_CHECK(!equivalent(sock1, sock2));
+  TEST_CHECK(equivalent(sock1, sock1));
 }
 
 TEST_SUITE_END()

Modified: libcxx/trunk/www/upcoming_meeting.html
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/upcoming_meeting.html?rev=307117&r1=307116&r2=307117&view=diff
==============================================================================
--- libcxx/trunk/www/upcoming_meeting.html (original)
+++ libcxx/trunk/www/upcoming_meeting.html Tue Jul  4 20:37:05 2017
@@ -64,7 +64,7 @@
 	<tr><td><a href="http://wg21.link/LWG2597">2597</a></td><td>std::log misspecified for complex numbers</td><td>Toronto</td><td></td></tr>
 	<tr><td><a href="http://wg21.link/LWG2783">2783</a></td><td>stack::emplace() and queue::emplace() should return decltype(auto)</td><td>Toronto</td><td></td></tr>
 	<tr><td><a href="http://wg21.link/LWG2932">2932</a></td><td>Constraints on parallel algorithm implementations are underspecified</td><td>Toronto</td><td></td></tr>
-	<tr><td><a href="http://wg21.link/LWG2937">2937</a></td><td>Is equivalent("existing_thing", "not_existing_thing") an error?</td><td>Toronto</td><td></td></tr>
+	<tr><td><a href="http://wg21.link/LWG2937">2937</a></td><td>Is equivalent("existing_thing", "not_existing_thing") an error?</td><td>Toronto</td><td>Complete</td></tr>
 	<tr><td><a href="http://wg21.link/LWG2940">2940</a></td><td>result_of specification also needs a little cleanup</td><td>Toronto</td><td></td></tr>
 	<tr><td><a href="http://wg21.link/LWG2942">2942</a></td><td>LWG 2873's resolution missed weak_ptr::owner_before</td><td>Toronto</td><td></td></tr>
 	<tr><td><a href="http://wg21.link/LWG2954">2954</a></td><td>Specialization of the convenience variable templates should be prohibited</td><td>Toronto</td><td></td></tr>
@@ -84,7 +84,7 @@
 <li>2597 - I think we do this already; probably needs tests</li>
 <li>2783 - should be easy to change; needs tests</li>
 <li>2932 - We're not doing the parallel algorithms yet.</li>
-<li>2937 - file system; Eric?</li>
+<li>2937 - Implemented with tests. The PR LGTM (Eric)</li>
 <li>2940 - We haven't implemented result_of yet, but I don't think that this will require any changes.</li>
 <li>2942 - all of our owner_before overloads are already noexcept; just need to update the tests.</li>
 <li>2954 - I don't think there's anything to do here.</li>




More information about the cfe-commits mailing list