Fixes errors with FS iterators caused by https://reviews.llvm.org/D44960

[Dor1s]

Summary

In https://reviews.llvm.org/D44960, file status check is executed every
time a real file system directory iterator is constructed or
incremented, and emits an error code. This change list fixes the errors
in VirtualFileSystem caused by https://reviews.llvm.org/D44960.

Patch by Yuke Liao (@liaoyuke).

[liaoyuke]

Fix styles.

[Dor1s]

Looks good from my perspective, added Peter and Zachary who might know this code better :)

[Dor1s]

Friendly ping. We'll probably land it tomorrow anyway, as it blocks another fix needed for llvm-cov, but having another pair of eyes to look at this would be still helpful :)

[vsk]

I'd like to test this out a bit more. Could you wait one more day?

[vsk]

Lgtm, thank you.

If you don't have commit access, let us know, and somebody will commit this on your behalf.

[Dor1s]

Thanks a lot, Vedant! No worries, I'll commit it on behalf of Yuke @liaoyuke

[Dor1s]

Resolve merge conflicts and fix whitespaces.

lib/Basic/VirtualFileSystem.cpp

IntrusiveRefCntPtr<FileSystem> vfs::getRealFileSystem() {
   return FS;
 }
 
 namespace {
 class RealFSDirIter : public clang::vfs::detail::DirIterImpl {
   llvm::sys::fs::directory_iterator Iter;
 public:
   RealFSDirIter(const Twine &Path, std::error_code &EC) : Iter(Path, EC) {
-    if (!EC && Iter != llvm::sys::fs::directory_iterator()) {
+    if (Iter != llvm::sys::fs::directory_iterator()) {
       llvm::sys::fs::file_status S;
-      EC = llvm::sys::fs::status(Iter->path(), S, true);
+      std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
       CurrentEntry = Status::copyWithNewName(S, Iter->path());
+      if (!EC)
+        EC = ErrorCode;
     }
   }
 
   std::error_code increment() override {
     std::error_code EC;
     Iter.increment(EC);
-    if (EC) {
-      return EC;
-    } else if (Iter == llvm::sys::fs::directory_iterator()) {
-      CurrentEntry = Status();
-    } else {
-      llvm::sys::fs::file_status S;
-      EC = llvm::sys::fs::status(Iter->path(), S, true);
-      CurrentEntry = Status::copyWithNewName(S, Iter->path());
-    }
+    if (Iter == llvm::sys::fs::directory_iterator()) {
+      CurrentEntry = Status();
+    } else {
+      llvm::sys::fs::file_status S;
+      std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
+      CurrentEntry = Status::copyWithNewName(S, Iter->path());
+      if (!EC)
+        EC = ErrorCode;
+    }
+
     return EC;
   }
 };
 }
 
 directory_iterator RealFileSystem::dir_begin(const Twine &Dir,
                                              std::error_code &EC) {
   return directory_iterator(std::make_shared<RealFSDirIter>(Dir, EC));

unittests/Basic/VirtualFileSystemTest.cpp

TEST(VirtualFileSystemTest, BrokenSymlinkRealFSRecursiveIteration) {
   ScopedLink _ba("no_such_file", TestDirectory + "/b/a");
   ScopedDir _bb(TestDirectory + "/b/b");
   ScopedLink _bc("no_such_file", TestDirectory + "/b/c");
   ScopedLink _c("no_such_file", TestDirectory + "/c");
   ScopedDir _d(TestDirectory + "/d");
   ScopedDir _dd(TestDirectory + "/d/d");
   ScopedDir _ddd(TestDirectory + "/d/d/d");
   ScopedLink _e("no_such_file", TestDirectory + "/e");
-  std::vector<StringRef> Expected = {_b, _bb, _d, _dd, _ddd};
 
-  std::vector<std::string> Contents;
+  std::vector<StringRef> ExpectedBrokenSymlinks = {_a, _ba, _bc, _c, _e};
+  std::vector<StringRef> ExpectedNonBrokenSymlinks = {_b, _bb, _d, _dd, _ddd};
+  std::vector<std::string> VisitedBrokenSymlinks;
+  std::vector<std::string> VisitedNonBrokenSymlinks;
   std::error_code EC;
   for (vfs::recursive_directory_iterator I(*FS, Twine(TestDirectory), EC), E;
        I != E; I.increment(EC)) {
-    // Skip broken symlinks.
     auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
     if (EC == EC2) {
-      EC.clear();
+      VisitedBrokenSymlinks.push_back(I->getName());
       continue;
     }
     // For bot debugging.
     if (EC) {
       outs() << "Error code found:\n"
              << "EC value: " << EC.value() << "\n"
              << "EC category: " << EC.category().name()
              << "EC message: " << EC.message() << "\n";
 
       outs() << "Error code tested for:\n"
              << "EC value: " << EC2.value() << "\n"
              << "EC category: " << EC2.category().name()
              << "EC message: " << EC2.message() << "\n";
     }
     ASSERT_FALSE(EC);
-    Contents.push_back(I->getName());
+    VisitedNonBrokenSymlinks.push_back(I->getName());
   }
 
-  // Check sorted contents.
-  std::sort(Contents.begin(), Contents.end());
-  EXPECT_EQ(Expected.size(), Contents.size());
-  EXPECT_TRUE(std::equal(Contents.begin(), Contents.end(), Expected.begin()));
+  // Check visited file names.
+  std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end());
+  std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end());
+  EXPECT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedBrokenSymlinks.begin(),
+                         VisitedBrokenSymlinks.end(),
+                         ExpectedBrokenSymlinks.begin()));
+  EXPECT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size());
+  EXPECT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(),
+                         VisitedNonBrokenSymlinks.end(),
+                         ExpectedNonBrokenSymlinks.begin()));
 }
 #endif
 
 template <typename DirIter>
 static void checkContents(DirIter I, ArrayRef<StringRef> ExpectedOut) {
   std::error_code EC;
   SmallVector<StringRef, 4> Expected(ExpectedOut.begin(), ExpectedOut.end());
   SmallVector<std::string, 4> InputToCheck;