Split out llvm/Support/FileSystem/UniqueID.h and clang/Basic/FileEntry.h, NFC

[dexonsmith]

Summary

Split out llvm/Support/FileSystem/UniqueID.h and clang/Basic/FileEntry.h, NFC

Split FileEntry and FileEntryRef out into a new file
clang/Basic/FileEntry.h. This allows current users of a
forward-declared FileEntry to transition to FileEntryRef without
adding more includers of FileManager.h.

Also split UniqueID out to llvm/Support/FileSystem/UniqueID.h, so
FileEntry.h doesn't need to include all of FileSystem.h for just
that type.

[JDevlieghere]

clang/include/clang/Basic/FileEntry.h

34

Won't this now make llvm::Optional visible as clang::Optional everywhere this header is included? Isn't this considered bad practice in a header?

[dexonsmith]

clang/include/clang/Basic/FileEntry.h

34

I thought I found both of these usable in clang headers elsewhere. I’ll double check.

[dexonsmith]

clang/include/clang/Basic/FileEntry.h

34

Ah, I was sort of right. The correct way to do this is:

#include <clang/Basic/LLVM.h>

which forward-declares a bunch of stuff and pulls it into clang::.

I'll update.

[dexonsmith]

Use #include <clang/Basic/LLVM.h> to pull StringRef and Optional into clang::.

[dexonsmith]

Thanks @JDevlieghere for the review! This landed in 23ed570af1cc165afea1b70a533a4a39d6656501.

[rnk]

I just saw this in passing. Thanks a lot, I had noticed that Filesystem.h is super expensive.

llvm/include/llvm/Support/FileSystem/UniqueID.h

36

FYI std::tie is pretty expensive to compile, unfortunately. You also might technically need to include some STL header for it here.

[dexonsmith]

llvm/include/llvm/Support/FileSystem/UniqueID.h

36

I just sent you https://reviews.llvm.org/D90471 to remove std::tie. (Already fixed the includes in 44d65efd95b353eeb26440ad2ef9f3bd05f5a927, not sure how/why it worked without them locally.)

clang/include/clang/Basic/FileEntry.h

