[VFS] Cleanups to VFS interfaces.

Summary:
- add comments clarifying semantics
- Status::copyWithNewName(Status, Name) --> instance method
- Status::copyWithNewName(fs::file_status, Name) --> constructor (it's not a copy)
- File::getName() -> getRealPath(), reflecting its actual behavior/function
  and stop returning status().getName() in the base class (callers can do this
  fallback if they want to, it complicates the contracts).

This is mostly NFC, but the behavior of File::getName() affects FileManager's
FileEntry::tryGetRealPathName(), which now fails in more cases:
 - non-real file cases
 - real-file cases where the underlying vfs::File was opened in a way that
   doesn't call realpath().
(In these cases we don't know a distinct real name, so in principle it seems OK)

Reviewers: klimek

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D49724

llvm-svn: 337834

3 files changed, 35 insertions, 31 deletions

diff --git a/clang/include/clang/Basic/VirtualFileSystem.h b/clang/include/clang/Basic/VirtualFileSystem.h
index 2480b91123c..bd752c8dadd 100644
--- a/clang/include/clang/Basic/VirtualFileSystem.h
+++ b/clang/include/clang/Basic/VirtualFileSystem.h
@@ -45,7 +45,8 @@ class MemoryBuffer;
 namespace clang {
 namespace vfs {
 
-/// The result of a \p status operation.
+/// File information from a \p File::status() operation.
+/// This is roughly equivalent to a `struct stat` plus a file path.
 class Status {
   std::string Name;
   llvm::sys::fs::UniqueID UID;
@@ -66,13 +67,14 @@ public:
          llvm::sys::TimePoint<> MTime, uint32_t User, uint32_t Group,
          uint64_t Size, llvm::sys::fs::file_type Type,
          llvm::sys::fs::perms Perms);
+  Status(const llvm::sys::fs::file_status &FSStatus, StringRef Name);
 
-  /// Get a copy of a Status with a different name.
-  static Status copyWithNewName(const Status &In, StringRef NewName);
-  static Status copyWithNewName(const llvm::sys::fs::file_status &In,
-                                StringRef NewName);
+  /// Get a copy of a this Status with a different name.
+  Status copyWithNewName(StringRef NewName);
 
   /// Returns the name that should be used for this file or directory.
+  /// This is usually the path that the file was opened as, without resolving
+  /// relative paths or symlinks.
   StringRef getName() const { return Name; }
 
   /// @name Status interface from llvm::sys::fs
@@ -107,15 +109,16 @@ public:
   virtual ~File();
 
   /// Get the status of the file.
+  /// This may access the filesystem (e.g. `stat()`), or return a cached value.
   virtual llvm::ErrorOr<Status> status() = 0;
 
-  /// Get the name of the file
-  virtual llvm::ErrorOr<std::string> getName() {
-    if (auto Status = status())
-      return Status->getName().str();
-    else
-      return Status.getError();
-  }
+  /// Get the "real name" of the file, if available.
+  /// This should be absolute, and typically has symlinks resolved.
+  ///
+  /// Only some VFS implementations provide this, and only sometimes.
+  /// FIXME: these maybe-available semantics are not very useful. It would be
+  /// nice if this was more consistent with FileSystem::getRealPath().
+  virtual llvm::Optional<std::string> getRealPath() { return llvm::None; }
 
   /// Get the contents of the file as a \p MemoryBuffer.
   virtual llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 7e2d01c4981..13a3d8f95ce 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -316,7 +316,7 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
   UFE.File = std::move(F);
   UFE.IsValid = true;
   if (UFE.File)
-    if (auto RealPathName = UFE.File->getName())
+    if (auto RealPathName = UFE.File->getRealPath())
       UFE.RealPathName = *RealPathName;
   return &UFE;
 }
diff --git a/clang/lib/Basic/VirtualFileSystem.cpp b/clang/lib/Basic/VirtualFileSystem.cpp
index bcfcbdbb901..b269a9fcc00 100644
--- a/clang/lib/Basic/VirtualFileSystem.cpp
+++ b/clang/lib/Basic/VirtualFileSystem.cpp
@@ -73,16 +73,14 @@ Status::Status(StringRef Name, UniqueID UID, sys::TimePoint<> MTime,
     : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group), Size(Size),
       Type(Type), Perms(Perms) {}
 
-Status Status::copyWithNewName(const Status &In, StringRef NewName) {
-  return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
-                In.getUser(), In.getGroup(), In.getSize(), In.getType(),
-                In.getPermissions());
-}
+Status::Status(const file_status &In, StringRef NewName)
+    : Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
+             In.getUser(), In.getGroup(), In.getSize(), In.type(),
+             In.permissions()) {}
 
-Status Status::copyWithNewName(const file_status &In, StringRef NewName) {
-  return Status(NewName, In.getUniqueID(), In.getLastModificationTime(),
-                In.getUser(), In.getGroup(), In.getSize(), In.type(),
-                In.permissions());
+Status Status::copyWithNewName(StringRef NewName) {
+  return Status(NewName, getUniqueID(), getLastModificationTime(), getUser(),
+                getGroup(), getSize(), getType(), getPermissions());
 }
 
 bool Status::equivalent(const Status &Other) const {
@@ -178,6 +176,7 @@ class RealFile : public File {
   Status S;
   std::string RealName;
 
+  // NewRealPathName is used only if non-empty.
   RealFile(int FD, StringRef NewName, StringRef NewRealPathName)
       : FD(FD), S(NewName, {}, {}, {}, {}, {},
                   llvm::sys::fs::file_type::status_error, {}),
@@ -189,7 +188,7 @@ public:
   ~RealFile() override;
 
   ErrorOr<Status> status() override;
-  ErrorOr<std::string> getName() override;
+  Optional<std::string> getRealPath() override;
   ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,
                                                    int64_t FileSize,
                                                    bool RequiresNullTerminator,
@@ -207,13 +206,15 @@ ErrorOr<Status> RealFile::status() {
     file_status RealStatus;
     if (std::error_code EC = sys::fs::status(FD, RealStatus))
       return EC;
-    S = Status::copyWithNewName(RealStatus, S.getName());
+    S = Status(RealStatus, S.getName());
   }
   return S;
 }
 
-ErrorOr<std::string> RealFile::getName() {
-  return RealName.empty() ? S.getName().str() : RealName;
+Optional<std::string> RealFile::getRealPath() {
+  if (RealName.empty())
+    return llvm::None;
+  return RealName;
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
@@ -251,7 +252,7 @@ ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
   sys::fs::file_status RealStatus;
   if (std::error_code EC = sys::fs::status(Path, RealStatus))
     return EC;
-  return Status::copyWithNewName(RealStatus, Path.str());
+  return Status(RealStatus, Path.str());
 }
 
 ErrorOr<std::unique_ptr<File>>
@@ -303,7 +304,7 @@ public:
     if (Iter != llvm::sys::fs::directory_iterator()) {
       llvm::sys::fs::file_status S;
       std::error_code ErrorCode = llvm::sys::fs::status(Iter->path(), S, true);
-      CurrentEntry = Status::copyWithNewName(S, Iter->path());
+      CurrentEntry = Status(S, Iter->path());
       if (!EC)
         EC = ErrorCode;
     }
@@ -317,7 +318,7 @@ public:
     } 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());
+      CurrentEntry = Status(S, Iter->path());
       if (!EC)
         EC = ErrorCode;
     }
@@ -1641,7 +1642,7 @@ static Status getRedirectedFileStatus(const Twine &Path, bool UseExternalNames,
                                       Status ExternalStatus) {
   Status S = ExternalStatus;
   if (!UseExternalNames)
-    S = Status::copyWithNewName(S, Path.str());
+    S = S.copyWithNewName(Path.str());
   S.IsVFSMapped = true;
   return S;
 }
@@ -1657,7 +1658,7 @@ ErrorOr<Status> RedirectingFileSystem::status(const Twine &Path, Entry *E) {
     return S;
   } else { // directory
     auto *DE = cast<RedirectingDirectoryEntry>(E);
-    return Status::copyWithNewName(DE->getStatus(), Path.str());
+    return DE->getStatus().copyWithNewName(Path.str());
   }
 }