PR libstdc++/83626 Don't throw for remove("") and remove_all("")

PR libstdc++/83626
	* src/filesystem/ops.cc (remove(const path&, error_code&))): Remove
	redundant call to ec.clear().
	(remove_all(const path&, error_code&))): Do not return an error for
	non-existent paths.
	* src/filesystem/std-ops.cc: Likewise.
	* testsuite/27_io/filesystem/operations/remove.cc: New test.
	* testsuite/27_io/filesystem/operations/remove_all.cc: Fix expected
	results for non-existent paths.
	* testsuite/experimental/filesystem/operations/remove.cc: New test.
	* testsuite/experimental/filesystem/operations/remove_all.cc: Fix
	expected results for non-existent paths.

From-SVN: r256269

libstdc++-v3/ChangeLog

@@ -1,5 +1,18 @@
 2018-01-04  Jonathan Wakely  <jwakely@redhat.com>
 
+	PR libstdc++/83626
+	* src/filesystem/ops.cc (remove(const path&, error_code&))): Remove
+	redundant call to ec.clear().
+	(remove_all(const path&, error_code&))): Do not return an error for
+	non-existent paths.
+	* src/filesystem/std-ops.cc: Likewise.
+	* testsuite/27_io/filesystem/operations/remove.cc: New test.
+	* testsuite/27_io/filesystem/operations/remove_all.cc: Fix expected
+	results for non-existent paths.
+	* testsuite/experimental/filesystem/operations/remove.cc: New test.
+	* testsuite/experimental/filesystem/operations/remove_all.cc: Fix
+	expected results for non-existent paths.
+
 	* include/bits/fs_ops.h (exists(const path&, error_code&))): Only
 	check status_known once.
 	* include/experimental/bits/fs_ops.h: Likewise.

libstdc++-v3/src/filesystem/ops.cc

@@ -1009,7 +1009,7 @@ fs::remove(const path& p)
 {
   error_code ec;
   bool result = fs::remove(p, ec);
-  if (ec.value())
+  if (ec)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove", p, ec));
   return result;
 }
@@ -1017,17 +1017,21 @@ fs::remove(const path& p)
 bool
 fs::remove(const path& p, error_code& ec) noexcept
 {
-  if (exists(symlink_status(p, ec)))
+  const auto s = symlink_status(p, ec);
+  if (!status_known(s))
+    return false;
+  if (s.type() == file_type::not_found)
     {
-      if (::remove(p.c_str()) == 0)
-	{
-	  ec.clear();
-	  return true;
-	}
-      else
-	ec.assign(errno, std::generic_category());
+      ec.clear();
+      return false; // Nothing to do, not an error.
     }
-  return false;
+  if (::remove(p.c_str()) != 0)
+    {
+      ec.assign(errno, std::generic_category());
+      return false;
+    }
+  ec.clear();
+  return true;
 }
 
 
@@ -1036,7 +1040,7 @@ fs::remove_all(const path& p)
 {
   error_code ec;
   bool result = remove_all(p, ec);
-  if (ec.value())
+  if (ec)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all", p, ec));
   return result;
 }
@@ -1044,14 +1048,19 @@ fs::remove_all(const path& p)
 std::uintmax_t
 fs::remove_all(const path& p, error_code& ec) noexcept
 {
-  auto fs = symlink_status(p, ec);
-  uintmax_t count = 0;
-  if (ec.value() == 0 && fs.type() == file_type::directory)
-    for (directory_iterator d(p, ec), end; ec.value() == 0 && d != end; ++d)
-      count += fs::remove_all(d->path(), ec);
-  if (ec.value())
+  const auto s = symlink_status(p, ec);
+  if (!status_known(s))
     return -1;
-  return fs::remove(p, ec) ? ++count : -1;  // fs:remove() calls ec.clear()
+  ec.clear();
+  uintmax_t count = 0;
+  if (s.type() == file_type::directory)
+    for (directory_iterator d(p, ec), end; !ec && d != end; ++d)
+      count += fs::remove_all(d->path(), ec);
+  if (!ec && fs::remove(p, ec))
+    ++count;
+  if (ec)
+    return -1;
+  return count;
 }
 
 void

libstdc++-v3/src/filesystem/std-ops.cc

