oontvoo updated this revision to Diff 252791.
oontvoo added a comment.

Handle includes/import from outer mods


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75951/new/

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.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
@@ -1619,7 +1619,7 @@
       endian::Writer LE(Out, little);
       unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
       LE.write<uint16_t>(KeyLen);
-      unsigned DataLen = 1 + 2 + 4 + 4;
+      unsigned DataLen = 1 + 2 + 4 + 4 + 4;
       for (auto ModInfo : Data.KnownHeaders)
         if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
           DataLen += 4;
@@ -1676,6 +1676,9 @@
       }
       LE.write<uint32_t>(Offset);
 
+      // Write this file UID.
+      LE.write<uint32_t>(Data.HFI.UID);
+
       auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
         if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
           uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator<HeaderFileInfoTrait> Generator;
   SmallVector<const char *, 4> SavedStrings;
@@ -1781,8 +1784,7 @@
     // changed since it was loaded. Also skip it if it's for a modular header
     // from a different module; in that case, we rely on the module(s)
     // containing the header to provide this information.
-    const HeaderFileInfo *HFI =
-        HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+    HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
     if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
       continue;
 
@@ -1799,8 +1801,13 @@
     HeaderFileInfoTrait::key_type Key = {
       Filename, File->getSize(), getTimestampForOutput(File)
     };
+    // Set the UID for this HFI so that its importers could use it
+    // when serializing.
+    HFI->UID = UID;
     HeaderFileInfoTrait::data_type Data = {
-      *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+        *HFI,
+        HS.getModuleMap().findAllModulesForHeader(File),
+        {},
     };
     Generator.insert(Key, Data, GeneratorTrait);
     ++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
       Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
     }
 
+    // Emit the imported header's UIDs.
+    {
+      auto it = PP->Submodules.find(Mod);
+      if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+        RecordData Record;
+        for (unsigned UID : it->second.IncludedFiles) {
+          // Only save it if the header is actually import/pragma once.
+          // FIXME When we first see a header, it always goes into the mod's
+          // list of included, regardless of whether it was pragma-once or not.
+          // Maybe better to fix that earlier?
+          auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+          if (HFI.isImport || HFI.isPragmaOnce) {
+            Record.push_back(HFI.UID);
+          }
+        }
+        Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+      }
+    }
+
     // Emit the exports.
     if (!Mod->Exports.empty()) {
       RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
 //===- ASTReader.cpp - AST File Reader ------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
     HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  HFI.UID = endian::readNext<uint32_t, little, unaligned>(d);
+
   assert((End - d) % 4 == 0 &&
          "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -5615,6 +5619,11 @@
       }
       break;
 
+    case SUBMODULE_IMPORTED_HEADERS:
+      for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+        PendingImportedHeaders[&F].push_back(Record[Idx]);
+      }
+      break;
     case SUBMODULE_EXPORTS:
       for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
         UnresolvedModuleRef Unresolved;
@@ -9271,6 +9280,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
     getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
+
+  // Fix up the HeaderSearchInfo UIDs.
+  if (!PendingImportedHeaders.empty()) {
+    std::map<unsigned, unsigned> UIDToIndex;
+
+    // These HFIs were deserialized and assigned their "old"
+    // UID.
+    // We need to update them and populate the OldToIndex map
+    // for use next.
+    HeaderSearch &HS = PP.getHeaderSearchInfo();
+    for (unsigned Idx = 0; Idx < HS.FileInfo.size(); ++Idx) {
+      UIDToIndex[HS.FileInfo[Idx].UID] = Idx;
+    }
+
+    const auto &Iter = PendingImportedHeaders.begin();
+    for (unsigned I = 0; I < PendingImportedHeaders.size(); ++I) {
+      ModuleFile *ModFile = Iter[I].first;
+      auto &HeaderUIDs = Iter[I].second;
+      Module *M = HS.lookupModule(ModFile->ModuleName);
+
+      Preprocessor::SubmoduleState &SubState = PP.Submodules[M];
+      for (unsigned OldUID : HeaderUIDs) {
+        auto IdxIt = UIDToIndex.find(OldUID);
+        if (IdxIt == UIDToIndex.end()) {
+          continue;
+        }
+        SubState.IncludedFiles.insert(IdxIt->second);
+      }
+    }
+    PendingImportedHeaders.clear();
+  }
 }
 
 void ASTReader::diagnoseOdrViolations() {
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1165,7 +1165,8 @@
     FileInfo.resize(FE->getUID() + 1);
 
   HeaderFileInfo *HFI = &FileInfo[FE->getUID()];
-  // FIXME: Use a generation count to check whether this is really up to date.
+  HFI->UID = FE->getUID();
+
   if (ExternalSource && !HFI->Resolved) {
     HFI->Resolved = true;
     auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE);
@@ -1182,9 +1183,8 @@
   return *HFI;
 }
 