+//===- clang/Basic/FileEntry.h - File references ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Defines interfaces for clang::FileEntry and clang::FileEntryRef.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_FILEENTRY_H
+#define LLVM_CLANG_BASIC_FILEENTRY_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
+
+namespace llvm {
+namespace vfs {
+
+class File;
+
+} // namespace vfs
+} // namespace llvm
+
+namespace clang {
+
+class DirectoryEntry;
+class FileEntry;
+
+/// A reference to a \c FileEntry that includes the name of the file as it was
+/// accessed by the FileManager's client.
+class FileEntryRef {
+public:
+  const StringRef getName() const { return Entry->first(); }
+  const FileEntry &getFileEntry() const {
+    return *Entry->second->V.get<FileEntry *>();
+  }
+
+  inline bool isValid() const;
+  inline off_t getSize() const;
+  inline unsigned getUID() const;
+  inline const llvm::sys::fs::UniqueID &getUniqueID() const;
+  inline time_t getModificationTime() const;
+
+  friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
+    return LHS.Entry == RHS.Entry;
+  }
+  friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
+    return !(LHS == RHS);
+  }
+
+  struct MapValue;
+
+  /// Type used in the StringMap.
+  using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<MapValue>>;
+
+  /// Type stored in the StringMap.
+  struct MapValue {
+    /// The pointer at another MapEntry is used when the FileManager should
+    /// silently forward from one name to another, which occurs in Redirecting
+    /// VFSs that use external names. In that case, the \c FileEntryRef
+    /// returned by the \c FileManager will have the external name, and not the
+    /// name that was used to lookup the file.
+    ///
+    /// The second type is really a `const MapEntry *`, but that confuses
+    /// gcc5.3.  Once that's no longer supported, change this back.
+    llvm::PointerUnion<FileEntry *, const void *> V;
+
+    MapValue() = delete;
+    MapValue(FileEntry &FE) : V(&FE) {}
+    MapValue(MapEntry &ME) : V(&ME) {}
+  };
+
+private:
+  friend class FileManager;
+
+  FileEntryRef() = delete;
+  explicit FileEntryRef(const MapEntry &Entry)
+      : Entry(&Entry) {
+    assert(Entry.second && "Expected payload");
+    assert(Entry.second->V && "Expected non-null");
+    assert(Entry.second->V.is<FileEntry *>() && "Expected FileEntry");
+  }
+
+  const MapEntry *Entry;
+};
+
+/// Cached information about one file (either on disk
+/// or in the virtual file system).
+///
+/// If the 'File' member is valid, then this FileEntry has an open file
+/// descriptor for the file.
+class FileEntry {
+  friend class FileManager;
+
+  std::string RealPathName;   // Real path to the file; could be empty.
+  off_t Size;                 // File size in bytes.
+  time_t ModTime;             // Modification time of file.
+  const DirectoryEntry *Dir;  // Directory file lives in.
+  llvm::sys::fs::UniqueID UniqueID;
+  unsigned UID;               // A unique (small) ID for the file.
+  bool IsNamedPipe;
+  bool IsValid;               // Is this \c FileEntry initialized and valid?
+
+  /// The open file, if it is owned by the \p FileEntry.
+  mutable std::unique_ptr<llvm::vfs::File> File;
+
+  // First access name for this FileEntry.
+  //
+  // This is Optional only to allow delayed construction (FileEntryRef has no
+  // default constructor). It should always have a value in practice.
+  //
+  // TODO: remove this once everyone that needs a name uses FileEntryRef.
+  Optional<FileEntryRef> LastRef;
+
+public:
+  FileEntry();
+  ~FileEntry();
+
+  FileEntry(const FileEntry &) = delete;
+  FileEntry &operator=(const FileEntry &) = delete;
+
+  StringRef getName() const { return LastRef->getName(); }
+  FileEntryRef getLastRef() const { return *LastRef; }
+
+  StringRef tryGetRealPathName() const { return RealPathName; }
+  bool isValid() const { return IsValid; }
+  off_t getSize() const { return Size; }
+  unsigned getUID() const { return UID; }
+  const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
+  time_t getModificationTime() const { return ModTime; }
+
+  /// Return the directory the file lives in.
+  const DirectoryEntry *getDir() const { return Dir; }
+
+  bool operator<(const FileEntry &RHS) const { return UniqueID < RHS.UniqueID; }
+
+  /// Check whether the file is a named pipe (and thus can't be opened by
+  /// the native FileManager methods).
+  bool isNamedPipe() const { return IsNamedPipe; }
+
+  void closeFile() const;
+
+  // Only for use in tests to see if deferred opens are happening, rather than
+  // relying on RealPathName being empty.
+  bool isOpenForTests() const { return File != nullptr; }
+};
+
+bool FileEntryRef::isValid() const { return getFileEntry().isValid(); }
+
+off_t FileEntryRef::getSize() const { return getFileEntry().getSize(); }
+
+unsigned FileEntryRef::getUID() const { return getFileEntry().getUID(); }
+
+const llvm::sys::fs::UniqueID &FileEntryRef::getUniqueID() const {
+  return getFileEntry().getUniqueID();
+}
+
+time_t FileEntryRef::getModificationTime() const {
+  return getFileEntry().getModificationTime();
+}
+
+} // end namespace clang
+
+#endif // LLVM_CLANG_BASIC_FILEENTRY_H

clang/include/clang/Basic/FileManager.h

 //===--- FileManager.h - File System Probing and Caching --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// Defines the clang::FileManager interface and associated types.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_FILEMANAGER_H
 #define LLVM_CLANG_BASIC_FILEMANAGER_H
 
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
private:
 
   DirectoryEntryRef(
       llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry)
       : Entry(Entry) {}
 
   const llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>> *Entry;
 };
 