@@ -1266,17 +1266,21 @@ fs::remove(const path& p)
 bool
 fs::remove(const path& p, error_code& ec) noexcept
 {
-  if (exists(symlink_status(p, ec)))
+  const auto s = symlink_status(p, ec);
+  if (!status_known(s))
+    return false;
+  if (s.type() == file_type::not_found)
     {
-      if (::remove(p.c_str()) == 0)
-	{
-	  ec.clear();
-	  return true;
-	}
-      else
-	ec.assign(errno, std::generic_category());
+      ec.clear();
+      return false; // Nothing to do, not an error.
     }
-  return false;
+  if (::remove(p.c_str()) != 0)
+    {
+      ec.assign(errno, std::generic_category());
+      return false;
+    }
+  ec.clear();
+  return true;
 }
 
 
@@ -1293,14 +1297,19 @@ fs::remove_all(const path& p)
 std::uintmax_t
 fs::remove_all(const path& p, error_code& ec)
 {
-  auto fs = symlink_status(p, ec);
+  const auto s = symlink_status(p, ec);
+  if (!status_known(s))
+    return -1;
+  ec.clear();
   uintmax_t count = 0;
-  if (!ec && fs.type() == file_type::directory)
+  if (s.type() == file_type::directory)
     for (directory_iterator d(p, ec), end; !ec && d != end; ++d)
       count += fs::remove_all(d->path(), ec);
+  if (!ec && fs::remove(p, ec))
+    ++count;
   if (ec)
     return -1;
-  return fs::remove(p, ec) ? ++count : -1;  // fs:remove() calls ec.clear()
+  return count;
 }
 
 void

libstdc++-v3/testsuite/27_io/filesystem/operations/remove.cc

@@ -0,0 +1,100 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17 -lstdc++fs" }
+// { dg-do run { target c++17 } }
+// { dg-require-filesystem-ts "" }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::filesystem;
+
+void
+test01()
+{
+  std::error_code ec;
+  const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
+  bool n;
+
+  n = fs::remove("", ec);
+  VERIFY( !ec ); // This seems odd, but is what the standard requires.
+  VERIFY( !n );
+
+  auto p = __gnu_test::nonexistent_path();
+  ec = bad_ec;
+  n = remove(p, ec);
+  VERIFY( !ec );
+  VERIFY( !n );
+
+  auto link = __gnu_test::nonexistent_path();
+  create_symlink(p, link);  // dangling symlink
+  ec = bad_ec;
+  n = remove(link, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(link)) );
+
+  __gnu_test::scoped_file f(p);
+  create_symlink(p, link);
+  ec = bad_ec;
+  n = remove(link, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(link)) );  // The symlink is removed, but
+  VERIFY( exists(p) );                      // its target is not.
+
+  ec = bad_ec;
+  n = remove(p, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(p)) );
+
+  const auto dir = __gnu_test::nonexistent_path();
+  create_directories(dir/"a/b");
+  ec.clear();
+  n = remove(dir/"a", ec);
+  VERIFY( ec );
+  VERIFY( !n );
+  VERIFY( exists(dir/"a/b") );
+
+  permissions(dir, fs::perms::none, ec);
+  if (!ec)
+  {
+    ec.clear();
+    n = remove(dir/"a/b", ec);
+    VERIFY( ec );
+    VERIFY( !n );
+    permissions(dir, fs::perms::owner_all, ec);
+  }
+
+  ec = bad_ec;
+  n = remove(dir/"a/b", ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(dir/"a/b") );
+
+  remove(dir/"a", ec);
+  remove(dir, ec);
+}
+
+int
+main()
+{
+  test01();
+}

libstdc++-v3/testsuite/27_io/filesystem/operations/remove_all.cc

