[llvm-branch-commits] [clang] 32c501d - Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, and DirectoryName, NFC

Duncan P. N. Exon Smith via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Dec 2 14:11:44 PST 2020


Author: Duncan P. N. Exon Smith
Date: 2020-12-02T14:07:23-08:00
New Revision: 32c501dd88b62787d3a5ffda7aabcf4650dbe3cd

URL: https://github.com/llvm/llvm-project/commit/32c501dd88b62787d3a5ffda7aabcf4650dbe3cd
DIFF: https://github.com/llvm/llvm-project/commit/32c501dd88b62787d3a5ffda7aabcf4650dbe3cd.diff

LOG: Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, and DirectoryName, NFC

Push `FileEntryRef` and `DirectoryEntryRef` further, using it them
`Module::Umbrella`, `Module::Header::Entry`, and
`Module::DirectoryName::Entry`.

- Add `DirectoryEntryRef::operator const DirectoryEntry *` and
  `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr`, to get the
  same "degrades to `DirectoryEntry*` behaviour `FileEntryRef` enjoys
  (this avoids a bunch of churn in various clang tools).
- Fix the `DirectoryEntryRef` constructor from `MapEntry` to take it by
  `const&`.

Note that we cannot get rid of the `...AsWritten` names leveraging the
new classes, since these need to be as written in the `ModuleMap` file
and the module directory path is preprended for the lookup in the
`FileManager`.

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

Added: 
    

Modified: 
    clang/include/clang/Basic/DirectoryEntry.h
    clang/include/clang/Basic/Module.h
    clang/include/clang/Lex/ModuleMap.h
    clang/lib/Basic/Module.cpp
    clang/lib/Frontend/FrontendActions.cpp
    clang/lib/Lex/ModuleMap.cpp
    clang/lib/Serialization/ASTReader.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DirectoryEntry.h b/clang/include/clang/Basic/DirectoryEntry.h
index a6ec5f89aa28..4e229962bdcc 100644
--- a/clang/include/clang/Basic/DirectoryEntry.h
+++ b/clang/include/clang/Basic/DirectoryEntry.h
@@ -51,7 +51,26 @@ class DirectoryEntryRef {
   const MapEntry &getMapEntry() const { return *ME; }
 
   DirectoryEntryRef() = delete;
-  DirectoryEntryRef(MapEntry &ME) : ME(&ME) {}
+  DirectoryEntryRef(const MapEntry &ME) : ME(&ME) {}
+
+  /// Allow DirectoryEntryRef to degrade into 'const DirectoryEntry*' to
+  /// facilitate incremental adoption.
+  ///
+  /// The goal is to avoid code churn due to dances like the following:
+  /// \code
+  /// // Old code.
+  /// lvalue = rvalue;
+  ///
+  /// // Temporary code from an incremental patch.
+  /// lvalue = &rvalue.getDirectoryEntry();
+  ///
+  /// // Final code.
+  /// lvalue = rvalue;
+  /// \endcode
+  ///
+  /// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::getName
+  /// has been deleted, delete this implicit conversion.
+  operator const DirectoryEntry *() const { return &getDirEntry(); }
 
 private:
   friend class FileMgr::MapEntryOptionalStorage<DirectoryEntryRef>;
@@ -147,4 +166,76 @@ static_assert(
 } // end namespace optional_detail
 } // end namespace llvm
 
