[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: include/clang/Basic/VirtualFileSystem.h:135
+  // For compatibility with old Status-based API. Prefer using Path directly.
+  StringRef getName() const { return Path; }
+};
----------------
sammccall wrote:
> bkramer wrote:
> > sammccall wrote:
> > > Backwards-compatibility notes:
> > > 
> > >  - Almost all users of `directory_iterator` require no source changes (with this change)
> > >  - Implementations of VFS require changes if they support directory iteration and do not merely wrap other VFSes. Anecdotally, most do not require changes. 
> > > 
> > > So this weird API seems worth having to make out-of-tree breakages less painful.
> > > Happy to update the internal uses though if that seems worthwhile.
> > Can we mirror llvm::sys::fs::directory_entry's interface? I want the APIs to be as close as possible. Upgrading users is not a big deal.
> How much of the interface are you talking about? :-)
> 
> Making these private and calling the accessors `file()` and `type()` is easy of course, and consistency is nice.
> 
> Supporting status() with similar semantics+performance is both complicated and... not a good idea, I think. See the other patch where I added a comment like "this interface is wrong" and didn't fix it :-)
> 
> The other random functions on fs::directory_entry seem like random implementation cruft to me.
I've done the `path()` and `type()` rename, so now we're consistent with fs::directory_entry.
And inconsistent with clang::vfs::Status, and with clang::DirectoryEntry (from FileManager).
Can't win em all!


Repository:
  rC Clang

https://reviews.llvm.org/D51921