[clang][lex] NFCI: Use FileEntryRef in ModuleMap::{load,lookup}ModuleMap()

[jansvoboda11]

Summary

This patch changes the return/argument types of ModuleMap::{load,lookup}ModuleMap() from const FileEntry * to FileEntryRef in order to remove uses of the deprecated DirectoryEntry::getName().

[bnbarham]

Few small comments, LGTM otherwise.

clang/include/clang/Lex/HeaderSearch.h

639–640

clang/lib/Frontend/FrontendAction.cpp

450–451

The empty case isn't checked as far as I can tell. The null case wasn't either, but seems like we should just return true?

clang/lib/Lex/HeaderSearch.cpp

1757

Nitpick: could just return F directly in the 3 ifs here.

[jansvoboda11]

clang/lib/Frontend/FrontendAction.cpp

450–451

Good point. I think the entry for MainFileID always exists, so I'll add an assert here.

[bnbarham]

The failure here is likely due to the hack in FileManager::getFileRef:

// FIXME: This hack ensures that `getDir()` will use the path that was
// used to lookup this file, even if we found a file by different path
// first. This is required in order to find a module's structure when its
// headers/module map are mapped in the VFS.

So the directory returned from the FileEntry is the *lookup* directory, but the FileEntryRef will be giving back the directory from the external path (if it has been remapped).

[jansvoboda11]

Rebase

[jansvoboda11]

TODO: Try rebasing this on top of D151398.

[jansvoboda11]

Rebase on top of D151398

clang/include/clang/Lex/HeaderSearch.h

public:
   /// in subdirectories.
   ///
   /// \returns The module with the given name.
   Module *lookupModule(StringRef ModuleName,
                        SourceLocation ImportLoc = SourceLocation(),
                        bool AllowSearch = true,
                        bool AllowExtraModuleMapSearch = false);
 
   /// Try to find a module map file in the given directory, returning
-  /// \c nullptr if none is found.
-  const FileEntry *lookupModuleMapFile(const DirectoryEntry *Dir,
+  /// \c nullopt if none is found.
+  OptionalFileEntryRef lookupModuleMapFile(const DirectoryEntry *Dir,
                                            bool IsFramework);
 
   /// Determine whether there is a module map that may map the header
   /// with the given file name to a (sub)module.
   /// Always returns false if modules are disabled.
   ///
   /// \param Filename The name of the file.
   ///
   /// \param Root The "root" directory, at which we should stop looking for
public:
   ///        processing a preprocessed module), the ID of the file.
   /// \param Offset [inout] An offset within ID to start parsing. On exit,
   ///        filled by the end of the parsed contents (either EOF or the
   ///        location of an end-of-module-map pragma).
   /// \param OriginalModuleMapFile The original path to the module map file,
   ///        used to resolve paths within the module (this is required when
   ///        building the module from preprocessed source).
   /// \returns true if an error occurred, false otherwise.
-  bool loadModuleMapFile(const FileEntry *File, bool IsSystem,
-                         FileID ID = FileID(), unsigned *Offset = nullptr,
+  bool loadModuleMapFile(FileEntryRef File, bool IsSystem, FileID ID = FileID(),
+                         unsigned *Offset = nullptr,
                          StringRef OriginalModuleMapFile = StringRef());
 
   /// Collect the set of all known, top-level modules.
   ///
   /// \param Modules Will be filled with the set of known, top-level modules.
   void collectAllModules(SmallVectorImpl<Module *> &Modules);
 
   /// Load all known, top-level system modules.
enum LoadModuleMapResult {
     /// There is was directory with the given name.
     LMM_NoDirectory,
 
     /// There was either no module map file or the module map file was
     /// invalid.
     LMM_InvalidModuleMap
   };
 
-  LoadModuleMapResult loadModuleMapFileImpl(const FileEntry *File,
-                                            bool IsSystem,
+  LoadModuleMapResult loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
                                             DirectoryEntryRef Dir,
                                             FileID ID = FileID(),
                                             unsigned *Offset = nullptr);
 
   /// Try to load the module map file in the given directory.
   ///
   /// \param DirName The name of the directory where we will look for a module
   /// map file.

clang/lib/Frontend/FrontendAction.cpp

static bool loadModuleMapForModuleBuild(CompilerInstance &CI, bool IsSystem,
                                         bool IsPreprocessed,
                                         std::string &PresumedModuleMapFile,
                                         unsigned &Offset) {
   auto &SrcMgr = CI.getSourceManager();
   HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
 
   // Map the current input to a file.
   FileID ModuleMapID = SrcMgr.getMainFileID();
-  const FileEntry *ModuleMap = SrcMgr.getFileEntryForID(ModuleMapID);
+  OptionalFileEntryRef ModuleMap = SrcMgr.getFileEntryRefForID(ModuleMapID);
+  assert(ModuleMap && "MainFileID without FileEntry");
 
   // If the module map is preprocessed, handle the initial line marker;
   // line directives are not part of the module map syntax in general.
   Offset = 0;
   if (IsPreprocessed) {
     SourceLocation EndOfLineMarker =
         ReadOriginalFileName(CI, PresumedModuleMapFile, /*IsModuleMap*/ true);
     if (EndOfLineMarker.isValid())
       Offset = CI.getSourceManager().getDecomposedLoc(EndOfLineMarker).second;
   }
 
   // Load the module map file.
-  if (HS.loadModuleMapFile(ModuleMap, IsSystem, ModuleMapID, &Offset,
+  if (HS.loadModuleMapFile(*ModuleMap, IsSystem, ModuleMapID, &Offset,
                            PresumedModuleMapFile))
     return true;
 
   if (SrcMgr.getBufferOrFake(ModuleMapID).getBufferSize() == Offset)
     Offset = 0;
 
   // Infer framework module if possible.
   if (HS.getModuleMap().canInferFrameworkModule(ModuleMap->getDir())) {
-    SmallString<128> InferredFrameworkPath = ModuleMap->getDir()->getName();
+    SmallString<128> InferredFrameworkPath = ModuleMap->getDir().getName();
     llvm::sys::path::append(InferredFrameworkPath,
                             CI.getLangOpts().ModuleName + ".framework");
     if (auto Dir = CI.getFileManager().getDirectory(InferredFrameworkPath))
       (void)HS.getModuleMap().inferFrameworkModule(*Dir, IsSystem, nullptr);
   }
 
   return false;
 }
bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   }
 
   // Initialize the action.
   if (!BeginSourceFileAction(CI))
     return false;
 
   // If we were asked to load any module map files, do so now.
   for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-    if (auto File = CI.getFileManager().getFile(Filename))
+    if (auto File = CI.getFileManager().getOptionalFileRef(Filename))
       CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
           *File, /*IsSystem*/false);
     else
       CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
   }
 
   // If compiling implementation of a module, load its module map file now.
   (void)CI.getPreprocessor().getCurrentModuleImplementation();

clang/lib/Lex/HeaderSearch.cpp

if (needModuleLookup(RequestingModule, SuggestedModule)) {
     // important so that we're consistent about whether this header
     // corresponds to a module. Possibly we should lock down framework modules
     // so that this is not possible.
     return suggestModule(*this, File, RequestingModule, SuggestedModule);
   }
   return true;
 }
 