-const HeaderFileInfo *
-HeaderSearch::getExistingFileInfo(const FileEntry *FE,
-                                  bool WantExternal) const {
+HeaderFileInfo *HeaderSearch::getExistingFileInfo(const FileEntry *FE,
+                                                  bool WantExternal) const {
   // If we have an external source, ensure we have the latest information.
   // FIXME: Use a generation count to check whether this is really up to date.
   HeaderFileInfo *HFI;
@@ -1250,63 +1250,40 @@
                                           bool ModulesEnabled, Module *M) {
   ++NumIncluded; // Count # of attempted #includes.
 
+  ModMap.resolveHeaderDirectives(File);
+  const bool SeenBefore =
+      getExistingFileInfo(File, /*WantExernal=*/true) != nullptr;
+
   // Get information about this file.
   HeaderFileInfo &FileInfo = getFileInfo(File);
 
-  // FIXME: this is a workaround for the lack of proper modules-aware support
-  // for #import / #pragma once
-  auto TryEnterImported = [&]() -> bool {
-    if (!ModulesEnabled)
-      return false;
-    // Ensure FileInfo bits are up to date.
-    ModMap.resolveHeaderDirectives(File);
-    // Modules with builtins are special; multiple modules use builtins as
-    // modular headers, example:
-    //
-    //    module stddef { header "stddef.h" export * }
-    //
-    // After module map parsing, this expands to:
-    //
-    //    module stddef {
-    //      header "/path_to_builtin_dirs/stddef.h"
-    //      textual "stddef.h"
-    //    }
-    //
-    // It's common that libc++ and system modules will both define such
-    // submodules. Make sure cached results for a builtin header won't
-    // prevent other builtin modules to potentially enter the builtin header.
-    // Note that builtins are header guarded and the decision to actually
-    // enter them is postponed to the controlling macros logic below.
-    bool TryEnterHdr = false;
-    if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
-      TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
-                    ModuleMap::isBuiltinHeader(
-                        llvm::sys::path::filename(File->getName()));
-
-    // Textual headers can be #imported from different modules. Since ObjC
-    // headers find in the wild might rely only on #import and do not contain
-    // controlling macros, be conservative and only try to enter textual headers
-    // if such macro is present.
-    if (!FileInfo.isModuleHeader &&
-        FileInfo.getControllingMacro(ExternalLookup))
-      TryEnterHdr = true;
-    return TryEnterHdr;
-  };
-
-  // If this is a #import directive, check that we have not already imported
-  // this header.
   if (isImport) {
     // If this has already been imported, don't import it again.
     FileInfo.isImport = true;
+  }
 
-    // Has this already been #import'ed or #include'd?
-    if (FileInfo.NumIncludes && !TryEnterImported())
+  if (!SeenBefore || FileInfo.NumIncludes == 0 || FileInfo.isPragmaOnce ||
+      FileInfo.isImport) {
+    if (PP.isIncludeVisible(FileInfo.UID)) {
       return false;
-  } else {
-    // Otherwise, if this is a #include of a file that was previously #import'd
-    // or if this is the second #include of a #pragma once file, ignore it.
-    if (FileInfo.isImport && !TryEnterImported())
+    } else if (FileInfo.isModuleHeader && FileInfo.isCompilingModuleHeader &&
+               PP.isIncludeVisibleFromOuterModule(FileInfo.UID)) {
       return false;
+    } else {
+      // Mark as 'included'.
+      PP.setIncludeVisible(FileInfo.UID);
+
+      // FIXME: This is kind of hack to handle textual-header in ObjC
+      // Textual headers can be #imported from different modules.
+      // ObjC headers rely only on #import and do not contain
+      // controlling macros. Hence we won't enter the header if
+      // macro is absent.
+      if (isImport && FileInfo.NumIncludes &&
+          (FileInfo.isModuleHeader ||
+           !FileInfo.getControllingMacro(ExternalLookup))) {
+        return false;
+      }
+    }
   }
 
   // Next, check to see if the file is wrapped with #ifndef guards.  If so, and
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -463,7 +463,7 @@
   void WriteSourceManagerBlock(SourceManager &SourceMgr,
                                const Preprocessor &PP);
   void WritePreprocessor(const Preprocessor &PP, bool IsModule);
-  void WriteHeaderSearch(const HeaderSearch &HS);
+  void WriteHeaderSearch(HeaderSearch &HS);
   void WritePreprocessorDetail(PreprocessingRecord &PPRec);
   void WriteSubmodules(Module *WritingModule);
 
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -736,6 +736,12 @@
   /// IDs have not yet been deserialized to the global IDs of those macros.
   PendingMacroIDsMap PendingMacroIDs;
 
+  /// Mapping from ModuleFile to a list of its imported headers' (old) UID.
+  /// These UIDs are yet to be mapped to their corresponding HeaderFileInfo
+  using PendingImportedHeadersMap =
+      llvm::MapVector<ModuleFile *, SmallVector<unsigned, 8>>;
+  PendingImportedHeadersMap PendingImportedHeaders;
+
   using GlobalPreprocessedEntityMapType =
       ContinuousRangeMap<unsigned, ModuleFile *, 4>;
 
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -781,6 +781,9 @@
       /// Specifies the name of the module that will eventually
       /// re-export the entities in this module.
       SUBMODULE_EXPORT_AS = 17,
+
+      // Specifies the headers' UID imported by this submodule.
+      SUBMODULE_IMPORTED_HEADERS = 18,
     };
 
     /// Record types used within a comments block.
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -51,6 +51,7 @@
 #include <cstdint>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -128,6 +129,8 @@
 class Preprocessor {
   friend class VAOptDefinitionContext;
   friend class VariadicMacroScopeGuard;
+  friend class ASTWriter;
+  friend class ASTReader;
 
   llvm::unique_function<void(const clang::Token &)> OnToken;
   std::shared_ptr<PreprocessorOptions> PPOpts;
@@ -735,6 +738,7 @@
   };
   SmallVector<BuildingSubmoduleInfo, 8> BuildingSubmoduleStack;
 
+  //  struct HeaderFileInfo;
   /// Information about a submodule's preprocessor state.
   struct SubmoduleState {
     /// The macros for the submodule.
@@ -743,6 +747,9 @@
     /// The set of modules that are visible within the submodule.
     VisibleModuleSet VisibleModules;
 
+    /// The set of the included headers for the submodule.
+    std::set<unsigned> IncludedFiles;
+
     // FIXME: CounterValue?
     // FIXME: PragmaPushMacroInfo?
   };
@@ -1038,6 +1045,24 @@
     OnToken = std::move(F);
   }
 
