Fix bugs in recursive_directory_iterator::increment(ec) implementation and tests.

r273060 didn't completely fix the issues in recursive_directory_iterator and
the tests. This patch follows up with more fixes

* Fix bug where recursive_directory_iterator::increment(ec) did not reset
  the error code if no failure occurred.

* Fix bad assertion in the recursive_directory_iterator::increment(ec) test
  that would only fire for certain iteration orders.

llvm-svn: 273070

2 files changed, 23 insertions, 18 deletions

diff --git a/libcxx/src/experimental/filesystem/directory_iterator.cpp b/libcxx/src/experimental/filesystem/directory_iterator.cpp
index cd9d8679f9c..fa217ba7a12 100644
--- a/libcxx/src/experimental/filesystem/directory_iterator.cpp
+++ b/libcxx/src/experimental/filesystem/directory_iterator.cpp
@@ -179,13 +179,12 @@ recursive_directory_iterator::recursive_directory_iterator(const path& p,
 void recursive_directory_iterator::__pop(error_code* ec)
 {
     _LIBCPP_ASSERT(__imp_, "Popping the end iterator");
+    if (ec) ec->clear();
     __imp_->__stack_.pop();
-    if (__imp_->__stack_.size() == 0) {
+    if (__imp_->__stack_.size() == 0)
         __imp_.reset();
-        if (ec) ec->clear();
-    } else {
+    else
         __advance(ec);
-    }
 }
 
 directory_options recursive_directory_iterator::options() const {
@@ -203,6 +202,7 @@ const directory_entry& recursive_directory_iterator::__deref() const {
 recursive_directory_iterator&
 recursive_directory_iterator::__increment(error_code *ec)
 {
+    if (ec) ec->clear();
     if (recursion_pending()) {
         if (__try_recursion(ec) || (ec && *ec))
             return *this;
@@ -213,22 +213,19 @@ recursive_directory_iterator::__increment(error_code *ec)
 }
 
 void recursive_directory_iterator::__advance(error_code* ec) {
+    // REQUIRES: ec must be cleared before calling this function.
     const directory_iterator end_it;
     auto& stack = __imp_->__stack_;
     std::error_code m_ec;
     while (stack.size() > 0) {
-        if (stack.top().advance(m_ec)) {
-            if (ec) ec->clear();
+        if (stack.top().advance(m_ec))
             return;
-        }
         if (m_ec) break;
         stack.pop();
     }
     __imp_.reset();
     if (m_ec)
         set_or_throw(m_ec, ec, "recursive_directory_iterator::operator++()");
-    else if (ec)
-        ec->clear();
 }
 
 bool recursive_directory_iterator::__try_recursion(error_code *ec) {
diff --git a/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp b/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
index db2fc1533b7..e67fc2f20f6 100644
--- a/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
+++ b/libcxx/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
@@ -194,16 +194,24 @@ TEST_CASE(access_denied_on_recursion_test_case)
         recursive_directory_iterator it(startDir, SkipEPerm, ec);
         TEST_REQUIRE(!ec);
         TEST_REQUIRE(it != endIt);
-        const path elem = *it;
-        if (elem == permDeniedDir) {
-            it.increment(ec);
-            TEST_REQUIRE(!ec);
-            TEST_REQUIRE(it != endIt);
-            TEST_CHECK(*it == otherFile);
-        } else if (elem == otherFile) {
-            it.increment(ec);
-            TEST_REQUIRE(!ec);
+
+        bool seenOtherFile = false;
+        if (*it == otherFile) {
+            ++it;
+            seenOtherFile = true;
+            TEST_REQUIRE (it != endIt);
+        }
+        TEST_REQUIRE(*it == permDeniedDir);
+
+        ec = GetTestEC();
+        it.increment(ec);
+        TEST_REQUIRE(!ec);
+
+        if (seenOtherFile) {
             TEST_CHECK(it == endIt);
+        } else {
+            TEST_CHECK(it != endIt);
+            TEST_CHECK(*it == otherFile);
         }
     }
     // Test that construction resulting in a "EACCESS" error is not ignored