Index: llvm/trunk/include/llvm/Support/FileSystem.h =================================================================== --- llvm/trunk/include/llvm/Support/FileSystem.h +++ llvm/trunk/include/llvm/Support/FileSystem.h @@ -956,14 +956,16 @@ SmallString<128> path_storage; ec = detail::directory_iterator_construct( *State, path.toStringRef(path_storage), FollowSymlinks); + update_error_code_for_current_entry(ec); } explicit directory_iterator(const directory_entry &de, std::error_code &ec, bool follow_symlinks = true) : FollowSymlinks(follow_symlinks) { State = std::make_shared(); - ec = - detail::directory_iterator_construct(*State, de.path(), FollowSymlinks); + ec = detail::directory_iterator_construct( + *State, de.path(), FollowSymlinks); + update_error_code_for_current_entry(ec); } /// Construct end iterator. @@ -972,6 +974,7 @@ // No operator++ because we need error_code. directory_iterator &increment(std::error_code &ec) { ec = directory_iterator_increment(*State); + update_error_code_for_current_entry(ec); return *this; } @@ -993,6 +996,24 @@ } // Other members as required by // C++ Std, 24.1.1 Input iterators [input.iterators] + +private: + // Checks if current entry is valid and populates error code. For example, + // current entry may not exist due to broken symbol links. + void update_error_code_for_current_entry(std::error_code &ec) { + // Bail out if error has already occured earlier to avoid overwriting it. + if (ec) + return; + + // Empty directory entry is used to mark the end of an interation, it's not + // an error. + if (State->CurrentEntry == directory_entry()) + return; + + ErrorOr status = State->CurrentEntry.status(); + if (!status) + ec = status.getError(); + } }; namespace detail { @@ -1030,11 +1051,9 @@ if (State->HasNoPushRequest) State->HasNoPushRequest = false; else { - ErrorOr st = State->Stack.top()->status(); - if (!st) return *this; - if (is_directory(*st)) { + ErrorOr status = State->Stack.top()->status(); + if (status && is_directory(*status)) { State->Stack.push(directory_iterator(*State->Stack.top(), ec, Follow)); - if (ec) return *this; if (State->Stack.top() != end_itr) { ++State->Level; return *this; Index: llvm/trunk/tools/llvm-cov/CodeCoverage.cpp =================================================================== --- llvm/trunk/tools/llvm-cov/CodeCoverage.cpp +++ llvm/trunk/tools/llvm-cov/CodeCoverage.cpp @@ -199,7 +199,7 @@ if (PathRemapping) addCollectedPath(Path); else - error("Missing source file", Path); + warning("Source file doesn't exist, proceeded by ignoring it.", Path); return; } @@ -211,12 +211,16 @@ if (llvm::sys::fs::is_directory(Status)) { std::error_code EC; for (llvm::sys::fs::recursive_directory_iterator F(Path, EC), E; - F != E && !EC; F.increment(EC)) { + F != E; F.increment(EC)) { + + if (EC) { + warning(EC.message(), F->path()); + continue; + } + if (llvm::sys::fs::is_regular_file(F->path())) addCollectedPath(F->path()); } - if (EC) - warning(EC.message(), Path); } } Index: llvm/trunk/unittests/Support/Path.cpp =================================================================== --- llvm/trunk/unittests/Support/Path.cpp +++ llvm/trunk/unittests/Support/Path.cpp @@ -885,58 +885,91 @@ fs::create_link("no_such_file", Twine(TestDirectory) + "/symlink/e")); typedef std::vector v_t; - v_t visited; - - // The directory iterator doesn't stat the file, so we should be able to - // iterate over the whole directory. + v_t VisitedNonBrokenSymlinks; + v_t VisitedBrokenSymlinks; std::error_code ec; + + // Broken symbol links are expected to throw an error. for (fs::directory_iterator i(Twine(TestDirectory) + "/symlink", ec), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); + continue; + } + ASSERT_NO_ERROR(ec); - visited.push_back(path::filename(i->path())); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - v_t expected = {"a", "b", "c", "d", "e"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); - visited.clear(); - - // The recursive directory iterator has to stat the file, so we need to skip - // the broken symlinks. - for (fs::recursive_directory_iterator - i(Twine(TestDirectory) + "/symlink", ec), - e; - i != e; i.increment(ec)) { - ASSERT_NO_ERROR(ec); - - ErrorOr status = i->status(); - if (status.getError() == - std::make_error_code(std::errc::no_such_file_or_directory)) { - i.no_push(); + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + v_t ExpectedNonBrokenSymlinks = {"b", "d"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + v_t ExpectedBrokenSymlinks = {"a", "c", "e"}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); + + // Broken symbol links are expected to throw an error. + for (fs::recursive_directory_iterator i( + Twine(TestDirectory) + "/symlink", ec), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); continue; } - visited.push_back(path::filename(i->path())); + ASSERT_NO_ERROR(ec); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - expected = {"b", "bb", "d", "da", "dd", "ddd", "ddd"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); - visited.clear(); - - // This recursive directory iterator doesn't follow symlinks, so we don't need - // to skip them. - for (fs::recursive_directory_iterator - i(Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false), - e; + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + ExpectedNonBrokenSymlinks = {"b", "bb", "d", "da", "dd", "ddd", "ddd"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + ExpectedBrokenSymlinks = {"a", "ba", "bc", "c", "e"}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); + + for (fs::recursive_directory_iterator i( + Twine(TestDirectory) + "/symlink", ec, /*follow_symlinks=*/false), e; i != e; i.increment(ec)) { + if (ec == std::make_error_code(std::errc::no_such_file_or_directory)) { + VisitedBrokenSymlinks.push_back(path::filename(i->path())); + continue; + } + ASSERT_NO_ERROR(ec); - visited.push_back(path::filename(i->path())); + VisitedNonBrokenSymlinks.push_back(path::filename(i->path())); } - std::sort(visited.begin(), visited.end()); - expected = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd", "ddd", "e"}; - ASSERT_TRUE(visited.size() == expected.size()); - ASSERT_TRUE(std::equal(visited.begin(), visited.end(), expected.begin())); + std::sort(VisitedNonBrokenSymlinks.begin(), VisitedNonBrokenSymlinks.end()); + std::sort(VisitedBrokenSymlinks.begin(), VisitedBrokenSymlinks.end()); + ExpectedNonBrokenSymlinks = {"a", "b", "ba", "bb", "bc", "c", "d", "da", "dd", + "ddd", "e"}; + ASSERT_EQ(ExpectedNonBrokenSymlinks.size(), VisitedNonBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedNonBrokenSymlinks.begin(), + VisitedNonBrokenSymlinks.end(), + ExpectedNonBrokenSymlinks.begin())); + VisitedNonBrokenSymlinks.clear(); + + ExpectedBrokenSymlinks = {}; + ASSERT_EQ(ExpectedBrokenSymlinks.size(), VisitedBrokenSymlinks.size()); + ASSERT_TRUE(std::equal(VisitedBrokenSymlinks.begin(), + VisitedBrokenSymlinks.end(), + ExpectedBrokenSymlinks.begin())); + VisitedBrokenSymlinks.clear(); ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) + "/symlink")); }