+  void setIncludeVisible(unsigned UID) {
+    CurSubmoduleState->IncludedFiles.insert(UID);
+  }
+
+  bool isIncludeVisibleFromOuterModule(unsigned UID) {
+    if (BuildingSubmoduleStack.empty())
+      return false;
+
+    auto OuterState = BuildingSubmoduleStack.back().OuterSubmoduleState;
+    return OuterState->IncludedFiles.find(UID) !=
+           OuterState->IncludedFiles.end();
+  }
+
+  bool isIncludeVisible(unsigned UID) {
+    return CurSubmoduleState->IncludedFiles.find(UID) !=
+           CurSubmoduleState->IncludedFiles.end();
+  }
+
   bool isMacroDefined(StringRef Id) {
     return isMacroDefined(&Identifiers.get(Id));
   }
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -110,10 +110,13 @@
   /// of the framework.
   StringRef Framework;
 
+  /// The file UID used during [de]serialization.
+  unsigned UID = 0;
+
   HeaderFileInfo()
       : isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
         External(false), isModuleHeader(false), isCompilingModuleHeader(false),
-        Resolved(false), IndexHeaderMapHeader(false), IsValid(false)  {}
+        Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
 
   /// Retrieve the controlling macro for this header file, if
   /// any.
@@ -157,6 +160,8 @@
 /// by a \#include or \#include_next, (sub-)framework lookup, etc.
 class HeaderSearch {
   friend class DirectoryLookup;
+  friend class ASTReader;
+  friend class ASTWriter;
 
   /// Header-search options used to initialize this header search.
   std::shared_ptr<HeaderSearchOptions> HSOpts;
@@ -664,8 +669,8 @@
   /// if it has ever been filled in.
   /// \param WantExternal Whether the caller wants purely-external header file
   ///        info (where \p External is true).
-  const HeaderFileInfo *getExistingFileInfo(const FileEntry *FE,
-                                            bool WantExternal = true) const;
+  HeaderFileInfo *getExistingFileInfo(const FileEntry *FE,
+                                      bool WantExternal = true) const;
 
   // Used by external tools
   using search_dir_iterator = std::vector<DirectoryLookup>::const_iterator;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to