-class FileEntry;
-
-/// A reference to a \c FileEntry that includes the name of the file as it was
-/// accessed by the FileManager's client.
-class FileEntryRef {
-public:
-  const StringRef getName() const { return Entry->first(); }
-  const FileEntry &getFileEntry() const {
-    return *Entry->second->V.get<FileEntry *>();
-  }
-
-  inline bool isValid() const;
-  inline off_t getSize() const;
-  inline unsigned getUID() const;
-  inline const llvm::sys::fs::UniqueID &getUniqueID() const;
-  inline time_t getModificationTime() const;
-
-  friend bool operator==(const FileEntryRef &LHS, const FileEntryRef &RHS) {
-    return LHS.Entry == RHS.Entry;
-  }
-  friend bool operator!=(const FileEntryRef &LHS, const FileEntryRef &RHS) {
-    return !(LHS == RHS);
-  }
-
-  struct MapValue;
-
-  /// Type used in the StringMap.
-  using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<MapValue>>;
-
-  /// Type stored in the StringMap.
-  struct MapValue {
-    /// The pointer at another MapEntry is used when the FileManager should
-    /// silently forward from one name to another, which occurs in Redirecting
-    /// VFSs that use external names. In that case, the \c FileEntryRef
-    /// returned by the \c FileManager will have the external name, and not the
-    /// name that was used to lookup the file.
-    // The second type is really a `const MapEntry *`, but that confuses gcc5.3.
-    // Once that's no longer supported, change this back.
-    llvm::PointerUnion<FileEntry *, const void *> V;
-
-    MapValue() = delete;
-    MapValue(FileEntry &FE) : V(&FE) {}
-    MapValue(MapEntry &ME) : V(&ME) {}
-  };
-
-private:
-  friend class FileManager;
-
-  FileEntryRef() = delete;
-  explicit FileEntryRef(const MapEntry &Entry)
-      : Entry(&Entry) {
-    assert(Entry.second && "Expected payload");
-    assert(Entry.second->V && "Expected non-null");
-    assert(Entry.second->V.is<FileEntry *>() && "Expected FileEntry");
-  }
-
-  const MapEntry *Entry;
-};
-
-/// Cached information about one file (either on disk
-/// or in the virtual file system).
-///
-/// If the 'File' member is valid, then this FileEntry has an open file
-/// descriptor for the file.
-class FileEntry {
-  friend class FileManager;
-
-  std::string RealPathName;   // Real path to the file; could be empty.
-  off_t Size;                 // File size in bytes.
-  time_t ModTime;             // Modification time of file.
-  const DirectoryEntry *Dir;  // Directory file lives in.
-  llvm::sys::fs::UniqueID UniqueID;
-  unsigned UID;               // A unique (small) ID for the file.
-  bool IsNamedPipe;
-  bool IsValid;               // Is this \c FileEntry initialized and valid?
-
-  /// The open file, if it is owned by the \p FileEntry.
-  mutable std::unique_ptr<llvm::vfs::File> File;
-
-  // First access name for this FileEntry.
-  //
-  // This is Optional only to allow delayed construction (FileEntryRef has no
-  // default constructor). It should always have a value in practice.
-  //
-  // TODO: remote this once everyone that needs a name uses FileEntryRef.
-  Optional<FileEntryRef> LastRef;
-
-public:
-  FileEntry()
-      : UniqueID(0, 0), IsNamedPipe(false), IsValid(false)
-  {}
-
-  FileEntry(const FileEntry &) = delete;
-  FileEntry &operator=(const FileEntry &) = delete;
-
-  StringRef getName() const { return LastRef->getName(); }
-  FileEntryRef getLastRef() const { return *LastRef; }
-
-  StringRef tryGetRealPathName() const { return RealPathName; }
-  bool isValid() const { return IsValid; }
-  off_t getSize() const { return Size; }
-  unsigned getUID() const { return UID; }
-  const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
-  time_t getModificationTime() const { return ModTime; }
-
-  /// Return the directory the file lives in.
-  const DirectoryEntry *getDir() const { return Dir; }
-
-  bool operator<(const FileEntry &RHS) const { return UniqueID < RHS.UniqueID; }
-
-  /// Check whether the file is a named pipe (and thus can't be opened by
-  /// the native FileManager methods).
-  bool isNamedPipe() const { return IsNamedPipe; }
-
-  void closeFile() const {
-    File.reset(); // rely on destructor to close File
-  }
-
-  // Only for use in tests to see if deferred opens are happening, rather than
-  // relying on RealPathName being empty.
-  bool isOpenForTests() const { return File != nullptr; }
-};
-
-bool FileEntryRef::isValid() const { return getFileEntry().isValid(); }
-
-off_t FileEntryRef::getSize() const { return getFileEntry().getSize(); }
-
-unsigned FileEntryRef::getUID() const { return getFileEntry().getUID(); }
-
-const llvm::sys::fs::UniqueID &FileEntryRef::getUniqueID() const {
-  return getFileEntry().getUniqueID();
-}
-
-time_t FileEntryRef::getModificationTime() const {
-  return getFileEntry().getModificationTime();
-}
-
 /// Implements support for file system lookup, file system caching,
 /// and directory search management.
 ///
 /// This also handles more advanced properties, such as uniquing files based
 /// on "inode", so that a file with two names (e.g. symlinked) will be treated
 /// as a single file.
 ///
 class FileManager : public RefCountedBase<FileManager> {

clang/lib/Basic/CMakeLists.txt

add_clang_library(clangBasic
   Builtins.cpp
   CharInfo.cpp
   CodeGenOptions.cpp
   Cuda.cpp
   Diagnostic.cpp
   DiagnosticIDs.cpp
   DiagnosticOptions.cpp
   ExpressionTraits.cpp
+  FileEntry.cpp
   FileManager.cpp
   FileSystemStatCache.cpp
   IdentifierTable.cpp
   LangOptions.cpp
   LangStandards.cpp
   Module.cpp
   ObjCRuntime.cpp
   OpenMPKinds.cpp

clang/lib/Basic/FileEntry.cpp

+//===- FileEntry.cpp - File references --------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// Defines implementation for clang::FileEntry and clang::FileEntryRef.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/FileEntry.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+using namespace clang;
+
+FileEntry::FileEntry() : UniqueID(0, 0), IsNamedPipe(false), IsValid(false) {}
+
+FileEntry::~FileEntry() = default;
+
+void FileEntry::closeFile() const { File.reset(); }

llvm/include/llvm/Support/FileSystem.h

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem/UniqueID.h"
 #include "llvm/Support/MD5.h"
 #include <cassert>
 #include <cstdint>
 #include <ctime>
 #include <memory>
 #include <stack>
 #include <string>
 #include <system_error>
inline perms &operator&=(perms &l, perms r) {
   return l;
 }
 inline perms operator~(perms x) {
   // Avoid UB by explicitly truncating the (unsigned) ~ result.
   return static_cast<perms>(
       static_cast<unsigned short>(~static_cast<unsigned short>(x)));
 }
 
-class UniqueID {
-  uint64_t Device;
-  uint64_t File;
-
-public:
-  UniqueID() = default;
-  UniqueID(uint64_t Device, uint64_t File) : Device(Device), File(File) {}
-
-  bool operator==(const UniqueID &Other) const {
-    return Device == Other.Device && File == Other.File;
-  }
-  bool operator!=(const UniqueID &Other) const { return !(*this == Other); }
-  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; }
-};
-
 /// 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)
   time_t fs_st_atime = 0;
   time_t fs_st_mtime = 0;

llvm/include/llvm/Support/FileSystem/UniqueID.h

+//===- llvm/Support/FileSystem/UniqueID.h - UniqueID for files --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is cut out of llvm/Support/FileSystem.h to allow UniqueID to be
+// reused without bloating the includes.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
+#define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
+
+#include <cstddef>
+
+namespace llvm {
+namespace sys {
+namespace fs {
+
+class UniqueID {
+  uint64_t Device;
+  uint64_t File;
+
+public:
+  UniqueID() = default;
+  UniqueID(uint64_t Device, uint64_t File) : Device(Device), File(File) {}
+
+  bool operator==(const UniqueID &Other) const {
+    return Device == Other.Device && File == Other.File;
+  }
+  bool operator!=(const UniqueID &Other) const { return !(*this == Other); }
+  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; }
+};
+
+} // end namespace fs
+} // end namespace sys
+} // end namespace llvm
+
+#endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H