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

[dexonsmith]

Summary

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.

[dexonsmith]

Removed the FIXMEs about the ...AsWritten fields, as I just took a deeper look and we need them. The headers/etc. are looked up in the FileManager with the module directory prepended, and these ...AsWritten fields are without that.

[dexonsmith]

ping

[dexonsmith]

ping

[jansvoboda11]

LGTM.

[dexonsmith]

This has unexpectedly caused a behaviour change (perhaps the same root cause as https://reviews.llvm.org/D92975#2453482, but the investigation is ongoing). I'll be reverting later today.

clang/include/clang/Basic/DirectoryEntry.h

public:
 
   StringRef getName() const { return ME->getKey(); }
 
   using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<DirectoryEntry &>>;
 
   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>;
   struct optional_none_tag {};
 
   // Private constructor for use by OptionalStorage.
   DirectoryEntryRef(optional_none_tag) : ME(nullptr) {}
   bool hasOptionalValue() const { return ME; }

 
 static_assert(
     std::is_trivially_copyable<Optional<clang::DirectoryEntryRef>>::value,
     "Optional<DirectoryEntryRef> should be trivially copyable");
 
 } // 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

clang/include/clang/Basic/Module.h

public:
   /// are found.
   const DirectoryEntry *Directory = nullptr;
 
   /// The presumed file name for the module map defining this module.
   /// Only non-empty when building from preprocessed source.
   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;
 
   /// The name of the umbrella entry, as written in the module map.
   std::string UmbrellaAsWritten;
 
   /// The module through which entities defined in this module will
