Support: Have directory_iterator::status() return FindFirstFileEx/FindNextFile results on Windows.

[pcc]

Summary

This allows clients to avoid an unnecessary fs::status() call on each
directory entry. Because the information returned by FindFirstFileEx
is a subset of the information returned by a regular status() call,
I needed to extract a base class from file_status that contains only
that information.

On my machine, this reduces the time required to enumerate a ThinLTO
cache directory containing 520k files from almost 4 minutes to less
than 2 seconds.

[amccarth]

Just a drive-by comment.

clang-tools-extra/modularize/CoverageChecker.cpp

250 ↗

(On Diff #118314)

Just a thought...

Is there much value in explicitly declaring the return type, especially since it happens a lot in this patch? I know the LLVM Coding Standards don't want us to always use auto, but, here, the type is so garrulous that it kind of obscures what's going on. Consider:

const auto ErrorOrStatus = I->status();
if (!ErrorOrStatus)
  return false;
ss::fs::file_type Type = ErrorOrStatus->type();

Another thought is that ErrorOr<> might be overkill for methods that get the file or directory status. Perhaps any error in retrieving the status should be represented in the status object itself. The most likely error is probably that the file or directory doesn't exist. Given that there's an exists() function that takes a status, it seems status needs to be able to represent some error conditions anyway.

[pcc]

clang-tools-extra/modularize/CoverageChecker.cpp

250 ↗

(On Diff #118314)

1. Yeah, it's a little ugly in this file, but in most of the rest of the code we have using declarations which mitigate this somewhat. I don't feel so strongly about it that I'd want to deviate from the style guide.

2. Maybe. The alternative solution, of course, would be to remove the ability to represent error conditions in file_status. It may be worth looking at which of those makes the code more readable, but it seems beyond the scope of this patch.

[zturner]

4 minutes to 2 seconds is pretty impressive!

clang-tools-extra/modularize/CoverageChecker.cpp

250 ↗

(On Diff #118314)

Eh, the style guide says "use auto where it makes the code more readable and easier to maintain". Seems to fit the bill here. That said, it's also minor and like you I don't feel strongly enough to push one way or the other.

llvm/include/llvm/Support/FileSystem.h

169 ↗

(On Diff #118314)

Should this be explicit?

235 ↗

(On Diff #118314)

Should this be marked explicit?

413 ↗

(On Diff #118314)

I just noticed all of these functions pass by value. I can't see a good reason for that, so can you make them pass by const reference (especially since now you'll be slicing otherwise).

llvm/lib/Support/CachePruning.cpp

186 ↗

(On Diff #118314)

Unrelated, but I find it surprising this is using a std::set. Wouldn't you get better performance by using a StringMap<uint64_t>? or a DenseSet<std::pair<uint64_t, std::string>>?

[pcc]

• Address review comments

llvm/include/llvm/Support/FileSystem.h

169 ↗

(On Diff #118314)

Makes sense, done.

235 ↗

(On Diff #118314)

Done.

413 ↗

(On Diff #118314)

Done.

llvm/lib/Support/CachePruning.cpp

186 ↗

(On Diff #118314)

I think we need to keep the set ordered by size for determinism (below we end up deleting in reverse set order, i.e. in decreasing order of size), and neither of those data structures will keep that ordering, I believe. I've added a comment here.

cfe/trunk/lib/Basic/VirtualFileSystem.cpp

 
 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()) {
       llvm::sys::fs::file_status S;
-      EC = Iter->status(S);
+      EC = llvm::sys::fs::status(Iter->path(), S, true);
       CurrentEntry = Status::copyWithNewName(S, Iter->path());
     }
   }
 
   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 = Iter->status(S);
+      EC = llvm::sys::fs::status(Iter->path(), S, true);
       CurrentEntry = Status::copyWithNewName(S, Iter->path());
     }
     return EC;
   }
 };
 }
 
 directory_iterator RealFileSystem::dir_begin(const Twine &Dir,

clang-tools-extra/trunk/modularize/CoverageChecker.cpp

bool CoverageChecker::collectUmbrellaHeaders(StringRef UmbrellaDirName) {
   // Initialize directory name.
   SmallString<256> Directory(ModuleMapDirectory);
   if (UmbrellaDirName.size())
     sys::path::append(Directory, UmbrellaDirName);
   if (Directory.size() == 0)
     Directory = ".";
   // Walk the directory.
   std::error_code EC;
-  sys::fs::file_status Status;
   for (sys::fs::directory_iterator I(Directory.str(), EC), E; I != E;
     I.increment(EC)) {
     if (EC)
       return false;
     std::string File(I->path());
-    I->status(Status);
-    sys::fs::file_type Type = Status.type();
+    llvm::ErrorOr<sys::fs::basic_file_status> Status = I->status();
+    if (!Status)
+      return false;
+    sys::fs::file_type Type = Status->type();
     // If the file is a directory, ignore the name and recurse.
     if (Type == sys::fs::file_type::directory_file) {
       if (!collectUmbrellaHeaders(File))
         return false;
       continue;
     }
     // If the file does not have a common header extension, ignore it.
     if (!ModularizeUtilities::isHeader(File))
if (IncludePath.startswith("/") || IncludePath.startswith("\\") ||
     ((IncludePath.size() >= 2) && (IncludePath[1] == ':'))) {
     llvm::errs() << "error: Include path \"" << IncludePath
       << "\" is not relative to the module map file.\n";
     return false;
   }
 
   // Recursively walk the directory tree.
   std::error_code EC;
-  sys::fs::file_status Status;
   int Count = 0;
   for (sys::fs::recursive_directory_iterator I(Directory.str(), EC), E; I != E;
     I.increment(EC)) {
     if (EC)
       return false;
     //std::string file(I->path());
     StringRef file(I->path());
-    I->status(Status);
-    sys::fs::file_type type = Status.type();
+    llvm::ErrorOr<sys::fs::basic_file_status> Status = I->status();
+    if (!Status)
+      return false;
+    sys::fs::file_type type = Status->type();
     // If the file is a directory, ignore the name (but still recurses).
     if (type == sys::fs::file_type::directory_file)
       continue;
     // Assume directories or files starting with '.' are private and not to
     // be considered.
     if ((file.find("\\.") != StringRef::npos) ||
         (file.find("/.") != StringRef::npos))
       continue;

clang-tools-extra/trunk/modularize/ModularizeUtilities.cpp

 
 // Collect headers from an umbrella directory.
 bool ModularizeUtilities::collectUmbrellaHeaders(StringRef UmbrellaDirName,
   DependentsVector &Dependents) {
   // Initialize directory name.
   SmallString<256> Directory(UmbrellaDirName);
   // Walk the directory.
   std::error_code EC;
-  llvm::sys::fs::file_status Status;
   for (llvm::sys::fs::directory_iterator I(Directory.str(), EC), E; I != E;
     I.increment(EC)) {
     if (EC)
       return false;
     std::string File(I->path());
-    I->status(Status);
-    llvm::sys::fs::file_type Type = Status.type();
+    llvm::ErrorOr<llvm::sys::fs::basic_file_status> Status = I->status();
+    if (!Status)
+      return false;
+    llvm::sys::fs::file_type Type = Status->type();
     // If the file is a directory, ignore the name and recurse.
     if (Type == llvm::sys::fs::file_type::directory_file) {
       if (!collectUmbrellaHeaders(File, Dependents))
         return false;
       continue;
     }
     // If the file does not have a common header extension, ignore it.
     if (!isHeader(File))

lldb/trunk/source/Commands/CommandCompletions.cpp

for (; Iter != End && !EC; Iter.increment(EC)) {
     auto Name = path::filename(Entry.path());
 
     // Omit ".", ".."
     if (Name == "." || Name == ".." || !Name.startswith(PartialItem))
       continue;
 
     // We have a match.
 
-    fs::file_status st;
-    if ((EC = Entry.status(st)))
+    llvm::ErrorOr<fs::basic_file_status> st = Entry.status();
+    if (!st)
       continue;
 
     // If it's a symlink, then we treat it as a directory as long as the target
     // is a directory.
-    bool is_dir = fs::is_directory(st);
-    if (fs::is_symlink_file(st)) {
+    bool is_dir = fs::is_directory(*st);
+    if (fs::is_symlink_file(*st)) {
       fs::file_status target_st;
       if (!fs::status(Entry.path(), target_st))
         is_dir = fs::is_directory(target_st);
     }
     if (only_directories && !is_dir)
       continue;
 
     // Shrink it back down so that it just has the original prefix the user

lldb/trunk/source/Utility/FileSpec.cpp

void FileSpec::EnumerateDirectory(llvm::StringRef dir_path,
                                   EnumerateDirectoryCallbackType callback,
                                   void *callback_baton) {
   namespace fs = llvm::sys::fs;
   std::error_code EC;
   fs::recursive_directory_iterator Iter(dir_path, EC);
   fs::recursive_directory_iterator End;
   for (; Iter != End && !EC; Iter.increment(EC)) {
     const auto &Item = *Iter;
-    fs::file_status Status;
-    if ((EC = Item.status(Status)))
+    llvm::ErrorOr<fs::basic_file_status> Status = Item.status();
+    if (!Status)
       break;
-    if (!find_files && fs::is_regular_file(Status))
+    if (!find_files && fs::is_regular_file(*Status))
       continue;
-    if (!find_directories && fs::is_directory(Status))
+    if (!find_directories && fs::is_directory(*Status))
       continue;
-    if (!find_other && fs::is_other(Status))
+    if (!find_other && fs::is_other(*Status))
       continue;
 
     FileSpec Spec(Item.path(), false);
-    auto Result = callback(callback_baton, Status.type(), Spec);
+    auto Result = callback(callback_baton, Status->type(), Spec);
     if (Result == eEnumerateDirectoryResultQuit)
       return;
     if (Result == eEnumerateDirectoryResultNext) {
       // Default behavior is to recurse.  Opt out if the callback doesn't want
       // this behavior.
       Iter.no_push();
     }
   }

llvm/trunk/include/llvm/Support/FileSystem.h

public:
   bool operator<(const UniqueID &Other) const {
     return std::tie(Device, File) < std::tie(Other.Device, Other.File);
   }
 
   uint64_t getDevice() const { return Device; }
   uint64_t getFile() const { return File; }
 };
 
-/// file_status - Represents the result of a call to stat and friends. It has
-///               a platform-specific member to store the result.
-class file_status
-{
-  friend bool equivalent(file_status A, file_status B);
-
+/// Represents the result of a call to directory_iterator::status(). This is a
+/// subset of the information returned by a regular sys::fs::status() call, and
+/// represents the information provided by Windows FileFirstFile/FindNextFile.
+class basic_file_status {
+protected:
   #if defined(LLVM_ON_UNIX)
-  dev_t fs_st_dev = 0;
-  nlink_t fs_st_nlinks = 0;
-  ino_t fs_st_ino = 0;
   time_t fs_st_atime = 0;
   time_t fs_st_mtime = 0;
   uid_t fs_st_uid = 0;
   gid_t fs_st_gid = 0;
   off_t fs_st_size = 0;
   #elif defined (LLVM_ON_WIN32)
-  uint32_t NumLinks = 0;
   uint32_t LastAccessedTimeHigh = 0;
   uint32_t LastAccessedTimeLow = 0;
   uint32_t LastWriteTimeHigh = 0;
   uint32_t LastWriteTimeLow = 0;
-  uint32_t VolumeSerialNumber = 0;
   uint32_t FileSizeHigh = 0;
   uint32_t FileSizeLow = 0;
-  uint32_t FileIndexHigh = 0;
-  uint32_t FileIndexLow = 0;
   #endif
   file_type Type = file_type::status_error;
   perms Perms = perms_not_known;
 
 public:
-  #if defined(LLVM_ON_UNIX)
-  file_status() = default;
+  basic_file_status() = default;
 
-  file_status(file_type Type) : Type(Type) {}
+  explicit basic_file_status(file_type Type) : Type(Type) {}
 
-  file_status(file_type Type, perms Perms, dev_t Dev, nlink_t Links, ino_t Ino,
-              time_t ATime, time_t MTime, uid_t UID, gid_t GID, off_t Size)
-      : fs_st_dev(Dev), fs_st_nlinks(Links), fs_st_ino(Ino), fs_st_atime(ATime),
-        fs_st_mtime(MTime), fs_st_uid(UID), fs_st_gid(GID), fs_st_size(Size),
-        Type(Type), Perms(Perms) {}
-  #elif defined(LLVM_ON_WIN32)
-  file_status() = default;
-
-  file_status(file_type Type) : Type(Type) {}
-
-  file_status(file_type Type, perms Perms, uint32_t LinkCount,
-              uint32_t LastAccessTimeHigh, uint32_t LastAccessTimeLow,
-              uint32_t LastWriteTimeHigh, uint32_t LastWriteTimeLow,
-              uint32_t VolumeSerialNumber, uint32_t FileSizeHigh,
-              uint32_t FileSizeLow, uint32_t FileIndexHigh,
-              uint32_t FileIndexLow)
-      : NumLinks(LinkCount), LastAccessedTimeHigh(LastAccessTimeHigh),
+  #if defined(LLVM_ON_UNIX)
+  basic_file_status(file_type Type, perms Perms, time_t ATime, time_t MTime,
+                    uid_t UID, gid_t GID, off_t Size)
+      : fs_st_atime(ATime), fs_st_mtime(MTime), fs_st_uid(UID), fs_st_gid(GID),
+        fs_st_size(Size), Type(Type), Perms(Perms) {}
+  basic_file_status(file_type Type, perms Perms, uint32_t LastAccessTimeHigh,
+                    uint32_t LastAccessTimeLow, uint32_t LastWriteTimeHigh,
+                    uint32_t LastWriteTimeLow, uint32_t FileSizeHigh,
+                    uint32_t FileSizeLow)
+      : LastAccessedTimeHigh(LastAccessTimeHigh),
         LastAccessedTimeLow(LastAccessTimeLow),
         LastWriteTimeHigh(LastWriteTimeHigh),
-        LastWriteTimeLow(LastWriteTimeLow),
-        VolumeSerialNumber(VolumeSerialNumber), FileSizeHigh(FileSizeHigh),
-        FileSizeLow(FileSizeLow), FileIndexHigh(FileIndexHigh),
-        FileIndexLow(FileIndexLow), Type(Type), Perms(Perms) {}
+        LastWriteTimeLow(LastWriteTimeLow), FileSizeHigh(FileSizeHigh),
+        FileSizeLow(FileSizeLow), Type(Type), Perms(Perms) {}
   #endif
 
   // getters
   file_type type() const { return Type; }
   perms permissions() const { return Perms; }
   TimePoint<> getLastAccessedTime() const;
   TimePoint<> getLastModificationTime() const;
-  UniqueID getUniqueID() const;
-  uint32_t getLinkCount() const;
 
   #if defined(LLVM_ON_UNIX)
   uint32_t getUser() const { return fs_st_uid; }
   uint32_t getGroup() const { return fs_st_gid; }
   uint64_t getSize() const { return fs_st_size; }
   #elif defined (LLVM_ON_WIN32)
   uint32_t getUser() const {
     return 9999; // Not applicable to Windows, so...
   }
 
   uint32_t getGroup() const {
     return 9999; // Not applicable to Windows, so...
   }
 
   uint64_t getSize() const {
     return (uint64_t(FileSizeHigh) << 32) + FileSizeLow;
   }
   #endif
 
   // setters
   void type(file_type v) { Type = v; }
   void permissions(perms p) { Perms = p; }
 };
 
+/// Represents the result of a call to sys::fs::status().
+class file_status : public basic_file_status {
+  friend bool equivalent(file_status A, file_status B);
+
+  #if defined(LLVM_ON_UNIX)
+  dev_t fs_st_dev = 0;
+  nlink_t fs_st_nlinks = 0;
+  ino_t fs_st_ino = 0;
+  #elif defined (LLVM_ON_WIN32)
+  uint32_t NumLinks = 0;
+  uint32_t VolumeSerialNumber = 0;
+  uint32_t FileIndexHigh = 0;
+  uint32_t FileIndexLow = 0;
+  #endif
+
+public:
+  file_status() = default;
+
+  explicit file_status(file_type Type) : basic_file_status(Type) {}
+
+  #if defined(LLVM_ON_UNIX)
+  file_status(file_type Type, perms Perms, dev_t Dev, nlink_t Links, ino_t Ino,
+              time_t ATime, time_t MTime, uid_t UID, gid_t GID, off_t Size)
+      : basic_file_status(Type, Perms, ATime, MTime, UID, GID, Size),
+        fs_st_dev(Dev), fs_st_nlinks(Links), fs_st_ino(Ino) {}
+  #elif defined(LLVM_ON_WIN32)
+  file_status(file_type Type, perms Perms, uint32_t LinkCount,
+              uint32_t LastAccessTimeHigh, uint32_t LastAccessTimeLow,
+              uint32_t LastWriteTimeHigh, uint32_t LastWriteTimeLow,
+              uint32_t VolumeSerialNumber, uint32_t FileSizeHigh,
+              uint32_t FileSizeLow, uint32_t FileIndexHigh,
+              uint32_t FileIndexLow)
+      : basic_file_status(Type, Perms, LastAccessTimeHigh, LastAccessTimeLow,
+                          LastWriteTimeHigh, LastWriteTimeLow, FileSizeHigh,
+                          FileSizeLow),
+        NumLinks(LinkCount), VolumeSerialNumber(VolumeSerialNumber),
+        FileIndexHigh(FileIndexHigh), FileIndexLow(FileIndexLow) {}
+  #endif
+
+  UniqueID getUniqueID() const;
+  uint32_t getLinkCount() const;
+};
+
 /// @}
 /// @name Physical Operators
 /// @{
 
 /// @brief Make \a path an absolute path.
 ///
 /// Makes \a path absolute using the \a current_directory if it is not already.
 /// An empty \a path will result in the \a current_directory.

 ErrorOr<MD5::MD5Result> md5_contents(const Twine &Path);
 
 /// @}
 /// @name Physical Observers
 /// @{
 
 /// @brief Does file exist?
 ///
-/// @param status A file_status previously returned from stat.
+/// @param status A basic_file_status previously returned from stat.
 /// @returns True if the file represented by status exists, false if it does
 ///          not.
-bool exists(file_status status);
+bool exists(const basic_file_status &status);
 
 enum class AccessMode { Exist, Write, Execute };
 
 /// @brief Can the file be accessed?
 ///
 /// @param Path Input path.
 /// @returns errc::success if the path can be accessed, otherwise a
 ///          platform-specific error_code.

 /// @param Path The path to get the type of.
 /// @param Follow For symbolic links, indicates whether to return the file type
 ///               of the link itself, or of the target.
 /// @returns A value from the file_type enumeration indicating the type of file.
 file_type get_file_type(const Twine &Path, bool Follow = true);
 
 /// @brief Does status represent a directory?
 ///
-/// @param status A file_status previously returned from status.
+/// @param status A basic_file_status previously returned from status.
 /// @returns status.type() == file_type::directory_file.
-bool is_directory(file_status status);
+bool is_directory(const basic_file_status &status);
 
 /// @brief Is path a directory?
 ///
 /// @param path Input path.
 /// @param result Set to true if \a path is a directory (after following
 ///               symlinks, false if it is not. Undefined otherwise.
 /// @returns errc::success if result has been successfully set, otherwise a
 ///          platform-specific error_code.
 std::error_code is_directory(const Twine &path, bool &result);
 
 /// @brief Simpler version of is_directory for clients that don't need to
 ///        differentiate between an error and false.
 inline bool is_directory(const Twine &Path) {
   bool Result;
   return !is_directory(Path, Result) && Result;
 }
 
 /// @brief Does status represent a regular file?
 ///
-/// @param status A file_status previously returned from status.
+/// @param status A basic_file_status previously returned from status.
 /// @returns status_known(status) && status.type() == file_type::regular_file.
-bool is_regular_file(file_status status);
+bool is_regular_file(const basic_file_status &status);
 
 /// @brief Is path a regular file?
 ///
 /// @param path Input path.
 /// @param result Set to true if \a path is a regular file (after following
 ///               symlinks), false if it is not. Undefined otherwise.
 /// @returns errc::success if result has been successfully set, otherwise a
 ///          platform-specific error_code.
 std::error_code is_regular_file(const Twine &path, bool &result);
 
 /// @brief Simpler version of is_regular_file for clients that don't need to
 ///        differentiate between an error and false.
 inline bool is_regular_file(const Twine &Path) {
   bool Result;
   if (is_regular_file(Path, Result))
     return false;
   return Result;
 }
 
 /// @brief Does status represent a symlink file?
 ///
-/// @param status A file_status previously returned from status.
+/// @param status A basic_file_status previously returned from status.
 /// @returns status_known(status) && status.type() == file_type::symlink_file.
-bool is_symlink_file(file_status status);
+bool is_symlink_file(const basic_file_status &status);
 
 /// @brief Is path a symlink file?
 ///
 /// @param path Input path.
 /// @param result Set to true if \a path is a symlink file, false if it is not.
 ///               Undefined otherwise.
 /// @returns errc::success if result has been successfully set, otherwise a
 ///          platform-specific error_code.
 std::error_code is_symlink_file(const Twine &path, bool &result);
 
 /// @brief Simpler version of is_symlink_file for clients that don't need to
 ///        differentiate between an error and false.
 inline bool is_symlink_file(const Twine &Path) {
   bool Result;
   if (is_symlink_file(Path, Result))
     return false;
   return Result;
 }
 
 /// @brief Does this status represent something that exists but is not a
 ///        directory or regular file?
 ///
-/// @param status A file_status previously returned from status.
+/// @param status A basic_file_status previously returned from status.
 /// @returns exists(s) && !is_regular_file(s) && !is_directory(s)
-bool is_other(file_status status);
+bool is_other(const basic_file_status &status);
 
 /// @brief Is path something that exists but is not a directory,
 ///        regular file, or symlink?
 ///
 /// @param path Input path.
 /// @param result Set to true if \a path exists, but is not a directory, regular
 ///               file, or a symlink, false if it does not. Undefined otherwise.
 /// @returns errc::success if result has been successfully set, otherwise a

 ///          platform-specific error_code or errc::function_not_supported on
 ///          platforms where the functionality isn't available.
 std::error_code setLastModificationAndAccessTime(int FD, TimePoint<> Time);
 
 /// @brief Is status available?
 ///
 /// @param s Input file status.
 /// @returns True if status() != status_error.
-bool status_known(file_status s);
+bool status_known(const basic_file_status &s);
 
 /// @brief Is status available?
 ///
 /// @param path Input path.
 /// @param result Set to true if status() != status_error.
 /// @returns errc::success if result has been successfully set, otherwise a
 ///          platform-specific error_code.
 std::error_code status_known(const Twine &path, bool &result);

 /// @{
 
 /// directory_entry - A single entry in a directory. Caches the status either
 /// from the result of the iteration syscall, or the first time status is
 /// called.
 class directory_entry {
   std::string Path;
   bool FollowSymlinks;
-  mutable file_status Status;
+  basic_file_status Status;
 
 public:
   explicit directory_entry(const Twine &path, bool follow_symlinks = true,
-                           file_status st = file_status())
+                           basic_file_status st = basic_file_status())
       : Path(path.str()), FollowSymlinks(follow_symlinks), Status(st) {}
 
   directory_entry() = default;
 
-  void assign(const Twine &path, file_status st = file_status()) {
+  void assign(const Twine &path, basic_file_status st = basic_file_status()) {
     Path = path.str();
     Status = st;
   }
 
-  void replace_filename(const Twine &filename, file_status st = file_status());
+  void replace_filename(const Twine &filename,
+                        basic_file_status st = basic_file_status());
 
   const std::string &path() const { return Path; }
-  std::error_code status(file_status &result) const;
+  ErrorOr<basic_file_status> status() const;
 
   bool operator==(const directory_entry& rhs) const { return Path == rhs.Path; }
   bool operator!=(const directory_entry& rhs) const { return !(*this == rhs); }
   bool operator< (const directory_entry& rhs) const;
   bool operator<=(const directory_entry& rhs) const;
   bool operator> (const directory_entry& rhs) const;
   bool operator>=(const directory_entry& rhs) const;
 };
public:
 
   // No operator++ because we need error_code.
   recursive_directory_iterator &increment(std::error_code &ec) {
     const directory_iterator end_itr = {};
 
     if (State->HasNoPushRequest)
       State->HasNoPushRequest = false;
     else {
-      file_status st;
-      if ((ec = State->Stack.top()->status(st))) return *this;
-      if (is_directory(st)) {
+      ErrorOr<basic_file_status> st = State->Stack.top()->status();
+      if (!st) return *this;
+      if (is_directory(*st)) {
         State->Stack.push(directory_iterator(*State->Stack.top(), ec, Follow));
         if (ec) return *this;
         if (State->Stack.top() != end_itr) {
           ++State->Level;
           return *this;
         }
         State->Stack.pop();
       }

llvm/trunk/lib/Support/CachePruning.cpp

if (auto EC = sys::fs::status(TimestampFile, FileStatus)) {
     // There is a benign race condition here, if two processes happen to
     // notice at the same time that the timestamp is out-of-date.
     writeTimestampFile(TimestampFile);
   }
 
   bool ShouldComputeSize =
       (Policy.MaxSizePercentageOfAvailableSpace > 0 || Policy.MaxSizeBytes > 0);
 
-  // Keep track of space
+  // Keep track of space. Needs to be kept ordered by size for determinism.
   std::set<std::pair<uint64_t, std::string>> FileSizes;
   uint64_t TotalSize = 0;
-  // Helper to add a path to the set of files to consider for size-based
-  // pruning, sorted by size.
-  auto AddToFileListForSizePruning =
-      [&](StringRef Path) {
-        if (!ShouldComputeSize)
-          return;
-        TotalSize += FileStatus.getSize();
-        FileSizes.insert(
-            std::make_pair(FileStatus.getSize(), std::string(Path)));
-      };
 
   // Walk the entire directory cache, looking for unused files.
   std::error_code EC;
   SmallString<128> CachePathNative;
   sys::path::native(Path, CachePathNative);
   // Walk all of the files within this directory.
   for (sys::fs::directory_iterator File(CachePathNative, EC), FileEnd;
        File != FileEnd && !EC; File.increment(EC)) {
     // Ignore any files not beginning with the string "llvmcache-". This
     // includes the timestamp file as well as any files created by the user.
     // This acts as a safeguard against data loss if the user specifies the
     // wrong directory as their cache directory.
     if (!sys::path::filename(File->path()).startswith("llvmcache-"))
       continue;
 
     // Look at this file. If we can't stat it, there's nothing interesting
     // there.
-    if (sys::fs::status(File->path(), FileStatus)) {
+    ErrorOr<sys::fs::basic_file_status> StatusOrErr = File->status();
+    if (!StatusOrErr) {
       DEBUG(dbgs() << "Ignore " << File->path() << " (can't stat)\n");
       continue;
     }
 
     // If the file hasn't been used recently enough, delete it
-    const auto FileAccessTime = FileStatus.getLastAccessedTime();
+    const auto FileAccessTime = StatusOrErr->getLastAccessedTime();
     auto FileAge = CurrentTime - FileAccessTime;
     if (FileAge > Policy.Expiration) {
       DEBUG(dbgs() << "Remove " << File->path() << " ("
                    << duration_cast<seconds>(FileAge).count() << "s old)\n");
       sys::fs::remove(File->path());
       continue;
     }
 
     // Leave it here for now, but add it to the list of size-based pruning.
-    AddToFileListForSizePruning(File->path());
+    if (!ShouldComputeSize)
+      continue;
+    TotalSize += StatusOrErr->getSize();
+    FileSizes.insert({StatusOrErr->getSize(), std::string(File->path())});
   }
 
   // Prune for size now if needed
   if (ShouldComputeSize) {
     auto ErrOrSpaceInfo = sys::fs::disk_space(Path);
     if (!ErrOrSpaceInfo) {
       report_fatal_error("Can't get available size");
     }

llvm/trunk/lib/Support/Path.cpp

ErrorOr<MD5::MD5Result> md5_contents(const Twine &Path) {
   if (auto EC = openFileForRead(Path, FD))
     return EC;
 
   auto Result = md5_contents(FD);
   close(FD);
   return Result;
 }
 
-bool exists(file_status status) {
+bool exists(const basic_file_status &status) {
   return status_known(status) && status.type() != file_type::file_not_found;
 }
 
-bool status_known(file_status s) {
+bool status_known(const basic_file_status &s) {
   return s.type() != file_type::status_error;
 }
 
 file_type get_file_type(const Twine &Path, bool Follow) {
   file_status st;
   if (status(Path, st, Follow))
     return file_type::status_error;
   return st.type();
 }
 
-bool is_directory(file_status status) {
+bool is_directory(const basic_file_status &status) {
   return status.type() == file_type::directory_file;
 }
 
 std::error_code is_directory(const Twine &path, bool &result) {
   file_status st;
   if (std::error_code ec = status(path, st))
     return ec;
   result = is_directory(st);
   return std::error_code();
 }
 
-bool is_regular_file(file_status status) {
+bool is_regular_file(const basic_file_status &status) {
   return status.type() == file_type::regular_file;
 }
 
 std::error_code is_regular_file(const Twine &path, bool &result) {
   file_status st;
   if (std::error_code ec = status(path, st))
     return ec;
   result = is_regular_file(st);
   return std::error_code();
 }
 
-bool is_symlink_file(file_status status) {
+bool is_symlink_file(const basic_file_status &status) {
   return status.type() == file_type::symlink_file;
 }
 
 std::error_code is_symlink_file(const Twine &path, bool &result) {
   file_status st;
   if (std::error_code ec = status(path, st, false))
     return ec;
   result = is_symlink_file(st);
   return std::error_code();
 }
 
-bool is_other(file_status status) {
+bool is_other(const basic_file_status &status) {
   return exists(status) &&
          !is_regular_file(status) &&
          !is_directory(status);
 }
 
 std::error_code is_other(const Twine &Path, bool &Result) {
   file_status FileStatus;
   if (std::error_code EC = status(Path, FileStatus))
     return EC;
   Result = is_other(FileStatus);
   return std::error_code();
 }
 
-void directory_entry::replace_filename(const Twine &filename, file_status st) {
+void directory_entry::replace_filename(const Twine &filename,
+                                       basic_file_status st) {
   SmallString<128> path = path::parent_path(Path);
   path::append(path, filename);
   Path = path.str();
   Status = st;
 }
 
-std::error_code directory_entry::status(file_status &result) const {
-  return fs::status(Path, result, FollowSymlinks);
-}
-
 ErrorOr<perms> getPermissions(const Twine &Path) {
   file_status Status;
   if (std::error_code EC = status(Path, Status))
     return EC;
 
   return Status.permissions();
 }
 

llvm/trunk/lib/Support/Unix/Path.inc

#elif defined(HAVE_DLFCN_H) && defined(HAVE_DLADDR)
   if (realpath(DLInfo.dli_fname, link_path))
     return link_path;
 #else
 #error GetMainExecutable is not implemented on this host yet.
 #endif
   return "";
 }
 
-TimePoint<> file_status::getLastAccessedTime() const {
+TimePoint<> basic_file_status::getLastAccessedTime() const {
   return toTimePoint(fs_st_atime);
 }
 
-TimePoint<> file_status::getLastModificationTime() const {
+TimePoint<> basic_file_status::getLastModificationTime() const {
   return toTimePoint(fs_st_mtime);
 }
 
 UniqueID file_status::getUniqueID() const {
   return UniqueID(fs_st_dev, fs_st_ino);
 }
 
 uint32_t file_status::getLinkCount() const {
if ((name.size() == 1 && name[0] == '.') ||
       return directory_iterator_increment(it);
     it.CurrentEntry.replace_filename(name);
   } else
     return directory_iterator_destruct(it);
 
   return std::error_code();
 }
 
+ErrorOr<basic_file_status> directory_entry::status() const {
+  file_status s;
+  if (auto EC = fs::status(Path, s, FollowSymlinks))
+    return EC;
+  return s;
+}
+
 #if !defined(F_GETPATH)
 static bool hasProcSelfFD() {
   // If we have a /proc filesystem mounted, we can quickly establish the
   // real name of the file with readlink
   static const bool Result = (::access("/proc/self/fd", R_OK) == 0);
   return Result;
 }
 #endif

 template <typename T>
 static std::error_code remove_directories_impl(const T &Entry,
                                                bool IgnoreErrors) {
   std::error_code EC;
   directory_iterator Begin(Entry, EC, false);
   directory_iterator End;
   while (Begin != End) {
     auto &Item = *Begin;
-    file_status st;
-    EC = Item.status(st);
-    if (EC && !IgnoreErrors)
-      return EC;
+    ErrorOr<basic_file_status> st = Item.status();
+    if (!st && !IgnoreErrors)
+      return st.getError();
 
-    if (is_directory(st)) {
+    if (is_directory(*st)) {
       EC = remove_directories_impl(Item, IgnoreErrors);
       if (EC && !IgnoreErrors)
         return EC;
     }
 
     EC = fs::remove(Item.path(), true);
     if (EC && !IgnoreErrors)
       return EC;

llvm/trunk/lib/Support/Windows/Path.inc

ErrorOr<space_info> disk_space(const Twine &Path) {
   SpaceInfo.capacity =
       (static_cast<uint64_t>(Total.HighPart) << 32) + Total.LowPart;
   SpaceInfo.free = (static_cast<uint64_t>(Free.HighPart) << 32) + Free.LowPart;
   SpaceInfo.available =
       (static_cast<uint64_t>(Avail.HighPart) << 32) + Avail.LowPart;
   return SpaceInfo;
 }
 
-TimePoint<> file_status::getLastAccessedTime() const {
+TimePoint<> basic_file_status::getLastAccessedTime() const {
   FILETIME Time;
   Time.dwLowDateTime = LastAccessedTimeLow;
   Time.dwHighDateTime = LastAccessedTimeHigh;
   return toTimePoint(Time);
 }
 
-TimePoint<> file_status::getLastModificationTime() const {
+TimePoint<> basic_file_status::getLastModificationTime() const {
   FILETIME Time;
   Time.dwLowDateTime = LastWriteTimeLow;
   Time.dwHighDateTime = LastWriteTimeHigh;
   return toTimePoint(Time);
 }
 
 uint32_t file_status::getLinkCount() const {
   return NumLinks;
for (size_t i = 0; i < array_lengthof(sReservedNames); ++i) {
     if (path.equals_lower(sReservedNames[i]))
       return true;
   }
 
   // The path isn't what we consider reserved.
   return false;
 }
 
+static file_type file_type_from_attrs(DWORD Attrs) {
+  return (Attrs & FILE_ATTRIBUTE_DIRECTORY) ? file_type::directory_file
+                                            : file_type::regular_file;
+}
+
+static perms perms_from_attrs(DWORD Attrs) {
+  return (Attrs & FILE_ATTRIBUTE_READONLY) ? (all_read | all_exe) : all_all;
+}
+
 static std::error_code getStatus(HANDLE FileHandle, file_status &Result) {
   if (FileHandle == INVALID_HANDLE_VALUE)
     goto handle_status_error;
 
   switch (::GetFileType(FileHandle)) {
   default:
     llvm_unreachable("Don't know anything about this file type");
   case FILE_TYPE_UNKNOWN: {
case FILE_TYPE_PIPE:
     Result = file_status(file_type::fifo_file);
     return std::error_code();
   }
 
   BY_HANDLE_FILE_INFORMATION Info;
   if (!::GetFileInformationByHandle(FileHandle, &Info))
     goto handle_status_error;
 
-  {
-    file_type Type = (Info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
-                         ? file_type::directory_file
-                         : file_type::regular_file;
-    perms Permissions = (Info.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
-                            ? (all_read | all_exe)
-                            : all_all;
-    Result = file_status(
-        Type, Permissions, Info.nNumberOfLinks,
-        Info.ftLastAccessTime.dwHighDateTime,
-        Info.ftLastAccessTime.dwLowDateTime,
-        Info.ftLastWriteTime.dwHighDateTime, Info.ftLastWriteTime.dwLowDateTime,
-        Info.dwVolumeSerialNumber, Info.nFileSizeHigh, Info.nFileSizeLow,
-        Info.nFileIndexHigh, Info.nFileIndexLow);
-    return std::error_code();
-  }
+      file_type_from_attrs(Info.dwFileAttributes),
+      perms_from_attrs(Info.dwFileAttributes), Info.nNumberOfLinks,
+      Info.ftLastAccessTime.dwHighDateTime, Info.ftLastAccessTime.dwLowDateTime,
 
 handle_status_error:
   DWORD LastError = ::GetLastError();
   if (LastError == ERROR_FILE_NOT_FOUND ||
       LastError == ERROR_PATH_NOT_FOUND)
     Result = file_status(file_type::file_not_found);
   else if (LastError == ERROR_SHARING_VIOLATION)
     Result = file_status(file_type::type_unknown);

 }
 
 int mapped_file_region::alignment() {
   SYSTEM_INFO SysInfo;
   ::GetSystemInfo(&SysInfo);
   return SysInfo.dwAllocationGranularity;
 }
 
+static basic_file_status status_from_find_data(WIN32_FIND_DATA *FindData) {
+  return basic_file_status(file_type_from_attrs(FindData->dwFileAttributes),
+                           perms_from_attrs(FindData->dwFileAttributes),
+                           FindData->ftLastAccessTime.dwHighDateTime,
+                           FindData->ftLastAccessTime.dwLowDateTime,
+                           FindData->ftLastWriteTime.dwHighDateTime,
+                           FindData->ftLastWriteTime.dwLowDateTime,
+                           FindData->nFileSizeHigh, FindData->nFileSizeLow);
+}
+
 std::error_code detail::directory_iterator_construct(detail::DirIterState &it,
                                                      StringRef path,
                                                      bool follow_symlinks) {
   SmallVector<wchar_t, 128> path_utf16;
 
   if (std::error_code ec = widenPath(path, path_utf16))
     return ec;
 
   // Convert path to the format that Windows is happy with.
   if (path_utf16.size() > 0 &&
       !is_separator(path_utf16[path.size() - 1]) &&
       path_utf16[path.size() - 1] != L':') {
     path_utf16.push_back(L'\\');
     path_utf16.push_back(L'*');
   } else {
     path_utf16.push_back(L'*');
   }
 
   //  Get the first directory entry.
   WIN32_FIND_DATAW FirstFind;
-  ScopedFindHandle FindHandle(::FindFirstFileW(c_str(path_utf16), &FirstFind));
+  ScopedFindHandle FindHandle(::FindFirstFileExW(
+      c_str(path_utf16), FindExInfoBasic, &FirstFind, FindExSearchNameMatch,
+      NULL, FIND_FIRST_EX_LARGE_FETCH));
   if (!FindHandle)
     return mapWindowsError(::GetLastError());
 
   size_t FilenameLen = ::wcslen(FirstFind.cFileName);
   while ((FilenameLen == 1 && FirstFind.cFileName[0] == L'.') ||
          (FilenameLen == 2 && FirstFind.cFileName[0] == L'.' &&
                               FirstFind.cFileName[1] == L'.'))
     if (!::FindNextFileW(FindHandle, &FirstFind)) {
std::error_code detail::directory_iterator_construct(detail::DirIterState &it,
   if (std::error_code ec =
           UTF16ToUTF8(FirstFind.cFileName, ::wcslen(FirstFind.cFileName),
                       directory_entry_name_utf8))
     return ec;
 
   it.IterationHandle = intptr_t(FindHandle.take());
   SmallString<128> directory_entry_path(path);
   path::append(directory_entry_path, directory_entry_name_utf8);
-  it.CurrentEntry = directory_entry(directory_entry_path, follow_symlinks);
+  it.CurrentEntry = directory_entry(directory_entry_path, follow_symlinks,
+                                    status_from_find_data(&FirstFind));
 
   return std::error_code();
 }
 
 std::error_code detail::directory_iterator_destruct(detail::DirIterState &it) {
   if (it.IterationHandle != 0)
     // Closes the handle if it's valid.
     ScopedFindHandle close(HANDLE(it.IterationHandle));
if ((FilenameLen == 1 && FindData.cFileName[0] == L'.') ||
     return directory_iterator_increment(it);
 
   SmallString<128> directory_entry_path_utf8;
   if (std::error_code ec =
           UTF16ToUTF8(FindData.cFileName, ::wcslen(FindData.cFileName),
                       directory_entry_path_utf8))
     return ec;
 
-  it.CurrentEntry.replace_filename(Twine(directory_entry_path_utf8));
+  it.CurrentEntry.replace_filename(Twine(directory_entry_path_utf8),
+                                   status_from_find_data(&FindData));
   return std::error_code();
 }
 
+ErrorOr<basic_file_status> directory_entry::status() const {
+  return Status;
+}
+
 static std::error_code realPathFromHandle(HANDLE H,
                                           SmallVectorImpl<char> &RealPath) {
   RealPath.clear();
   llvm::SmallVector<wchar_t, MAX_PATH> Buffer;
   DWORD CountChars = ::GetFinalPathNameByHandleW(
       H, Buffer.begin(), Buffer.capacity() - 1, FILE_NAME_NORMALIZED);
   if (CountChars > Buffer.capacity()) {
     // The buffer wasn't big enough, try again.  In this case the return value

llvm/trunk/unittests/Support/Path.cpp

TEST_F(FileSystemTest, BrokenSymlinkDirectoryIteration) {
   // 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);
 
-    fs::file_status status;
-    if (i->status(status) ==
+    ErrorOr<fs::basic_file_status> status = i->status();
+    if (status.getError() ==
         std::make_error_code(std::errc::no_such_file_or_directory)) {
       i.no_push();
       continue;
     }
 
     visited.push_back(path::filename(i->path()));
   }
   std::sort(visited.begin(), visited.end());