-static const FileEntry *getPrivateModuleMap(const FileEntry *File,
+static const FileEntry *getPrivateModuleMap(FileEntryRef File,
                                             FileManager &FileMgr) {
-  StringRef Filename = llvm::sys::path::filename(File->getName());
-  SmallString<128>  PrivateFilename(File->getDir()->getName());
+  StringRef Filename = llvm::sys::path::filename(File.getName());
+  SmallString<128>  PrivateFilename(File.getDir().getName());
   if (Filename == "module.map")
     llvm::sys::path::append(PrivateFilename, "module_private.map");
   else if (Filename == "module.modulemap")
     llvm::sys::path::append(PrivateFilename, "module.private.modulemap");
   else
     return nullptr;
   if (auto File = FileMgr.getFile(PrivateFilename))
     return *File;
   return nullptr;
 }
 
-bool HeaderSearch::loadModuleMapFile(const FileEntry *File, bool IsSystem,
+bool HeaderSearch::loadModuleMapFile(FileEntryRef File, bool IsSystem,
                                      FileID ID, unsigned *Offset,
                                      StringRef OriginalModuleMapFile) {
   // Find the directory for the module. For frameworks, that may require going
   // up from the 'Modules' directory.
   OptionalDirectoryEntryRef Dir;
   if (getHeaderSearchOpts().ModuleMapFileHomeIsCwd) {
     Dir = FileMgr.getOptionalDirectoryRef(".");
   } else {
     if (!OriginalModuleMapFile.empty()) {
       // We're building a preprocessed module map. Find or invent the directory
       // that it originally occupied.
       Dir = FileMgr.getOptionalDirectoryRef(
           llvm::sys::path::parent_path(OriginalModuleMapFile));
       if (!Dir) {
         auto FakeFile = FileMgr.getVirtualFileRef(OriginalModuleMapFile, 0, 0);
         Dir = FakeFile.getDir();
       }
     } else {
-      // TODO: Replace with `Dir = File.getDir()` when `File` is switched to
-      // `FileEntryRef`.
-      Dir = FileMgr.getOptionalDirectoryRef(File->getDir()->getName());
+      Dir = File.getDir();
     }
 
     assert(Dir && "parent must exist");
     StringRef DirName(Dir->getName());
     if (llvm::sys::path::filename(DirName) == "Modules") {
       DirName = llvm::sys::path::parent_path(DirName);
       if (DirName.endswith(".framework"))
         if (auto MaybeDir = FileMgr.getOptionalDirectoryRef(DirName))
bool HeaderSearch::loadModuleMapFile(FileEntryRef File, bool IsSystem,
   case LMM_NoDirectory:
   case LMM_InvalidModuleMap:
     return true;
   }
   llvm_unreachable("Unknown load module map result");
 }
 
 HeaderSearch::LoadModuleMapResult
-HeaderSearch::loadModuleMapFileImpl(const FileEntry *File, bool IsSystem,
+HeaderSearch::loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
                                     DirectoryEntryRef Dir, FileID ID,
                                     unsigned *Offset) {
-  assert(File && "expected FileEntry");
-
   // Check whether we've already loaded this module map, and mark it as being
   // loaded in case we recursively try to load it from itself.
   auto AddResult = LoadedModuleMaps.insert(std::make_pair(File, true));
   if (!AddResult.second)
     return AddResult.first->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;
 
   if (ModMap.parseModuleMapFile(File, IsSystem, Dir, ID, Offset)) {
     LoadedModuleMaps[File] = false;
     return LMM_InvalidModuleMap;
   }
 
   // Try to load a corresponding private module map.
   if (const FileEntry *PMMFile = getPrivateModuleMap(File, FileMgr)) {
     if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir)) {
       LoadedModuleMaps[File] = false;
       return LMM_InvalidModuleMap;
     }
   }
 
   // This directory has a module map.
   return LMM_NewlyLoaded;
 }
 
-const FileEntry *
+OptionalFileEntryRef
 HeaderSearch::lookupModuleMapFile(const DirectoryEntry *Dir, bool IsFramework) {
   if (!HSOpts->ImplicitModuleMaps)
-    return nullptr;
+    return std::nullopt;
   // For frameworks, the preferred spelling is Modules/module.modulemap, but
   // module.map at the framework root is also accepted.
   SmallString<128> ModuleMapFileName(Dir->getName());
   if (IsFramework)
     llvm::sys::path::append(ModuleMapFileName, "Modules");
   llvm::sys::path::append(ModuleMapFileName, "module.modulemap");
-  if (auto F = FileMgr.getFile(ModuleMapFileName))
+  if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName))
     return *F;
 
   // Continue to allow module.map
   ModuleMapFileName = Dir->getName();
   llvm::sys::path::append(ModuleMapFileName, "module.map");
-  if (auto F = FileMgr.getFile(ModuleMapFileName))
+  if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName))
     return *F;
 
   // For frameworks, allow to have a private module map with a preferred
   // spelling when a public module map is absent.
   if (IsFramework) {
     ModuleMapFileName = Dir->getName();
     llvm::sys::path::append(ModuleMapFileName, "Modules",
                             "module.private.modulemap");
-    if (auto F = FileMgr.getFile(ModuleMapFileName))
+    if (auto F = FileMgr.getOptionalFileRef(ModuleMapFileName))
       return *F;
   }