+namespace clang {
+
+/// Wrapper around Optional<DirectoryEntryRef> that degrades to 'const
+/// DirectoryEntry*', facilitating incremental patches to propagate
+/// DirectoryEntryRef.
+///
+/// This class can be used as return value or field where it's convenient for
+/// an Optional<DirectoryEntryRef> to degrade to a 'const DirectoryEntry*'. The
+/// purpose is to avoid code churn due to dances like the following:
+/// \code
+/// // Old code.
+/// lvalue = rvalue;
+///
+/// // Temporary code from an incremental patch.
+/// Optional<DirectoryEntryRef> MaybeF = rvalue;
+/// lvalue = MaybeF ? &MaybeF.getDirectoryEntry() : nullptr;
+///
+/// // Final code.
+/// lvalue = rvalue;
+/// \endcode
+///
+/// FIXME: Once DirectoryEntryRef is "everywhere" and DirectoryEntry::LastRef
+/// and DirectoryEntry::getName have been deleted, delete this class and
+/// replace instances with Optional<DirectoryEntryRef>.
+class OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr
+    : public Optional<DirectoryEntryRef> {
+public:
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr() = default;
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
+      OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(
+      const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
+  operator=(OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &&) = default;
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
+  operator=(const OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &) = default;
+
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(llvm::NoneType) {}
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(DirectoryEntryRef Ref)
+      : Optional<DirectoryEntryRef>(Ref) {}
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr(Optional<DirectoryEntryRef> MaybeRef)
+      : Optional<DirectoryEntryRef>(MaybeRef) {}
+
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(llvm::NoneType) {
+    Optional<DirectoryEntryRef>::operator=(None);
+    return *this;
+  }
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &operator=(DirectoryEntryRef Ref) {
+    Optional<DirectoryEntryRef>::operator=(Ref);
+    return *this;
+  }
+  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr &
+  operator=(Optional<DirectoryEntryRef> MaybeRef) {
+    Optional<DirectoryEntryRef>::operator=(MaybeRef);
+    return *this;
+  }
+
+  /// Degrade to 'const DirectoryEntry *' to allow  DirectoryEntry::LastRef and
+  /// DirectoryEntry::getName have been deleted, delete this class and replace
+  /// instances with Optional<DirectoryEntryRef>
+  operator const DirectoryEntry *() const {
+    return hasValue() ? &getValue().getDirEntry() : nullptr;
+  }
+};
+
+static_assert(std::is_trivially_copyable<
+                  OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr>::value,
+              "OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr should be "
+              "trivially copyable");
+
+} // end namespace clang
+
 #endif // LLVM_CLANG_BASIC_DIRECTORYENTRY_H

diff  --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 82ea1f462949..f5b176f8d20b 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -133,7 +133,9 @@ class Module {
   std::string PresumedModuleMapFile;
 
   /// The umbrella header or directory.
-  llvm::PointerUnion<const FileEntry *, const DirectoryEntry *> Umbrella;
+  llvm::PointerUnion<const FileEntryRef::MapEntry *,
+                     const DirectoryEntryRef::MapEntry *>
+      Umbrella;
 
   /// The module signature.
   ASTFileSignature Signature;
@@ -188,18 +190,18 @@ class Module {
   /// file.
   struct Header {
     std::string NameAsWritten;
-    const FileEntry *Entry;
+    OptionalFileEntryRefDegradesToFileEntryPtr Entry;
 
-    explicit operator bool() { return Entry; }
+    explicit operator bool() { return Entry != None; }
   };
 
   /// Information about a directory name as found in the module map
   /// file.
   struct DirectoryName {
     std::string NameAsWritten;
-    const DirectoryEntry *Entry;
+    OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr Entry;
 
-    explicit operator bool() { return Entry; }
+    explicit operator bool() { return Entry != None; }
   };
 
   /// The headers that are part of this module.
@@ -544,15 +546,15 @@ class Module {
   /// Retrieve the header that serves as the umbrella header for this
   /// module.
   Header getUmbrellaHeader() const {
-    if (auto *FE = Umbrella.dyn_cast<const FileEntry *>())
-      return Header{UmbrellaAsWritten, FE};
+    if (auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
+      return Header{UmbrellaAsWritten, FileEntryRef(*ME)};
     return Header{};
   }
 
   /// Determine whether this module has an umbrella directory that is
   /// not based on an umbrella header.
   bool hasUmbrellaDir() const {
-    return Umbrella && Umbrella.is<const DirectoryEntry *>();
+    return Umbrella && Umbrella.is<const DirectoryEntryRef::MapEntry *>();
   }
 
   /// Add a top-level header associated with this module.

diff  --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index f6423e5b4258..6827408f10a3 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_LEX_MODULEMAP_H
 #define LLVM_CLANG_LEX_MODULEMAP_H
 
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
@@ -37,7 +38,6 @@ namespace clang {
 
 class DiagnosticsEngine;
 class DirectoryEntry;
-class FileEntry;
 class FileManager;
 class HeaderSearch;
 class SourceManager;
@@ -648,12 +648,12 @@ class ModuleMap {
 
   /// Sets the umbrella header of the given module to the given
   /// header.
-  void setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
+  void setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
                          Twine NameAsWritten);
 
   /// Sets the umbrella directory of the given module to the given
   /// directory.
-  void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
+  void setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
                       Twine NameAsWritten);
 
   /// Adds this header to the given module.

diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 8a8c400c9503..9284c5947893 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -247,7 +247,10 @@ Module::DirectoryName Module::getUmbrellaDir() const {
   if (Header U = getUmbrellaHeader())
     return {"", U.Entry->getDir()};
 
-  return {UmbrellaAsWritten, Umbrella.dyn_cast<const DirectoryEntry *>()};
+  if (auto *ME = Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
+    return {UmbrellaAsWritten, DirectoryEntryRef(*ME)};
+
+  return {"", None};
 }
 
 void Module::addTopHeader(const FileEntry *File) {

diff  --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index cb2b89e4a571..42531fa3481b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -304,7 +304,7 @@ bool GenerateHeaderModuleAction::BeginSourceFileAction(
         << Name;
       continue;
     }
-    Headers.push_back({std::string(Name), &FE->getFileEntry()});
+    Headers.push_back({std::string(Name), *FE});
   }
   HS.getModuleMap().createHeaderModule(CI.getLangOpts().CurrentModule, Headers);
 

diff  --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 28dd7ed969ad..bbda1f15a702 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -300,7 +300,7 @@ bool ModuleMap::resolveAsBuiltinHeader(
   // supplied by Clang. Find that builtin header.
   SmallString<128> Path;
   llvm::sys::path::append(Path, BuiltinIncludeDir->getName(), Header.FileName);
-  auto File = SourceMgr.getFileManager().getFile(Path);
+  auto File = SourceMgr.getFileManager().getOptionalFileRef(Path);
   if (!File)
     return false;
 
@@ -1012,7 +1012,7 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
   // Look for an umbrella header.
   SmallString<128> UmbrellaName = StringRef(FrameworkDir->getName());
   llvm::sys::path::append(UmbrellaName, "Headers", ModuleName + ".h");
-  auto UmbrellaHeader = FileMgr.getFile(UmbrellaName);
+  auto UmbrellaHeader = FileMgr.getOptionalFileRef(UmbrellaName);
 
   // FIXME: If there's no umbrella header, we could probably scan the
   // framework to load *everything*. But, it's not clear that this is a good
@@ -1121,21 +1121,21 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
   return Result;
 }
 
-void ModuleMap::setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
+void ModuleMap::setUmbrellaHeader(Module *Mod, FileEntryRef UmbrellaHeader,
                                   Twine NameAsWritten) {
   Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
-  Mod->Umbrella = UmbrellaHeader;
+  Mod->Umbrella = &UmbrellaHeader.getMapEntry();
   Mod->UmbrellaAsWritten = NameAsWritten.str();
-  UmbrellaDirs[UmbrellaHeader->getDir()] = Mod;
+  UmbrellaDirs[UmbrellaHeader.getDir()] = Mod;
 
   // Notify callbacks that we just added a new header.
   for (const auto &Cb : Callbacks)
     Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
 }
 
-void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
+void ModuleMap::setUmbrellaDir(Module *Mod, DirectoryEntryRef UmbrellaDir,
                                Twine NameAsWritten) {
-  Mod->Umbrella = UmbrellaDir;
+  Mod->Umbrella = &UmbrellaDir.getMapEntry();
   Mod->UmbrellaAsWritten = NameAsWritten.str();
   UmbrellaDirs[UmbrellaDir] = Mod;
 }
@@ -2416,15 +2416,15 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
   }
 
   // Look for this file.
-  const DirectoryEntry *Dir = nullptr;
+  Optional<DirectoryEntryRef> Dir;
   if (llvm::sys::path::is_absolute(DirName)) {
-    if (auto D = SourceMgr.getFileManager().getDirectory(DirName))
+    if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(DirName))
       Dir = *D;
   } else {
     SmallString<128> PathName;
     PathName = Directory->getName();
     llvm::sys::path::append(PathName, DirName);
-    if (auto D = SourceMgr.getFileManager().getDirectory(PathName))
+    if (auto D = SourceMgr.getFileManager().getOptionalDirectoryRef(PathName))
       Dir = *D;
   }
 
@@ -2445,7 +2445,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
         SourceMgr.getFileManager().getVirtualFileSystem();
     for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
          I != E && !EC; I.increment(EC)) {
-      if (auto FE = SourceMgr.getFileManager().getFile(I->path())) {
+      if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
         Module::Header Header = {std::string(I->path()), *FE};
         Headers.push_back(std::move(Header));
       }
@@ -2459,7 +2459,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
     return;
   }
 
-  if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
+  if (Module *OwningModule = Map.UmbrellaDirs[*Dir]) {
     Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
       << OwningModule->getFullModuleName();
     HadError = true;
@@ -2467,7 +2467,7 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
   }
 
   // Record this umbrella directory.
-  Map.setUmbrellaDir(ActiveModule, Dir, DirName);
+  Map.setUmbrellaDir(ActiveModule, *Dir, DirName);
 }
 
 /// Parse a module export declaration.

diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 6206ea552838..c2ddd8e3df70 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1915,7 +1915,8 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
     // FIXME: This is not always the right filename-as-written, but we're not
     // going to use this information to rebuild the module, so it doesn't make
     // a lot of 
diff erence.
-    Module::Header H = {std::string(key.Filename), *FileMgr.getFile(Filename)};
+    Module::Header H = {std::string(key.Filename),
+                        *FileMgr.getOptionalFileRef(Filename)};
     ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
     HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
   }
@@ -5564,7 +5565,7 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
     case SUBMODULE_UMBRELLA_HEADER: {
       std::string Filename = std::string(Blob);
       ResolveImportedPath(F, Filename);
-      if (auto Umbrella = PP.getFileManager().getFile(Filename)) {
+      if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
         if (!CurrentModule->getUmbrellaHeader())
           ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob);
         else if (CurrentModule->getUmbrellaHeader().Entry != *Umbrella) {
@@ -5597,7 +5598,8 @@ ASTReader::ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
     case SUBMODULE_UMBRELLA_DIR: {
       std::string Dirname = std::string(Blob);
       ResolveImportedPath(F, Dirname);
-      if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
+      if (auto Umbrella =
+              PP.getFileManager().getOptionalDirectoryRef(Dirname)) {
         if (!CurrentModule->getUmbrellaDir())
           ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob);
         else if (CurrentModule->getUmbrellaDir().Entry != *Umbrella) {


        


More information about the llvm-branch-commits mailing list