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. Tested powerpc64le-linux, committed to trunk.
commit fd5e47d7a20365c880d1c80425640abb323cc832 Author: Jonathan Wakely <jwak...@redhat.com> Date: Fri Apr 5 13:38:15 2019 +0100 Share all recursive_directory_iterator state [LWG 2708] 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. diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index c4f12152147..019b581df71 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -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; diff --git a/libstdc++-v3/include/bits/fs_dir.h b/libstdc++-v3/include/bits/fs_dir.h index fd2da0f25c1..a5947b39541 100644 --- a/libstdc++-v3/include/bits/fs_dir.h +++ b/libstdc++-v3/include/bits/fs_dir.h @@ -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 diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc index 629c4ebf78f..d8c48f6d6d8 100644 --- a/libstdc++-v3/src/c++17/fs_dir.cc +++ b/libstdc++-v3/src/c++17/fs_dir.cc @@ -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 index 00000000000..c837ce18959 --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/filesystem/iterators/recursion_pending.cc @@ -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(); +} diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h index 48f503b3c27..baba90eaa0c 100644 --- a/libstdc++-v3/testsuite/util/testsuite_fs.h +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h @@ -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; };