-  return nullptr;
+  return std::nullopt;
 }
 
 Module *HeaderSearch::loadFrameworkModule(StringRef Name, DirectoryEntryRef Dir,
                                           bool IsSystem) {
   if (Module *Module = ModMap.findModule(Name))
     return Module;
 
   // Try to load a module map file.

 
 HeaderSearch::LoadModuleMapResult
 HeaderSearch::loadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
                                 bool IsFramework) {
   auto KnownDir = DirectoryHasModuleMap.find(Dir);
   if (KnownDir != DirectoryHasModuleMap.end())
     return KnownDir->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;
 
-  if (const FileEntry *ModuleMapFile = lookupModuleMapFile(Dir, IsFramework)) {
+  if (OptionalFileEntryRef ModuleMapFile =
+          lookupModuleMapFile(Dir, IsFramework)) {
     LoadModuleMapResult Result =
-        loadModuleMapFileImpl(ModuleMapFile, IsSystem, Dir);
+        loadModuleMapFileImpl(*ModuleMapFile, IsSystem, Dir);
     // Add Dir explicitly in case ModuleMapFile is in a subdirectory.
     // E.g. Foo.framework/Modules/module.modulemap
     //      ^Dir                  ^ModuleMapFile
     if (Result == LMM_NewlyLoaded)
       DirectoryHasModuleMap[Dir] = true;
     else if (Result == LMM_InvalidModuleMap)
       DirectoryHasModuleMap[Dir] = false;
     return Result;

clang/lib/Lex/ModuleMap.cpp

if (llvm::sys::path::has_parent_path(FrameworkDirName)) {
         // Check whether we have already looked into the parent directory
         // for a module map.
         llvm::DenseMap<const DirectoryEntry *, InferredDirectory>::const_iterator
           inferred = InferredDirectories.find(*ParentDir);
         if (inferred == InferredDirectories.end()) {
           // We haven't looked here before. Load a module map, if there is
           // one.
           bool IsFrameworkDir = Parent.endswith(".framework");
-          if (const FileEntry *ModMapFile =
+          if (OptionalFileEntryRef ModMapFile =
                   HeaderInfo.lookupModuleMapFile(*ParentDir, IsFrameworkDir)) {
-            parseModuleMapFile(ModMapFile, Attrs.IsSystem, *ParentDir);
+            parseModuleMapFile(*ModMapFile, Attrs.IsSystem, *ParentDir);
             inferred = InferredDirectories.find(*ParentDir);
           }
 
           if (inferred == InferredDirectories.end())
             inferred = InferredDirectories.insert(
                          std::make_pair(*ParentDir, InferredDirectory())).first;
         }