[libc++] [API BREAK] Change `fs::path::iterator::iterator_category` to `input_iterato...

This essentially reverts e02ed1c255d71 and puts in a new fix, which makes `path::iterator`
a true C++20 `bidirectional_iterator`, but downgrades it to an `input_iterator` in C++17.

Fixes #37852.

Differential Revision: https://reviews.llvm.org/D116489

diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
index ef08d6d..8f57df3 100644 (file)
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -97,6 +97,18 @@ API Changes
   from all modes. Their symbols are still provided by the dynamic library for the benefit of
   existing compiled code. All of these functions have always behaved as no-ops.
 
+- ``std::filesystem::path::iterator``, which (in our implementation) stashes
+  a ``path`` value inside itself similar to ``istream_iterator``, now sets its
+  ``reference`` type to ``path`` and its ``iterator_category`` to ``input_iterator_tag``,
+  so that it is a conforming input iterator in C++17 and a conforming
+  ``std::bidirectional_iterator`` in C++20. Before this release, it had set its
+  ``reference`` type to ``const path&`` and its ``iterator_category`` to
+  ``bidirectional_iterator_tag``, making it a non-conforming bidirectional iterator.
+  After this change, ``for`` loops of the form ``for (auto& c : path)`` must be rewritten
+  as either ``for (auto&& c : path)`` or ``for (const auto& c : path)``.
+  ``std::reverse_iterator<path::iterator>`` is no longer rejected.
+
+
 ABI Changes
 -----------
 

diff --git a/libcxx/include/__filesystem/path_iterator.h b/libcxx/include/__filesystem/path_iterator.h
index 55e97b1..08039e4 100644 (file)
--- a/libcxx/include/__filesystem/path_iterator.h
+++ b/libcxx/include/__filesystem/path_iterator.h
@@ -38,15 +38,13 @@ public:
   };
 
 public:
-  typedef bidirectional_iterator_tag iterator_category;
+  typedef input_iterator_tag iterator_category;
+  typedef bidirectional_iterator_tag iterator_concept;
 
   typedef path value_type;
   typedef ptrdiff_t difference_type;
   typedef const path* pointer;
-  typedef const path& reference;
-
-  typedef void
-      __stashing_iterator_tag; // See reverse_iterator and __is_stashing_iterator
+  typedef path reference;
 
 public:
   _LIBCPP_INLINE_VISIBILITY

diff --git a/libcxx/include/__iterator/reverse_iterator.h b/libcxx/include/__iterator/reverse_iterator.h
index 7807b80..4540545 100644 (file)
--- a/libcxx/include/__iterator/reverse_iterator.h
+++ b/libcxx/include/__iterator/reverse_iterator.h
@@ -24,13 +24,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <class _Tp, class = void>
-struct __is_stashing_iterator : false_type {};
-
-template <class _Tp>
-struct __is_stashing_iterator<_Tp, typename __void_t<typename _Tp::__stashing_iterator_tag>::type>
-  : true_type {};
-
 _LIBCPP_SUPPRESS_DEPRECATED_PUSH
 template <class _Iter>
 class _LIBCPP_TEMPLATE_VIS reverse_iterator
@@ -48,10 +41,6 @@ private:
     _Iter __t; // no longer used as of LWG #2360, not removed due to ABI break
 #endif
 
-    static_assert(!__is_stashing_iterator<_Iter>::value,
-      "The specified iterator type cannot be used with reverse_iterator; "
-      "Using stashing iterators with reverse_iterator causes undefined behavior");
-
 protected:
     _Iter current;
 public:

diff --git a/libcxx/test/libcxx/input.output/filesystems/class.path/path.itr/reverse_iterator_produces_diagnostic.verify.cpp b/libcxx/test/libcxx/input.output/filesystems/class.path/path.itr/reverse_iterator_produces_diagnostic.verify.cpp
deleted file mode 100644 (file)
index c4abf6d..0000000
--- a/libcxx/test/libcxx/input.output/filesystems/class.path/path.itr/reverse_iterator_produces_diagnostic.verify.cpp
+++ /dev/null
@@ -1,30 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// UNSUPPORTED: c++03
-
-// <filesystem>
-
-// class path
-
-#include "filesystem_include.h"
-#include <iterator>
-
-
-int main(int, char**) {
-  using namespace fs;
-  using RIt = std::reverse_iterator<path::iterator>;
-
-  // expected-error-re@*:* {{static_assert failed{{.*}} "The specified iterator type cannot be used with reverse_iterator; Using stashing iterators with reverse_iterator causes undefined behavior"}}
-  {
-    RIt r;
-    ((void)r);
-  }
-
-  return 0;
-}

diff --git a/libcxx/test/std/input.output/filesystems/class.path/path.itr/iterator.pass.cpp b/libcxx/test/std/input.output/filesystems/class.path/path.itr/iterator.pass.cpp
index adb6b1b..99cee94 100644 (file)
--- a/libcxx/test/std/input.output/filesystems/class.path/path.itr/iterator.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/class.path/path.itr/iterator.pass.cpp
@@ -19,28 +19,25 @@
 
 
 #include "filesystem_include.h"
+#include <cassert>
 #include <iterator>
 #include <type_traits>
-#include <cassert>
 
 #include "test_macros.h"
 #include "filesystem_test_helper.h"
 
-
-
-template <class It>
-std::reverse_iterator<It> mkRev(It it) {
-  return std::reverse_iterator<It>(it);
-}
-
 void checkIteratorConcepts() {
   using namespace fs;
   using It = path::iterator;
   using Traits = std::iterator_traits<It>;
-  ASSERT_SAME_TYPE(Traits::iterator_category, std::bidirectional_iterator_tag);
+  ASSERT_SAME_TYPE(path::const_iterator, It);
+#if TEST_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_RANGES)
+  static_assert(std::bidirectional_iterator<It>);
+#endif
   ASSERT_SAME_TYPE(Traits::value_type, path);
-  ASSERT_SAME_TYPE(Traits::pointer,   path const*);
-  ASSERT_SAME_TYPE(Traits::reference, path const&);
+  LIBCPP_STATIC_ASSERT(std::is_same<Traits::iterator_category, std::input_iterator_tag>::value, "");
+  LIBCPP_STATIC_ASSERT(std::is_same<Traits::pointer, path const*>::value, "");
+  LIBCPP_STATIC_ASSERT(std::is_same<Traits::reference, path>::value, "");
   {
     It it;
     ASSERT_SAME_TYPE(It&, decltype(++it));
@@ -93,16 +90,13 @@ void checkBeginEndBasic() {
 #endif
     assert(checkCollectionsEqual(p.begin(), p.end(), std::begin(expect), std::end(expect)));
     assert(checkCollectionsEqualBackwards(p.begin(), p.end(), std::begin(expect), std::end(expect)));
-
   }
   {
     path p("////foo/bar/baz///");
     const path expect[] = {"/", "foo", "bar", "baz", ""};
     assert(checkCollectionsEqual(p.begin(), p.end(), std::begin(expect), std::end(expect)));
     assert(checkCollectionsEqualBackwards(p.begin(), p.end(), std::begin(expect), std::end(expect)));
-
   }
-
 }
 
 int main(int, char**) {

diff --git a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.proximate/proximate.pass.cpp b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.proximate/proximate.pass.cpp
index cc89fb7..5555e11 100644 (file)
--- a/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.proximate/proximate.pass.cpp
+++ b/libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.proximate/proximate.pass.cpp
@@ -28,7 +28,7 @@
 
 static int count_path_elems(const fs::path& p) {
   int count = 0;
-  for (auto& elem : p) {
+  for (auto&& elem : p) {
     if (elem != p.root_name() && elem != "/" && elem != "")
       ++count;
   }