bkramer 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:
> 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!
Not exposing random fields and having the same names as the other
directory_entry is what I wanted :)
status() is deep in YAGNI territory.
Repository:
rC Clang
https://reviews.llvm.org/D51921
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
