Share all recursive_directory_iterator state [LWG 2708]
authorJonathan Wakely <jwakely@redhat.com>
Fri, 5 Apr 2019 16:56:23 +0000 (17:56 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Fri, 5 Apr 2019 16:56:23 +0000 (17:56 +0100)
Implement the proposed resolution of LWG 2708 by moving the _M_options
and _M_pending members out of the recursive_directory_iterator into the
shared _Dir_stack object. Because _Dir_stack is an opaque type, the
member functions that access the _M_options and _M_pending variables
cannot be inline. Move them into the library.

As a drive-by fix, add noexcept to the non-throwing member functions of
recursive_directory_iterator.

* config/abi/pre/gnu.ver: Export new symbols.
* include/bits/fs_dir.h (recursive_directory_iterator::options())
(recursive_directory_iterator::recursion_pending())
(recursive_directory_iterator::disable_recursion_pending()): Remove
inline definitions. Make noexcept.
(recursive_directory_iterator::depth())
(recursive_directory_iterator::operator*())
(recursive_directory_iterator::operator->()): Make noexcept.
(recursive_directory_iterator::_M_options)
(recursive_directory_iterator::_M_pending): Remove data members.
* src/c++17/fs_path.cc (_Dir_stack): Add constructor and data members.
(recursive_directory_iterator::recursive_directory_iterator): Remove
ctor-initializer. Use new constructor for _Dir_stack.
(recursive_directory_iterator::options())
(recursive_directory_iterator::recursion_pending())
(recursive_directory_iterator::disable_recursion_pending()): Add
non-inline definitions.
(recursive_directory_iterator::depth()): Make noexcept.
(recursive_directory_iterator::increment(error_code&))
(recursive_directory_iterator::pop(error_code&)): Adjust to new
location of options and recursion_pending members.
* testsuite/27_io/filesystem/iterators/recursion_pending.cc: New test.
* testsuite/util/testsuite_fs.h (__gnu_test::scoped_file): Add
user-declared move constructor and assignment operator, to make the
type move-only.

From-SVN: r270173

libstdc++-v3/ChangeLog
libstdc++-v3/config/abi/pre/gnu.ver
libstdc++-v3/include/bits/fs_dir.h
libstdc++-v3/src/c++17/fs_dir.cc
libstdc++-v3/testsuite/27_io/filesystem/iterators/recursion_pending.cc [new file with mode: 0644]
libstdc++-v3/testsuite/util/testsuite_fs.h

index 2141946f1798060bf778bca98c3f1812cb9647fb..e2aa3962f280a34d242f7ce0b70a228755f3f22e 100644 (file)
@@ -1,5 +1,31 @@
 2019-04-05  Jonathan Wakely  <jwakely@redhat.com>
 
+       * config/abi/pre/gnu.ver: Export new symbols.
+       * include/bits/fs_dir.h (recursive_directory_iterator::options())
+       (recursive_directory_iterator::recursion_pending())
+       (recursive_directory_iterator::disable_recursion_pending()): Remove
+       inline definitions. Make noexcept.
+       (recursive_directory_iterator::depth())
+       (recursive_directory_iterator::operator*())
+       (recursive_directory_iterator::operator->()): Make noexcept.
+       (recursive_directory_iterator::_M_options)
+       (recursive_directory_iterator::_M_pending): Remove data members.
+       * src/c++17/fs_path.cc (_Dir_stack): Add constructor and data members.
+       (recursive_directory_iterator::recursive_directory_iterator): Remove
+       ctor-initializer. Use new constructor for _Dir_stack.
+       (recursive_directory_iterator::options())
+       (recursive_directory_iterator::recursion_pending())
+       (recursive_directory_iterator::disable_recursion_pending()): Add
+       non-inline definitions.
+       (recursive_directory_iterator::depth()): Make noexcept.
+       (recursive_directory_iterator::increment(error_code&))
+       (recursive_directory_iterator::pop(error_code&)): Adjust to new
+       location of options and recursion_pending members.
+       * testsuite/27_io/filesystem/iterators/recursion_pending.cc: New test.
+       * testsuite/util/testsuite_fs.h (__gnu_test::scoped_file): Add
+       user-declared move constructor and assignment operator, to make the
+       type move-only.
+
        * src/c++17/fs_dir.cc (_Dir::advance(bool, error_code&)): Handle
        d_type == DT_UNKNOWN immediately.
        (_Dir::should_recurse(bool, error_code&)): Remove file_type::unknown
index c4f12152147ef6fa8b75a6205bb2490df29ea76a..019b581df711afafeb435aea941ced60a250c83d 100644 (file)
@@ -2200,7 +2200,10 @@ GLIBCXX_3.4.26 {
     _ZNSt10filesystem16weakly_canonical*;
 
     _ZNKSt10filesystem18directory_iteratordeEv;
+    _ZNKSt10filesystem28recursive_directory_iterator7optionsEv;
     _ZNKSt10filesystem28recursive_directory_iterator5depthEv;
+    _ZNKSt10filesystem28recursive_directory_iterator17recursion_pendingEv;
+    _ZNSt10filesystem28recursive_directory_iterator25disable_recursion_pendingEv;
     _ZNKSt10filesystem28recursive_directory_iteratordeEv;
     _ZNSt10filesystem18directory_iteratorC[12]ERKNS_4pathENS_17directory_optionsEPSt10error_code;
     _ZNSt10filesystem18directory_iteratorppEv;
@@ -2213,8 +2216,11 @@ GLIBCXX_3.4.26 {
     _ZNSt10filesystem28recursive_directory_iteratorppEv;
 
     _ZNKSt10filesystem7__cxx1118directory_iteratordeEv;
+    _ZNKSt10filesystem7__cxx1128recursive_directory_iterator7optionsEv;
     _ZNKSt10filesystem7__cxx1128recursive_directory_iterator5depthEv;
+    _ZNKSt10filesystem7__cxx1128recursive_directory_iterator17recursion_pendingEv;
     _ZNKSt10filesystem7__cxx1128recursive_directory_iteratordeEv;
+    _ZNSt10filesystem7__cxx1128recursive_directory_iterator25disable_recursion_pendingEv;
     _ZNSt10filesystem7__cxx1118directory_iteratorC[12]ERKNS0_4pathENS_17directory_optionsEPSt10error_code;
     _ZNSt10filesystem7__cxx1118directory_iteratorppEv;
     _ZNSt10filesystem7__cxx1128recursive_directory_iterator3popERSt10error_code;
index fd2da0f25c1ce06ede4105ac83dd7be4f01c5041..a5947b39541b994b08e657b379e67cfcc16a6cbd 100644 (file)
@@ -466,12 +466,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     ~recursive_directory_iterator();
 
     // observers
-    directory_options  options() const { return _M_options; }
-    int                depth() const;
-    bool               recursion_pending() const { return _M_pending; }
+    directory_options  options() const noexcept;
+    int                depth() const noexcept;
+    bool               recursion_pending() const noexcept;
 
-    const directory_entry& operator*() const;
-    const directory_entry* operator->() const { return &**this; }
+    const directory_entry& operator*() const noexcept;
+    const directory_entry* operator->() const noexcept { return &**this; }
 
     // modifiers
     recursive_directory_iterator&
@@ -492,7 +492,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
     void pop();
     void pop(error_code&);
 
-    void disable_recursion_pending() { _M_pending = false; }
+    void disable_recursion_pending() noexcept;
 
   private:
     recursive_directory_iterator(const path&, directory_options, error_code*);
@@ -503,8 +503,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
     struct _Dir_stack;
     std::__shared_ptr<_Dir_stack> _M_dirs;
-    directory_options _M_options = {};
-    bool _M_pending = false;
   };
 
   inline recursive_directory_iterator
index 629c4ebf78f5ed01dcfa6cc9207387dba34203b3..d8c48f6d6d861ef37466747a384393335dca69ee 100644 (file)
@@ -183,20 +183,27 @@ fs::directory_iterator::increment(error_code& ec)
 
 struct fs::recursive_directory_iterator::_Dir_stack : std::stack<_Dir>
 {
+  _Dir_stack(directory_options opts, posix::DIR* dirp, const path& p)
+  : options(opts), pending(true)
+  {
+    this->emplace(dirp, p);
+  }
+
+  const directory_options options;
+  bool pending;
+
   void clear() { c.clear(); }
 };
 
 fs::recursive_directory_iterator::
 recursive_directory_iterator(const path& p, directory_options options,
                              error_code* ecptr)
-: _M_options(options), _M_pending(true)
 {
   if (posix::DIR* dirp = posix::opendir(p.c_str()))
     {
       if (ecptr)
        ecptr->clear();
-      auto sp = std::__make_shared<_Dir_stack>();
-      sp->push(_Dir{ dirp, p });
+      auto sp = std::__make_shared<_Dir_stack>(options, dirp, p);
       if (ecptr ? sp->top().advance(*ecptr) : sp->top().advance())
        _M_dirs.swap(sp);
     }
@@ -222,14 +229,26 @@ recursive_directory_iterator(const path& p, directory_options options,
 
 fs::recursive_directory_iterator::~recursive_directory_iterator() = default;
 
+fs::directory_options
+fs::recursive_directory_iterator::options() const noexcept
+{
+  return _M_dirs->options;
+}
+
 int
-fs::recursive_directory_iterator::depth() const
+fs::recursive_directory_iterator::depth() const noexcept
 {
   return int(_M_dirs->size()) - 1;
 }
 
+bool
+fs::recursive_directory_iterator::recursion_pending() const noexcept
+{
+  return _M_dirs->pending;
+}
+
 const fs::directory_entry&
-fs::recursive_directory_iterator::operator*() const
+fs::recursive_directory_iterator::operator*() const noexcept
 {
   return _M_dirs->top().entry;
 }
@@ -263,13 +282,13 @@ fs::recursive_directory_iterator::increment(error_code& ec)
     }
 
   const bool follow
-    = is_set(_M_options, directory_options::follow_directory_symlink);
+    = is_set(_M_dirs->options, directory_options::follow_directory_symlink);
   const bool skip_permission_denied
-    = is_set(_M_options, directory_options::skip_permission_denied);
+    = is_set(_M_dirs->options, directory_options::skip_permission_denied);
 
   auto& top = _M_dirs->top();
 
-  if (std::exchange(_M_pending, true) && top.should_recurse(follow, ec))
+  if (std::exchange(_M_dirs->pending, true) && top.should_recurse(follow, ec))
     {
       _Dir dir(top.entry.path(), skip_permission_denied, ec);
       if (ec)
@@ -303,7 +322,7 @@ fs::recursive_directory_iterator::pop(error_code& ec)
     }
 
   const bool skip_permission_denied
-    = is_set(_M_options, directory_options::skip_permission_denied);
+    = is_set(_M_dirs->options, directory_options::skip_permission_denied);
 
   do {
     _M_dirs->pop();
@@ -327,3 +346,9 @@ fs::recursive_directory_iterator::pop()
          : "non-dereferenceable recursive directory iterator cannot pop",
          ec));
 }
+
+void
+fs::recursive_directory_iterator::disable_recursion_pending() noexcept
+{
+  _M_dirs->pending = false;
+}
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/iterators/recursion_pending.cc b/libstdc++-v3/testsuite/27_io/filesystem/iterators/recursion_pending.cc
new file mode 100644 (file)
index 0000000..c837ce1
--- /dev/null
@@ -0,0 +1,139 @@
+// Copyright (C) 2019 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" }
+// { dg-do run { target c++17 } }
+// { dg-require-filesystem-ts "" }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::filesystem;
+
+__gnu_test::scoped_file
+create_dir(fs::path dir = __gnu_test::nonexistent_path())
+{
+  fs::create_directory(dir);
+  return { dir, __gnu_test::scoped_file::adopt_file };
+}
+
+void
+test01()
+{
+  const auto testdir = create_dir();
+  __gnu_test::scoped_file file(testdir.path / "file");
+
+  fs::recursive_directory_iterator r(testdir.path);
+  VERIFY( r.recursion_pending() );
+
+  r.disable_recursion_pending();
+  VERIFY( !r.recursion_pending() );
+}
+
+void
+test02()
+{
+  const auto testdir = create_dir();
+  __gnu_test::scoped_file file(testdir.path / "file");
+
+  fs::recursive_directory_iterator r(testdir.path);
+  VERIFY( r.recursion_pending() );
+  const auto r2 = r;
+  // recursion pending flag should be copied:
+  VERIFY( r2.recursion_pending() == r.recursion_pending() );
+
+  r.disable_recursion_pending();
+  VERIFY( !r.recursion_pending() );
+  const auto r3 = r;
+  // recursion pending flag should be copied:
+  VERIFY( r3.recursion_pending() == r.recursion_pending() );
+}
+
+void
+test03()
+{
+  std::error_code ec = make_error_code(std::errc::invalid_argument);
+
+  const auto testdir = create_dir();
+  __gnu_test::scoped_file file1(testdir.path / "file1");
+  __gnu_test::scoped_file file2(testdir.path / "file2");
+  __gnu_test::scoped_file file3(testdir.path / "file3");
+  __gnu_test::scoped_file file4(testdir.path / "file4");
+
+  fs::recursive_directory_iterator r(testdir.path);
+  r.disable_recursion_pending();
+  VERIFY( !r.recursion_pending() );
+  ++r;
+  // recursion pending flag should be true after incrementing:
+  VERIFY( r.recursion_pending() );
+
+  r.disable_recursion_pending();
+  VERIFY( !r.recursion_pending() );
+  r.increment(ec);
+  VERIFY( !ec );
+  // recursion pending flag should be true after incrementing:
+  VERIFY( r.recursion_pending() );
+
+  r.disable_recursion_pending();
+  VERIFY( !r.recursion_pending() );
+  r++;
+  // recursion pending flag should be true after post-incrementing:
+  VERIFY( r.recursion_pending() );
+
+  VERIFY( ++r == fs::recursive_directory_iterator() );
+}
+
+void
+test04()
+{
+  const auto testdir = create_dir();
+  const auto sub1 = create_dir(testdir.path/"sub1");
+  __gnu_test::scoped_file file1(sub1.path / "file");
+  const auto sub2 = create_dir(testdir.path/"sub2");
+  __gnu_test::scoped_file file2(sub2.path / "file");
+
+  fs::recursive_directory_iterator r(testdir.path);
+  ++r;
+  r.pop();
+  // recursion pending flag should be true after popping:
+  VERIFY( r.recursion_pending() );
+
+  // and recursion should actually happen:
+  ++r;
+  VERIFY( r.depth() == 1 );
+  VERIFY( r->is_regular_file() );
+  // recursion pending flag should still be true:
+  VERIFY( r.recursion_pending() );
+
+  r = fs::recursive_directory_iterator(testdir.path);
+  r.disable_recursion_pending();
+  ++r;
+  // when recursion is disabled, should not enter subdirectories:
+  VERIFY( r.depth() == 0 );
+  r.disable_recursion_pending();
+  ++r;
+  VERIFY( r == fs::recursive_directory_iterator() );
+}
+
+int main()
+{
+  test01();
+  test02();
+  test03();
+  test04();
+}
index 48f503b3c2766ba83f9dedb7126e1403e15e5409..baba90eaa0cca0ffa26c9689f63cf30fc0da4962 100644 (file)
@@ -143,6 +143,9 @@ namespace __gnu_test
 
     ~scoped_file() { if (!path.empty()) remove(path); }
 
+    scoped_file(scoped_file&&) = default;
+    scoped_file& operator=(scoped_file&&) = default;
+
     path_type path;
   };