Diagnose when reverse_iterator is used on path::iterator.
path::iterator isn't a strictly conforming iterator. Specifically
it stashes the current element inside the iterator. This leads to
UB when used with reverse_iterator since it requires the element
to outlive the lifetime of the iterator.
This patch adds a static_assert inside reverse_iterator to disallow
"stashing iterator types", and it tags path::iterator as such a type.
Additionally this patch removes all uses of reverse_iterator<path::iterator>
within the tests.
llvm-svn: 300164
Cr-Mirrored-From: sso://chromium.googlesource.com/_direct/external/github.com/llvm/llvm-project
Cr-Mirrored-Commit: e02ed1c255d717827268fd0789148a06df5d0970
diff --git a/include/experimental/filesystem b/include/experimental/filesystem
index 04180d1..42157ba 100644
--- a/include/experimental/filesystem
+++ b/include/experimental/filesystem
@@ -1092,20 +1092,12 @@
public:
typedef bidirectional_iterator_tag iterator_category;
- // FIXME: As of 3/April/2017 Clang warns on `value_type` shadowing the
- // definition in path. Clang should be fixed and this should be removed.
-#if defined(__clang__)
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wshadow"
-#endif
typedef path value_type;
-#if defined(__clang__)
-#pragma clang diagnostic pop
-#endif
-
typedef std::ptrdiff_t difference_type;
typedef const path* pointer;
typedef const path& reference;
+
+ typedef void __stashing_iterator_tag; // See reverse_iterator and __is_stashing_iterator
public:
_LIBCPP_INLINE_VISIBILITY
iterator() : __stashed_elem_(), __path_ptr_(nullptr),
diff --git a/include/iterator b/include/iterator
index b8f6570..66d1a31 100644
--- a/include/iterator
+++ b/include/iterator
@@ -615,6 +615,14 @@
return __x;
}
+
+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 {};
+
template <class _Iter>
class _LIBCPP_TEMPLATE_VIS reverse_iterator
: public iterator<typename iterator_traits<_Iter>::iterator_category,
@@ -625,6 +633,11 @@
{
private:
/*mutable*/ _Iter __t; // no longer used as of LWG #2360, not removed due to ABI break
+
+ 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/test/libcxx/experimental/filesystem/class.path/path.itr/reverse_iterator_produces_diagnostic.fail.cpp b/test/libcxx/experimental/filesystem/class.path/path.itr/reverse_iterator_produces_diagnostic.fail.cpp
new file mode 100644
index 0000000..6f839be
--- /dev/null
+++ b/test/libcxx/experimental/filesystem/class.path/path.itr/reverse_iterator_produces_diagnostic.fail.cpp
@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// <experimental/filesystem>
+
+// class path
+
+#include <experimental/filesystem>
+#include <iterator>
+
+
+namespace fs = std::experimental::filesystem;
+
+int main() {
+ using namespace fs;
+ using RIt = std::reverse_iterator<path::iterator>;
+
+ // expected-error@iterator:* {{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);
+ }
+}
diff --git a/test/std/experimental/filesystem/class.path/path.member/path.decompose/path.decompose.pass.cpp b/test/std/experimental/filesystem/class.path/path.member/path.decompose/path.decompose.pass.cpp
index 4c83481..078e006 100644
--- a/test/std/experimental/filesystem/class.path/path.member/path.decompose/path.decompose.pass.cpp
+++ b/test/std/experimental/filesystem/class.path/path.member/path.decompose/path.decompose.pass.cpp
@@ -54,12 +54,6 @@
#include "count_new.hpp"
#include "filesystem_test_helper.hpp"
-template <class It>
-std::reverse_iterator<It> mkRev(It it) {
- return std::reverse_iterator<It>(it);
-}
-
-
namespace fs = std::experimental::filesystem;
struct PathDecomposeTestcase
{
@@ -147,7 +141,11 @@
assert(checkCollectionsEqual(p.begin(), p.end(),
TC.elements.begin(), TC.elements.end()));
// check backwards
- assert(checkCollectionsEqual(mkRev(p.end()), mkRev(p.begin()),
+
+ std::vector<fs::path> Parts;
+ for (auto it = p.end(); it != p.begin(); )
+ Parts.push_back(*--it);
+ assert(checkCollectionsEqual(Parts.begin(), Parts.end(),
TC.elements.rbegin(), TC.elements.rend()));
}
}