[libcxx-commits] [libcxx] [libcxx] Correct and clean-up filesystem operations error_code paths (PR #88341)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/88341

>From c312b30abb845d514d7ab0a0a1a9745925a2263a Mon Sep 17 00:00:00 2001
From: Rodrigo Salazar <4rodrigosalazar at gmail.com>
Date: Wed, 10 Apr 2024 17:46:39 -0700
Subject: [PATCH 1/2] [libcxx] Correct and clean-up filesystem:: error_code
 paths

---
 libcxx/src/filesystem/operations.cpp          | 28 ++++++++++---------
 .../weakly_canonical.pass.cpp                 |  8 ++++++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp
index 62bb248d5eca9..6a8e42d2090c4 100644
--- a/libcxx/src/filesystem/operations.cpp
+++ b/libcxx/src/filesystem/operations.cpp
@@ -126,9 +126,6 @@ void __copy(const path& from, const path& to, copy_options options, error_code*
     return err.report(errc::function_not_supported);
   }
 
-  if (ec)
-    ec->clear();
-
   if (is_symlink(f)) {
     if (bool(copy_options::skip_symlinks & options)) {
       // do nothing
@@ -166,15 +163,15 @@ void __copy(const path& from, const path& to, copy_options options, error_code*
       return;
     }
     error_code m_ec2;
-    for (; it != directory_iterator(); it.increment(m_ec2)) {
-      if (m_ec2) {
-        return err.report(m_ec2);
-      }
+    for (; !m_ec2 && it != directory_iterator(); it.increment(m_ec2)) {
       __copy(it->path(), to / it->path().filename(), options | copy_options::__in_recursive_copy, ec);
       if (ec && *ec) {
         return;
       }
     }
+    if (m_ec2) {
+      return err.report(m_ec2);
+    }
   }
 }
 
@@ -936,23 +933,28 @@ path __weakly_canonical(const path& p, error_code* ec) {
   --PP;
   vector<string_view_t> DNEParts;
 
+  error_code m_ec;
   while (PP.State != PathParser::PS_BeforeBegin) {
     tmp.assign(createView(p.native().data(), &PP.RawEntry.back()));
-    error_code m_ec;
     file_status st = __status(tmp, &m_ec);
     if (!status_known(st)) {
       return err.report(m_ec);
     } else if (exists(st)) {
-      result = __canonical(tmp, ec);
+      result = __canonical(tmp, &m_ec);
+      if (m_ec) {
+        return err.report(m_ec);
+      }
       break;
     }
     DNEParts.push_back(*PP);
     --PP;
   }
-  if (PP.State == PathParser::PS_BeforeBegin)
-    result = __canonical("", ec);
-  if (ec)
-    ec->clear();
+  if (PP.State == PathParser::PS_BeforeBegin) {
+    result = __canonical("", &m_ec);
+    if (m_ec) {
+      return err.report(m_ec);
+    }
+  }
   if (DNEParts.empty())
     return result;
   for (auto It = DNEParts.rbegin(); It != DNEParts.rend(); ++It)
diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
index 053bddcad9b43..4e4f459f8cf36 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
@@ -79,6 +79,14 @@ int main(int, char**) {
     TEST_REQUIRE(PathEq(output, expect),
                  TEST_WRITE_CONCATENATED(
                      "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string()));
+
+    // Get coverage over the error_code form of the api.
+    std::error_code ec;
+    const fs::path output_c = fs::weakly_canonical(p, ec);
+
+    TEST_REQUIRE(PathEq(output_c, expect),
+                 TEST_WRITE_CONCATENATED(
+                     "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string()));
   }
   return 0;
 }

>From 72246a74c40b3610f068ac4a073ec694c04bdf54 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 12 Jun 2024 12:10:42 -0400
Subject: [PATCH 2/2] Minor reformatting in the test

---
 .../weakly_canonical.pass.cpp                 | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
index 4e4f459f8cf36..59bb3c78667b4 100644
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.weakly_canonical/weakly_canonical.pass.cpp
@@ -74,19 +74,23 @@ int main(int, char**) {
     fs::path p      = TC.input;
     fs::path expect = TC.expect;
     expect.make_preferred();
-    const fs::path output = fs::weakly_canonical(p);
 
-    TEST_REQUIRE(PathEq(output, expect),
-                 TEST_WRITE_CONCATENATED(
-                     "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string()));
+    {
+      const fs::path output = fs::weakly_canonical(p);
+      TEST_REQUIRE(PathEq(output, expect),
+                   TEST_WRITE_CONCATENATED(
+                       "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output.string()));
+    }
 
-    // Get coverage over the error_code form of the api.
-    std::error_code ec;
-    const fs::path output_c = fs::weakly_canonical(p, ec);
+    // Test the error_code variant
+    {
+      std::error_code ec;
+      const fs::path output_c = fs::weakly_canonical(p, ec);
 
-    TEST_REQUIRE(PathEq(output_c, expect),
-                 TEST_WRITE_CONCATENATED(
-                     "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string()));
+      TEST_REQUIRE(PathEq(output_c, expect),
+                   TEST_WRITE_CONCATENATED(
+                       "Input: ", TC.input.string(), "\nExpected: ", expect.string(), "\nOutput: ", output_c.string()));
+    }
   }
   return 0;
 }