[clang][modules] NFCI: Distinguish as-written and effective umbrella directories

[jansvoboda11]

Summary

For modules with umbrellas, we track how they were written in the module map. Unfortunately, the getter for the umbrella directory conflates the "as written" directory and the "effective" directory (either the written one or the parent of the written umbrella header).

This patch makes the distinction between "as written" and "effective" umbrella directories clearer. No functional change intended.

[benlangmuir]

clang/include/clang/Basic/Module.h

655

Nit: I would prefer we use the same ...AsWritten naming scheme for these methods that we use for the UmbrellaAsWritten field.

clang-tools-extra/modularize/CoverageChecker.cpp

 }
 
 // Collect referenced headers from one module.
 // Collects the headers referenced in the given module into
 // ModuleMapHeadersSet.
 // FIXME: Doesn't collect files from umbrella header.
 bool CoverageChecker::collectModuleHeaders(const Module &Mod) {
 
-  if (const FileEntry *UmbrellaHeader = Mod.getUmbrellaHeader().Entry) {
+  if (const FileEntry *UmbrellaHeader =
+          Mod.getUmbrellaHeaderAsWritten().Entry) {
     // Collect umbrella header.
     ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(
       UmbrellaHeader->getName()));
     // Preprocess umbrella header and collect the headers it references.
     if (!collectUmbrellaHeaderHeaders(UmbrellaHeader->getName()))
       return false;
-  }
-  else if (const DirectoryEntry *UmbrellaDir = Mod.getUmbrellaDir().Entry) {
+  } else if (const DirectoryEntry *UmbrellaDir =
+                 Mod.getUmbrellaDirAsWritten().Entry) {
     // Collect headers in umbrella directory.
     if (!collectUmbrellaHeaders(UmbrellaDir->getName()))
       return false;
   }
 
   for (auto &HeaderKind : Mod.Headers)
     for (auto &Header : HeaderKind)
       ModuleMapHeadersSet.insert(ModularizeUtilities::getCanonicalPath(

clang-tools-extra/modularize/ModularizeUtilities.cpp

bool ModularizeUtilities::collectModuleHeaders(const clang::Module &Mod) {
 
   // Treat headers in umbrella directory as dependencies.
   DependentsVector UmbrellaDependents;
 
   // Recursively do submodules.
   for (auto *Submodule : Mod.submodules())
     collectModuleHeaders(*Submodule);
 
-  if (const FileEntry *UmbrellaHeader = Mod.getUmbrellaHeader().Entry) {
+  if (const FileEntry *UmbrellaHeader =
+          Mod.getUmbrellaHeaderAsWritten().Entry) {
     std::string HeaderPath = getCanonicalPath(UmbrellaHeader->getName());
     // Collect umbrella header.
     HeaderFileNames.push_back(HeaderPath);
 
     // FUTURE: When needed, umbrella header header collection goes here.
-  }
-  else if (const DirectoryEntry *UmbrellaDir = Mod.getUmbrellaDir().Entry) {
+  } else if (const DirectoryEntry *UmbrellaDir =
+                 Mod.getUmbrellaDirAsWritten().Entry) {
     // If there normal headers, assume these are umbrellas and skip collection.
     if (Mod.Headers->size() == 0) {
       // Collect headers in umbrella directory.
       if (!collectUmbrellaHeaders(UmbrellaDir->getName(), UmbrellaDependents))
         return false;
     }
   }
 

clang/include/clang/Basic/Module.h

public:
   }
 
   /// Set the serialized AST file for the top-level module of this module.
   void setASTFile(OptionalFileEntryRef File) {
     assert((!getASTFile() || getASTFile() == File) && "file path changed");
     getTopLevelModule()->ASTFile = File;
   }
 
-  /// Retrieve the directory for which this module serves as the
-  /// umbrella.
-  DirectoryName getUmbrellaDir() const;
+  /// Retrieve the umbrella directory as written.
+  DirectoryName getUmbrellaDirAsWritten() const {
+    if (const auto *ME = Umbrella.dyn_cast<const DirectoryEntry *>())
+      return DirectoryName{UmbrellaAsWritten,
+                           UmbrellaRelativeToRootModuleDirectory, ME};
+    return DirectoryName{};
+  }
 
-  /// Retrieve the header that serves as the umbrella header for this
-  /// module.
-  Header getUmbrellaHeader() const {
-    if (auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
+  /// Retrieve the umbrella header as written.
+  Header getUmbrellaHeaderAsWritten() const {
+    if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
       return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
                     FileEntryRef(*ME)};
     return Header{};
   }
 
+  /// Get the effective umbrella directory for this module: either the one
+  /// explicitly written in the module map file, or the parent of the umbrella
+  /// header.
+  const DirectoryEntry *getEffectiveUmbrellaDir() const;
+
   /// 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 *>();
   }
 
   /// Add a top-level header associated with this module.
   void addTopHeader(const FileEntry *File);

clang/include/clang/Lex/ModuleMap.h

public:
   /// \param Mod The module whose conflicts should be resolved.
   ///
   /// \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, FileEntryRef UmbrellaHeader,
+  /// Sets the umbrella header of the given module to the given header.
+  void
+  setUmbrellaHeaderAsWritten(Module *Mod, FileEntryRef UmbrellaHeader,
                              const Twine &NameAsWritten,
                              const Twine &PathRelativeToRootModuleDirectory);
 
-  /// Sets the umbrella directory of the given module to the given
-  /// directory.
-  void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
+  /// Sets the umbrella directory of the given module to the given directory.
+  void setUmbrellaDirAsWritten(Module *Mod, const DirectoryEntry *UmbrellaDir,
                                const Twine &NameAsWritten,
                                const Twine &PathRelativeToRootModuleDirectory);
 
   /// 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);
 
   /// Parse the given module map file, and record any modules we
   /// encounter.

clang/lib/Basic/Module.cpp

bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
   for (const Module *M = this; M; M = M->Parent) {
     if (nameParts.empty() || M->Name != nameParts.back())
       return false;
     nameParts = nameParts.drop_back();
   }
   return nameParts.empty();
 }
 
-Module::DirectoryName Module::getUmbrellaDir() const {
-  if (Header U = getUmbrellaHeader())
-    return {"", "", U.Entry->getDir()};
-
-  return {UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
-          Umbrella.dyn_cast<const DirectoryEntry *>()};
+const DirectoryEntry *Module::getEffectiveUmbrellaDir() const {
+  if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
+    return FileEntryRef(*ME).getDir();
+  if (const auto *ME = Umbrella.dyn_cast<const DirectoryEntry *>())
+    return ME;
+  return nullptr;
 }
 
 void Module::addTopHeader(const FileEntry *File) {
   assert(File);
   TopHeaders.insert(File);
 }
 
 ArrayRef<const FileEntry *> Module::getTopHeaders(FileManager &FileMgr) {
for (unsigned I = 0, N = Requirements.size(); I != N; ++I) {
         OS << ", ";
       if (!Requirements[I].second)
         OS << "!";
       OS << Requirements[I].first;
     }
     OS << "\n";
   }
 
-  if (Header H = getUmbrellaHeader()) {
+  if (Header H = getUmbrellaHeaderAsWritten()) {
     OS.indent(Indent + 2);
     OS << "umbrella header \"";
     OS.write_escaped(H.NameAsWritten);
     OS << "\"\n";
-  } else if (DirectoryName D = getUmbrellaDir()) {
+  } else if (DirectoryName D = getUmbrellaDirAsWritten()) {
     OS.indent(Indent + 2);
     OS << "umbrella \"";
     OS.write_escaped(D.NameAsWritten);
     OS << "\"\n";
   }
 
   if (!ConfigMacros.empty() || ConfigMacrosExhaustive) {
     OS.indent(Indent + 2);

clang/lib/Frontend/FrontendAction.cpp

for (Module::Header &H : Module->Headers[HK]) {
       // the module map file) so this will find the same file that we found
       // while parsing the module map.
       addHeaderInclude(H.PathRelativeToRootModuleDirectory, Includes, LangOpts,
                        Module->IsExternC);
     }
   }
   // Note that Module->PrivateHeaders will not be a TopHeader.
 
-  if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
+  if (Module::Header UmbrellaHeader = Module->getUmbrellaHeaderAsWritten()) {
     Module->addTopHeader(UmbrellaHeader.Entry);
     if (Module->Parent)
       // Include the umbrella header for submodules.
       addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
                        Includes, LangOpts, Module->IsExternC);
-  } else if (Module::DirectoryName UmbrellaDir = Module->getUmbrellaDir()) {
+  } else if (Module::DirectoryName UmbrellaDir =
+                 Module->getUmbrellaDirAsWritten()) {
     // Add all of the headers we find in this subdirectory.
     std::error_code EC;
     SmallString<128> DirNative;
     llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
 
     llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
     SmallVector<
         std::pair<std::string, OptionalFileEntryRefDegradesToFileEntryPtr>, 8>

 /// Compute the input buffer that should be used to build the specified module.
 static std::unique_ptr<llvm::MemoryBuffer>
 getInputBufferForModule(CompilerInstance &CI, Module *M) {
   FileManager &FileMgr = CI.getFileManager();
 
   // Collect the set of #includes we need to build the module.
   SmallString<256> HeaderContents;
   std::error_code Err = std::error_code();
-  if (Module::Header UmbrellaHeader = M->getUmbrellaHeader())
+  if (Module::Header UmbrellaHeader = M->getUmbrellaHeaderAsWritten())
     addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
                      HeaderContents, CI.getLangOpts(), M->IsExternC);
   Err = collectModuleHeaderIncludes(
       CI.getLangOpts(), FileMgr, CI.getDiagnostics(),
       CI.getPreprocessor().getHeaderSearchInfo().getModuleMap(), M,
       HeaderContents);
 
   if (Err) {

clang/lib/Lex/ModuleMap.cpp

if (OptionalFileEntryRef File =
           findHeader(Mod, Header, RelativePathName, NeedsFramework)) {
     if (Header.IsUmbrella) {
       const DirectoryEntry *UmbrellaDir = &File->getDir().getDirEntry();
       if (Module *UmbrellaMod = UmbrellaDirs[UmbrellaDir])
         Diags.Report(Header.FileNameLoc, diag::err_mmap_umbrella_clash)
           << UmbrellaMod->getFullModuleName();
       else
         // Record this umbrella header.
-        setUmbrellaHeader(Mod, *File, Header.FileName, RelativePathName.str());
+        setUmbrellaHeaderAsWritten(Mod, *File, Header.FileName,
+                                   RelativePathName.str());
     } else {
       Module::Header H = {Header.FileName, std::string(RelativePathName.str()),
                           *File};
       addHeader(Mod, H, headerKindToRole(Header.Kind));
     }
   } else if (Header.HasBuiltinHeader && !Header.Size && !Header.ModTime) {
     // There's a builtin header but no corresponding on-disk header. Assume
     // this was supposed to modularize the builtin header alone.
ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
   SmallVector<const DirectoryEntry *, 2> SkippedDirs;
   KnownHeader H = findHeaderInUmbrellaDirs(File, SkippedDirs);
   if (H) {
     Module *Result = H.getModule();
 
     // Search up the module stack until we find a module with an umbrella
     // directory.
     Module *UmbrellaModule = Result;
-    while (!UmbrellaModule->getUmbrellaDir() && UmbrellaModule->Parent)
+    while (!UmbrellaModule->getEffectiveUmbrellaDir() && UmbrellaModule->Parent)
       UmbrellaModule = UmbrellaModule->Parent;
 
     if (UmbrellaModule->InferSubmodules) {
       OptionalFileEntryRefDegradesToFileEntryPtr UmbrellaModuleMap =
           getModuleMapFileForUniquing(UmbrellaModule);
 
       // Infer submodules for each of the directories we found between
       // the directory of the umbrella header and the directory where
do {
     if (KnownDir != UmbrellaDirs.end()) {
       Module *Found = KnownDir->second;
       if (IsUnavailable(Found))
         return true;
 
       // Search up the module stack until we find a module with an umbrella
       // directory.
       Module *UmbrellaModule = Found;
-      while (!UmbrellaModule->getUmbrellaDir() && UmbrellaModule->Parent)
+      while (!UmbrellaModule->getEffectiveUmbrellaDir() &&
+             UmbrellaModule->Parent)
         UmbrellaModule = UmbrellaModule->Parent;
 
       if (UmbrellaModule->InferSubmodules) {
         for (const DirectoryEntry *SkippedDir : llvm::reverse(SkippedDirs)) {
           // Find or create the module that corresponds to this directory name.
           SmallString<32> NameBuf;
           StringRef Name = sanitizeFilenameAsIdentifier(
               llvm::sys::path::stem(SkippedDir->getName()), NameBuf);
Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
   Result->Directory = FrameworkDir;
 
   // Chop off the first framework bit, as that is implied.
   StringRef RelativePath = UmbrellaName.str().substr(
       Result->getTopLevelModule()->Directory->getName().size());
   RelativePath = llvm::sys::path::relative_path(RelativePath);
 
   // umbrella header "umbrella-header-name"
-  setUmbrellaHeader(Result, *UmbrellaHeader, ModuleName + ".h", RelativePath);
+  setUmbrellaHeaderAsWritten(Result, *UmbrellaHeader, ModuleName + ".h",
+                             RelativePath);
 
   // export *
   Result->Exports.push_back(Module::ExportDecl(nullptr, true));
 
   // module * { export * }
   Result->InferSubmodules = true;
   Result->InferExportWildcard = true;
 
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(
+void ModuleMap::setUmbrellaHeaderAsWritten(
     Module *Mod, FileEntryRef UmbrellaHeader, const Twine &NameAsWritten,
     const Twine &PathRelativeToRootModuleDirectory) {
   Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
   Mod->Umbrella = &UmbrellaHeader.getMapEntry();
   Mod->UmbrellaAsWritten = NameAsWritten.str();
   Mod->UmbrellaRelativeToRootModuleDirectory =
       PathRelativeToRootModuleDirectory.str();
   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,
-                               const Twine &NameAsWritten,
-                               const Twine &PathRelativeToRootModuleDirectory) {
+void ModuleMap::setUmbrellaDirAsWritten(
+    Module *Mod, const DirectoryEntry *UmbrellaDir, const Twine &NameAsWritten,
   Mod->Umbrella = UmbrellaDir;
   Mod->UmbrellaAsWritten = NameAsWritten.str();
   Mod->UmbrellaRelativeToRootModuleDirectory =
       PathRelativeToRootModuleDirectory.str();
   UmbrellaDirs[UmbrellaDir] = Mod;
 }
 
 void ModuleMap::addUnresolvedHeader(Module *Mod,
void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
   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, DirNameAsWritten, DirName);
+  Map.setUmbrellaDirAsWritten(ActiveModule, Dir, DirNameAsWritten, DirName);
 }
 
 /// Parse a module export declaration.
 ///
 ///   export-declaration:
 ///     'export' wildcard-module-id
 ///
 ///   wildcard-module-id:
void ModuleMapParser::parseInferredModuleDecl(bool Framework, bool Explicit) {
   if (!ActiveModule && !Framework) {
     Diags.Report(StarLoc, diag::err_mmap_top_level_inferred_submodule);
     Failed = true;
   }
 
   if (ActiveModule) {
     // Inferred modules must have umbrella directories.
     if (!Failed && ActiveModule->IsAvailable &&
-        !ActiveModule->getUmbrellaDir()) {
+        !ActiveModule->getEffectiveUmbrellaDir()) {
       Diags.Report(StarLoc, diag::err_mmap_inferred_no_umbrella);
       Failed = true;
     }
 
     // Check for redefinition of an inferred module.
     if (!Failed && ActiveModule->InferSubmodules) {
       Diags.Report(StarLoc, diag::err_mmap_inferred_redef);
       if (ActiveModule->InferredSubmoduleLoc.isValid())

clang/lib/Lex/PPLexerChange.cpp

if (EndPos != CurLexer->BufferStart &&
       --EndPos;
   }
 
   return EndPos;
 }
 
 static void collectAllSubModulesWithUmbrellaHeader(
     const Module &Mod, SmallVectorImpl<const Module *> &SubMods) {
-  if (Mod.getUmbrellaHeader())
+  if (Mod.getUmbrellaHeaderAsWritten())
     SubMods.push_back(&Mod);
   for (auto *M : Mod.submodules())
     collectAllSubModulesWithUmbrellaHeader(*M, SubMods);
 }
 
 void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
-  const Module::Header &UmbrellaHeader = Mod.getUmbrellaHeader();
+  Module::Header UmbrellaHeader = Mod.getUmbrellaHeaderAsWritten();
   assert(UmbrellaHeader.Entry && "Module must use umbrella header");
   const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
   SourceLocation ExpectedHeadersLoc = SourceMgr.getLocForEndOfFile(File);
   if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header,
                                  ExpectedHeadersLoc))
     return;
 
   ModuleMap &ModMap = getHeaderSearchInfo().getModuleMap();
-  const DirectoryEntry *Dir = Mod.getUmbrellaDir().Entry;
+  const DirectoryEntry *Dir = Mod.getEffectiveUmbrellaDir();
   llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
   std::error_code EC;
   for (llvm::vfs::recursive_directory_iterator Entry(FS, Dir->getName(), EC),
        End;
        Entry != End && !EC; Entry.increment(EC)) {
     using llvm::StringSwitch;
 
     // Check whether this entry has an extension typically associated with

clang/lib/Serialization/ASTReader.cpp

while (true) {
 
     case SUBMODULE_UMBRELLA_HEADER: {
       // FIXME: This doesn't work for framework modules as `Filename` is the
       //        name as written in the module file and does not include
       //        `Headers/`, so this path will never exist.
       std::string Filename = std::string(Blob);
       ResolveImportedPath(F, Filename);
       if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
-        if (!CurrentModule->getUmbrellaHeader()) {
+        if (!CurrentModule->getUmbrellaHeaderAsWritten()) {
           // FIXME: NameAsWritten
-          ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob, "");
+          ModMap.setUmbrellaHeaderAsWritten(CurrentModule, *Umbrella, Blob, "");
         }
         // Note that it's too late at this point to return out of date if the
         // name from the PCM doesn't match up with the one in the module map,
         // but also quite unlikely since we will have already checked the
         // modification time and size of the module map file itself.
       }
       break;
     }
case SUBMODULE_TOPHEADER: {
       break;
     }
 
     case SUBMODULE_UMBRELLA_DIR: {
       // See comments in SUBMODULE_UMBRELLA_HEADER
       std::string Dirname = std::string(Blob);
       ResolveImportedPath(F, Dirname);
       if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
-        if (!CurrentModule->getUmbrellaDir()) {
+        if (!CurrentModule->getUmbrellaDirAsWritten()) {
           // FIXME: NameAsWritten
-          ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob, "");
+          ModMap.setUmbrellaDirAsWritten(CurrentModule, *Umbrella, Blob, "");
         }
       }
       break;
     }
 
     case SUBMODULE_METADATA: {
       F.BaseSubmoduleID = getTotalNumSubmodules();
       F.LocalNumSubmodules = Record[0];

clang/lib/Serialization/ASTWriter.cpp

while (!Q.empty()) {
 
     // Emit the requirements.
     for (const auto &R : Mod->Requirements) {
       RecordData::value_type Record[] = {SUBMODULE_REQUIRES, R.second};
       Stream.EmitRecordWithBlob(RequiresAbbrev, Record, R.first);
     }
 
     // Emit the umbrella header, if there is one.
-    if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
+    if (Module::Header UmbrellaHeader = Mod->getUmbrellaHeaderAsWritten()) {
       RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
       Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
                                 UmbrellaHeader.NameAsWritten);
-    } else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
+    } else if (Module::DirectoryName UmbrellaDir =
+                   Mod->getUmbrellaDirAsWritten()) {
       RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
       Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
                                 UmbrellaDir.NameAsWritten);
     }
 
     // Emit the headers.
     struct {
       unsigned RecordKind;