enum HeaderKind {
     HK_Excluded
   };
   static const int NumHeaderKinds = HK_Excluded + 1;
 
   /// Information about a header directive as found in the module map
   /// 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.
   SmallVector<Header, 2> Headers[5];
 
   /// Stored information about a header directive that was found in the
   /// module map file but has not been resolved to a file.
   struct UnresolvedHeaderDirective {
public:
 
   /// Retrieve the directory for which this module serves as the
   /// umbrella.
   DirectoryName getUmbrellaDir() const;
 
   /// 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.
   void addTopHeader(const FileEntry *File);
 
   /// Add a top-level header filename associated with this module.
   void addTopHeaderFilename(StringRef Filename) {
     TopHeaderNames.push_back(std::string(Filename));

clang/include/clang/Lex/ModuleMap.h

 //===- ModuleMap.h - Describe the layout of modules -------------*- 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 defines the ModuleMap interface, which describes the layout of a
 // module as it relates to headers.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/ADT/Twine.h"
 #include <ctime>
 #include <memory>
 #include <string>
 #include <utility>
 
 namespace clang {
 
 class DiagnosticsEngine;
 class DirectoryEntry;
-class FileEntry;
 class FileManager;
 class HeaderSearch;
 class SourceManager;
 
 /// A mechanism to observe the actions of the module map parser as it
 /// reads module map files.
 class ModuleMapCallbacks {
   virtual void anchor();
public:
   /// \param Complain Whether to emit diagnostics for failures.
   ///
   /// \returns true if any errors were encountered while resolving conflicts,
   /// false otherwise.
   bool resolveConflicts(Module *Mod, bool Complain);
 
   /// 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.
   /// \param Role The role of the header wrt the module.
   void addHeader(Module *Mod, Module::Header Header,
                  ModuleHeaderRole Role, bool Imported = false);
 
   /// Marks this header as being excluded from the given module.

clang/lib/Basic/Module.cpp

bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
   }
   return nameParts.empty();
 }
 
 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) {
   assert(File);
   TopHeaders.insert(File);
 }
 
 ArrayRef<const FileEntry *> Module::getTopHeaders(FileManager &FileMgr) {

clang/lib/Frontend/FrontendActions.cpp

for (StringRef Name : ModuleHeaders) {
     Optional<FileEntryRef> FE = HS.LookupFile(
         Name, SourceLocation(), /*Angled*/ false, nullptr, CurDir, None,
         nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
     if (!FE) {
       CI.getDiagnostics().Report(diag::err_module_header_file_not_found)
         << Name;
       continue;
     }
-    Headers.push_back({std::string(Name), &FE->getFileEntry()});
+    Headers.push_back({std::string(Name), *FE});
   }
   HS.getModuleMap().createHeaderModule(CI.getLangOpts().CurrentModule, Headers);
 
   return GenerateModuleAction::BeginSourceFileAction(CI);
 }
 
 std::unique_ptr<raw_pwrite_stream>
 GenerateHeaderModuleAction::CreateOutputFile(CompilerInstance &CI,

clang/lib/Lex/ModuleMap.cpp

if (Header.Kind == Module::HK_Excluded ||
       !isBuiltinHeader(Header.FileName))
     return false;
 
   // This is a system module with a top-level header. This header
   // may have a counterpart (or replacement) in the set of headers
   // 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;
 
   auto Role = headerKindToRole(Header.Kind);
   Module::Header H = {std::string(Path.str()), *File};
   addHeader(Mod, H, Role);
   return true;
 }
if (!canInfer)
       return nullptr;
   } else
     ModuleMapFile = getModuleMapFileForUniquing(Parent);
 
 
   // 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
   // idea.
   if (!UmbrellaHeader)
     return nullptr;
 
   Module *Result = new Module(ModuleName, SourceLocation(), Parent,
Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
   Result->ShadowingModule = ShadowingModule;
   Result->markUnavailable(/*Unimportable*/true);
   ModuleScopeIDs[Result] = CurrentModuleScopeID;
   ShadowModules.push_back(Result);
 
   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;
 }
 
 void ModuleMap::addUnresolvedHeader(Module *Mod,
                                     Module::UnresolvedHeaderDirective Header,
                                     bool &NeedsFramework) {
   // If there is a builtin counterpart to this file, add it now so it can
void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
   if (ActiveModule->Umbrella) {
     Diags.Report(DirNameLoc, diag::err_mmap_umbrella_clash)
       << ActiveModule->getFullModuleName();
     HadError = true;
     return;
   }
 
   // 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;
   }
 
   if (!Dir) {
     Diags.Report(DirNameLoc, diag::warn_mmap_umbrella_dir_not_found)
       << DirName;
     return;
   }
 
   if (UsesRequiresExcludedHack.count(ActiveModule)) {
     // Mark this header 'textual' (see doc comment for
     // ModuleMapParser::UsesRequiresExcludedHack). Although iterating over the
     // directory is relatively expensive, in practice this only applies to the
     // uncommonly used Tcl module on Darwin platforms.
     std::error_code EC;
     SmallVector<Module::Header, 6> Headers;
     llvm::vfs::FileSystem &FS =
         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));
       }
     }
 
     // Sort header paths so that the pcm doesn't depend on iteration order.
     llvm::array_pod_sort(Headers.begin(), Headers.end(), compareModuleHeaders);
 
     for (auto &Header : Headers)
       Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);
     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;
     return;
   }
 
   // Record this umbrella directory.
-  Map.setUmbrellaDir(ActiveModule, Dir, DirName);
+  Map.setUmbrellaDir(ActiveModule, *Dir, DirName);
 }
 
 /// Parse a module export declaration.
 ///
 ///   export-declaration:
 ///     'export' wildcard-module-id
 ///
 ///   wildcard-module-id:

clang/lib/Serialization/ASTReader.cpp

ModuleMap &ModMap =
         Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
 
     std::string Filename = std::string(key.Filename);
     if (key.Imported)
       Reader.ResolveImportedPath(M, Filename);
     // 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 difference.
-    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);
   }
 
   // This HeaderFileInfo was externally loaded.
   HFI.External = true;
   HFI.IsValid = true;
   return HFI;
case SUBMODULE_DEFINITION: {
           ParentModule && ParentModule->IsUnimportable;
       CurrentModule->IsAvailable = !CurrentModule->IsUnimportable;
       break;
     }
 
     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) {
           if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
             Error("mismatched umbrella headers in submodule");
           return OutOfDate;
         }
       }
while (true) {
 
     case SUBMODULE_TOPHEADER:
       CurrentModule->addTopHeaderFilename(Blob);
       break;
 
     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) {
           if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
             Error("mismatched umbrella directories in submodule");
           return OutOfDate;
         }
       }