@@ -29,19 +29,19 @@ void
 test01()
 {
   std::error_code ec;
+  const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
   std::uintmax_t n;
 
   n = fs::remove_all("", ec);
-  VERIFY( ec );
-  VERIFY( n == std::uintmax_t(-1) );
+  VERIFY( !ec ); // This seems odd, but is what the standard requires.
+  VERIFY( n == 0 );
 
   auto p = __gnu_test::nonexistent_path();
-  ec.clear();
+  ec = bad_ec;
   n = remove_all(p, ec);
-  VERIFY( ec );
-  VERIFY( n == std::uintmax_t(-1) );
+  VERIFY( !ec );
+  VERIFY( n == 0 );
 
-  const auto bad_ec = ec;
   auto link = __gnu_test::nonexistent_path();
   create_symlink(p, link);  // dangling symlink
   ec = bad_ec;
@@ -59,7 +59,7 @@ test01()
   VERIFY( !exists(symlink_status(link)) );  // The symlink is removed, but
   VERIFY( exists(p) );                      // its target is not.
 
-  auto dir = __gnu_test::nonexistent_path();
+  const auto dir = __gnu_test::nonexistent_path();
   create_directories(dir/"a/b/c");
   ec = bad_ec;
   n = remove_all(dir/"a", ec);

libstdc++-v3/testsuite/experimental/filesystem/operations/remove.cc

@@ -0,0 +1,100 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+  std::error_code ec;
+  const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
+  bool n;
+
+  n = fs::remove("", ec);
+  VERIFY( !ec ); // This seems odd, but is what the standard requires.
+  VERIFY( !n );
+
+  auto p = __gnu_test::nonexistent_path();
+  ec = bad_ec;
+  n = remove(p, ec);
+  VERIFY( !ec );
+  VERIFY( !n );
+
+  auto link = __gnu_test::nonexistent_path();
+  create_symlink(p, link);  // dangling symlink
+  ec = bad_ec;
+  n = remove(link, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(link)) );
+
+  __gnu_test::scoped_file f(p);
+  create_symlink(p, link);
+  ec = bad_ec;
+  n = remove(link, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(link)) );  // The symlink is removed, but
+  VERIFY( exists(p) );                      // its target is not.
+
+  ec = bad_ec;
+  n = remove(p, ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(symlink_status(p)) );
+
+  const auto dir = __gnu_test::nonexistent_path();
+  create_directories(dir/"a/b");
+  ec.clear();
+  n = remove(dir/"a", ec);
+  VERIFY( ec );
+  VERIFY( !n );
+  VERIFY( exists(dir/"a/b") );
+
+  permissions(dir, fs::perms::none, ec);
+  if (!ec)
+  {
+    ec.clear();
+    n = remove(dir/"a/b", ec);
+    VERIFY( ec );
+    VERIFY( !n );
+    permissions(dir, fs::perms::owner_all, ec);
+  }
+
+  ec = bad_ec;
+  n = remove(dir/"a/b", ec);
+  VERIFY( !ec );
+  VERIFY( n );
+  VERIFY( !exists(dir/"a/b") );
+
+  remove(dir/"a", ec);
+  remove(dir, ec);
+}
+
+int
+main()
+{
+  test01();
+}

libstdc++-v3/testsuite/experimental/filesystem/operations/remove_all.cc

@@ -29,19 +29,19 @@ void
 test01()
 {
   std::error_code ec;
+  const std::error_code bad_ec = make_error_code(std::errc::invalid_argument);
   std::uintmax_t n;
 
   n = fs::remove_all("", ec);
-  VERIFY( ec );
-  VERIFY( n == std::uintmax_t(-1) );
+  VERIFY( !ec ); // This seems odd, but is what the TS requires.
+  VERIFY( n == 0 );
 
   auto p = __gnu_test::nonexistent_path();
-  ec.clear();
+  ec = bad_ec;
   n = remove_all(p, ec);
-  VERIFY( ec );
-  VERIFY( n == std::uintmax_t(-1) );
+  VERIFY( !ec );
+  VERIFY( n == 0 );
 
-  const auto bad_ec = ec;
   auto link = __gnu_test::nonexistent_path();
   create_symlink(p, link);  // dangling symlink
   ec = bad_ec;
@@ -59,7 +59,7 @@ test01()
   VERIFY( !exists(symlink_status(link)) );  // The symlink is removed, but
   VERIFY( exists(p) );                      // its target is not.
 
-  auto dir = __gnu_test::nonexistent_path();
+  const auto dir = __gnu_test::nonexistent_path();
   create_directories(dir/"a/b/c");
   ec = bad_ec;
   n = remove_all(dir/"a", ec);