jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, bnbarham.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Most users of `Module::Header` already assume its `Entry` is populated. Enforce 
this assumption in the type system and handle the only case where this is not 
the case by wrapping the whole struct in `std::optional`. Do the same for 
`Module::DirectoryName`.

Depends on D151584 <https://reviews.llvm.org/D151584>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151586

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2849,11 +2849,11 @@
     if (auto UmbrellaHeader = Mod->getWrittenUmbrellaHeader()) {
       RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
       Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-                                UmbrellaHeader.NameAsWritten);
+                                UmbrellaHeader->NameAsWritten);
     } else if (auto UmbrellaDir = Mod->getWrittenUmbrellaDir()) {
       RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
       Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-                                UmbrellaDir.NameAsWritten);
+                                UmbrellaDir->NameAsWritten);
     }
 
     // Emit the headers.
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1973,10 +1973,11 @@
     std::string Filename = std::string(key.Filename);
     if (key.Imported)
       Reader.ResolveImportedPath(M, Filename);
-    // FIXME: NameAsWritten
-    Module::Header H = {std::string(key.Filename), "",
-                        FileMgr.getOptionalFileRef(Filename)};
-    ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
+    if (auto FE = FileMgr.getOptionalFileRef(Filename)) {
+      // FIXME: NameAsWritten
+      Module::Header H = {std::string(key.Filename), "", *FE};
+      ModMap.addHeader(Mod, H, HeaderRole, /*Imported=*/true);
+    }
     HFI.isModuleHeader |= ModuleMap::isModular(HeaderRole);
   }
 
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -289,9 +289,9 @@
 }
 
 void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
-  Module::Header UmbrellaHeader = Mod.getWrittenUmbrellaHeader();
-  assert(UmbrellaHeader.Entry && "Module must use umbrella header");
-  const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
+  auto UmbrellaHeader = Mod.getWrittenUmbrellaHeader();
+  assert(UmbrellaHeader && "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))
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1291,7 +1291,7 @@
 
   // Notify callbacks that we just added a new header.
   for (const auto &Cb : Callbacks)
-    Cb->moduleMapAddHeader(Header.Entry->getName());
+    Cb->moduleMapAddHeader(Header.Entry.getName());
 }
 
 OptionalFileEntryRef
@@ -2543,7 +2543,7 @@
     for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
          I != E && !EC; I.increment(EC)) {
       if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
-        Module::Header Header = {"", std::string(I->path()), FE};
+        Module::Header Header = {"", std::string(I->path()), *FE};
         Headers.push_back(std::move(Header));
       }
     }
Index: clang/lib/Frontend/FrontendAction.cpp
===================================================================
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -365,16 +365,16 @@
   // Note that Module->PrivateHeaders will not be a TopHeader.
 
   if (auto UmbrellaHeader = Module->getWrittenUmbrellaHeader()) {
-    Module->addTopHeader(UmbrellaHeader.Entry);
+    Module->addTopHeader(UmbrellaHeader->Entry);
     if (Module->Parent)
       // Include the umbrella header for submodules.
-      addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+      addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
                        Includes, LangOpts, Module->IsExternC);
   } else if (auto UmbrellaDir = Module->getWrittenUmbrellaDir()) {
     // 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::sys::path::native(UmbrellaDir->Entry.getName(), DirNative);
 
     llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
     SmallVector<
@@ -406,7 +406,7 @@
       for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
         Components.push_back(*PathIt);
       SmallString<128> RelativeHeader(
-          UmbrellaDir.PathRelativeToRootModuleDirectory);
+          UmbrellaDir->PathRelativeToRootModuleDirectory);
       for (auto It = Components.rbegin(), End = Components.rend(); It != End;
            ++It)
         llvm::sys::path::append(RelativeHeader, *It);
@@ -550,8 +550,8 @@
   // 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->getWrittenUmbrellaHeader())
-    addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+  if (auto UmbrellaHeader = M->getWrittenUmbrellaHeader())
+    addHeaderInclude(UmbrellaHeader->PathRelativeToRootModuleDirectory,
                      HeaderContents, CI.getLangOpts(), M->IsExternC);
   Err = collectModuleHeaderIncludes(
       CI.getLangOpts(), FileMgr, CI.getDiagnostics(),
Index: clang/lib/Basic/Module.cpp
===================================================================
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -483,15 +483,15 @@
     OS << "\n";
   }
 
-  if (Header H = getWrittenUmbrellaHeader()) {
+  if (auto H = getWrittenUmbrellaHeader()) {
     OS.indent(Indent + 2);
     OS << "umbrella header \"";
-    OS.write_escaped(H.NameAsWritten);
+    OS.write_escaped(H->NameAsWritten);
     OS << "\"\n";
-  } else if (DirectoryName D = getWrittenUmbrellaDir()) {
+  } else if (auto D = getWrittenUmbrellaDir()) {
     OS.indent(Indent + 2);
     OS << "umbrella \"";
-    OS.write_escaped(D.NameAsWritten);
+    OS.write_escaped(D->NameAsWritten);
     OS << "\"\n";
   }
 
@@ -523,8 +523,8 @@
       OS.indent(Indent + 2);
       OS << K.Prefix << "header \"";
       OS.write_escaped(H.NameAsWritten);
-      OS << "\" { size " << H.Entry->getSize()
-         << " mtime " << H.Entry->getModificationTime() << " }\n";
+      OS << "\" { size " << H.Entry.getSize()
+         << " mtime " << H.Entry.getModificationTime() << " }\n";
     }
   }
   for (auto *Unresolved : {&UnresolvedHeaders, &MissingHeaders}) {
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -243,9 +243,7 @@
   struct Header {
     std::string NameAsWritten;
     std::string PathRelativeToRootModuleDirectory;
-    OptionalFileEntryRefDegradesToFileEntryPtr Entry;
-
-    explicit operator bool() { return Entry.has_value(); }
+    FileEntryRef Entry;
   };
 
   /// Information about a directory name as found in the module map
@@ -253,9 +251,7 @@
   struct DirectoryName {
     std::string NameAsWritten;
     std::string PathRelativeToRootModuleDirectory;
-    OptionalDirectoryEntryRef Entry;
-
-    explicit operator bool() { return Entry.has_value(); }
+    DirectoryEntryRef Entry;
   };
 
   /// The headers that are part of this module.
@@ -653,21 +649,21 @@
   }
 
   /// Retrieve the explicitly written umbrella directory for this module.
-  DirectoryName getWrittenUmbrellaDir() const {
+  std::optional<DirectoryName> getWrittenUmbrellaDir() const {
     if (const auto *ME =
             Umbrella.dyn_cast<const DirectoryEntryRef::MapEntry *>())
       return DirectoryName{UmbrellaAsWritten,
                            UmbrellaRelativeToRootModuleDirectory,
                            DirectoryEntryRef(*ME)};
-    return DirectoryName{};
+    return std::nullopt;
   }
 
   /// Retrieve the explicitly written umbrella header for this module.
-  Header getWrittenUmbrellaHeader() const {
+  std::optional<Header> getWrittenUmbrellaHeader() const {
     if (const auto *ME = Umbrella.dyn_cast<const FileEntryRef::MapEntry *>())
       return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
                     FileEntryRef(*ME)};
-    return Header{};
+    return std::nullopt;
   }
 
   /// Get the effective umbrella directory for this module: either the one
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to