jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai, rsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When building a module consisting of submodules, the preprocessor keeps a 
"global" state that for each header file (`HeaderFileInfo`) tracks (amongst 
other things) the number of times it was included/imported. This information is 
serialized into the PCM file.

When importing such module (either the top-level module or any of its 
submodules), this number is merged into the state of the importing preprocessor.

This can incorrectly prevent imports of headers. Consider that submodule `M.A` 
imports header `H`. If a TU imports submodule `M.B` (that didn't import header 
`H`), subsequent import of header `H` in the current TU will be skipped, since 
the preprocessor thinks it already imported that header. However, submodule 
`M.B` doesn't contain the AST of header `H`, which leads to compilation errors.

This patch fixes this bug by making the number of times a header was included 
more granular and tracks it on per-submodule basis.

(This patch is an alternative approach to the same issue addressed in 
https://reviews.llvm.org/D104344.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112915

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/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Headers/A.h
  
clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Modules/module.modulemap
  clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Headers/B1.h
  clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Headers/B2.h
  
clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Modules/module.modulemap
  clang/test/Modules/Inputs/import-submodule-visibility/C/C.h
  clang/test/Modules/Inputs/import-submodule-visibility/C/module.modulemap
  clang/test/Modules/Inputs/import-submodule-visibility/D/D1.h
  clang/test/Modules/Inputs/import-submodule-visibility/D/D2.h
  clang/test/Modules/Inputs/import-submodule-visibility/D/module.modulemap
  clang/test/Modules/Inputs/import-submodule-visibility/Textual.h
  clang/test/Modules/import-submodule-visibility.c

Index: clang/test/Modules/import-submodule-visibility.c
===================================================================
--- /dev/null
+++ clang/test/Modules/import-submodule-visibility.c
@@ -0,0 +1,58 @@
+// RUN: rm -rf %t && mkdir -p %t
+
+// This test checks that imports of headers that appeared in a different submodule than
+// what is imported by the current TU don't affect the compilation.
+
+// Framework "A" includes "Textual.h" in the top-level module.
+// This test checks that simply specifying the PCM file on the command line (without actually importing "A") does not
+// prevent "Textual.h" to be included in the TU.
+//
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility \
+// RUN:   -emit-module %S/Inputs/import-submodule-visibility/A.framework/Modules/module.modulemap -fmodule-name=A -o %t/A.pcm
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility -fsyntax-only %s -DA \
+// RUN:   -fmodule-file=%t/A.pcm -fmodule-map-file=%S/Inputs/import-submodule-visibility/A.framework/Modules/module.modulemap
+
+// Framework "B" includes "Textual.h" in the "B1" submodule and its "B2" submodule does not include "Textual.h" at all.
+// This test checks that specifying the PCM file on the command line and importing "B2" in the source does not
+// prevent "Textual.h" to be included in the TU.
+//
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility \
+// RUN:   -emit-module %S/Inputs/import-submodule-visibility/B.framework/Modules/module.modulemap -fmodule-name=B -o %t/B.pcm
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility -fsyntax-only %s -DB -iframework %S/Inputs/import-submodule-visibility \
+// RUN:   -fmodule-file=%t/B.pcm -fmodule-map-file=%S/Inputs/import-submodule-visibility/B.framework/Modules/module.modulemap
+
+// Module "C" includes "Textual.h" in the top level module.
+// This is a module-only version of the test with framework A.
+//
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility \
+// RUN:   -emit-module %S/Inputs/import-submodule-visibility/C/module.modulemap -fmodule-name=C -o %t/C.pcm
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility -fsyntax-only %s -DC \
+// RUN:   -fmodule-file=%t/C.pcm -fmodule-map-file=%S/Inputs/import-submodule-visibility/C/module.modulemap
+
+// Module "D" includes "Textual.h" in the "D1" submodule and its "D2" submodules does not include "Textual.h" at all.
+// This is a module-only version of the test with framework B.
+//
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility \
+// RUN:   -emit-module %S/Inputs/import-submodule-visibility/D/module.modulemap -fmodule-name=D -o %t/D.pcm
+// RUN: %clang_cc1 -fmodules -I %S/Inputs/import-submodule-visibility -fsyntax-only %s -DD \
+// RUN:   -fmodule-file=%t/D.pcm -fmodule-map-file=%S/Inputs/import-submodule-visibility/D/module.modulemap
+
+#ifdef A
+//
+#endif
+
+#ifdef B
+#import <B/B2.h>
+#endif
+
+#ifdef C
+//
+#endif
+
+#ifdef D
+#import "D/D2.h"
+#endif
+
+#import "Textual.h"
+
+static int x = MACRO_TEXTUAL;
Index: clang/test/Modules/Inputs/import-submodule-visibility/Textual.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/Textual.h
@@ -0,0 +1 @@
+#define MACRO_TEXTUAL 1
Index: clang/test/Modules/Inputs/import-submodule-visibility/D/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/D/module.modulemap
@@ -0,0 +1,4 @@
+module D {
+  module D1 { header "D1.h" }
+  module D2 { header "D2.h" }
+}
Index: clang/test/Modules/Inputs/import-submodule-visibility/D/D1.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/D/D1.h
@@ -0,0 +1 @@
+#include "Textual.h"
Index: clang/test/Modules/Inputs/import-submodule-visibility/C/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/C/module.modulemap
@@ -0,0 +1 @@
+module C { header "C.h" }
Index: clang/test/Modules/Inputs/import-submodule-visibility/C/C.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/C/C.h
@@ -0,0 +1 @@
+#include "Textual.h"
Index: clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module B {
+  module B1 { header "B1.h" }
+  module B2 { header "B2.h" }
+}
Index: clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Headers/B1.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/B.framework/Headers/B1.h
@@ -0,0 +1 @@
+#include "Textual.h"
Index: clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Modules/module.modulemap
@@ -0,0 +1,3 @@
+framework module A {
+  header "A.h"
+}
Index: clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Headers/A.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-submodule-visibility/A.framework/Headers/A.h
@@ -0,0 +1 @@
+#include "Textual.h"
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -116,6 +116,8 @@
 #include <utility>
 #include <vector>
 
+#include <map>
+
 using namespace clang;
 using namespace clang::serialization;
 
@@ -1696,7 +1698,7 @@
     std::pair<unsigned, unsigned>
     EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
       unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
-      unsigned DataLen = 1 + 2 + 4 + 4;
+      unsigned DataLen = 1 + 4 + 4;
       for (auto ModInfo : Data.KnownHeaders)
         if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
           DataLen += 4;
@@ -1728,7 +1730,6 @@
                           | (Data.HFI.DirInfo << 1)
                           | Data.HFI.IndexHeaderMapHeader;
       LE.write<uint8_t>(Flags);
-      LE.write<uint16_t>(Data.HFI.NumIncludes);
 
       if (!Data.HFI.ControllingMacro)
         LE.write<uint32_t>(Data.HFI.ControllingMacroID);
@@ -2379,6 +2380,47 @@
                                        MacroOffsetsBase - ASTBlockStartOffset};
     Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
+
+  Abbrev = std::make_shared<BitCodeAbbrev>();
+  Abbrev->Add(BitCodeAbbrevOp(INCLUDES));
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # submodule ID
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # of includes
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+
+  unsigned IncludesAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
+
+  // TODO: Find a more efficient way to stabilize the order here.
+  std::map<unsigned, std::map<StringRef, unsigned>> ToBeSerialized;
+
+  auto CollectModuleIncludes = [&](Module *M, const auto &Includes){
+    unsigned LocalSMID = getLocalOrImportedSubmoduleID(M);
+
+    // TODO: If we make this more granular ("ImportedIncludes"), serialize them
+    //  too.
+    // Only serializing local includes. We don't want includes made by other
+    // submodules to propagate into consumers of this submodule.
+    for (const auto &Inc : Includes.LocalIncludes) {
+      unsigned NumIncludes = Inc.getSecond();
+      StringRef Filename = Inc.getFirst()->getName();
+      ToBeSerialized[LocalSMID].insert({Filename, NumIncludes});
+    }
+  };
+
+  CollectModuleIncludes(const_cast<Preprocessor &>(PP).getCurrentModule(),
+                        PP.getRootIncludes());
+  for (const auto &SubM : PP.getSubmoduleIncludes())
+    CollectModuleIncludes(SubM.first, SubM.second);
+
+  for (const auto& Include : ToBeSerialized) {
+    unsigned LocalSMID = Include.first;
+
+    for (const auto& X : Include.second) {
+      StringRef Filename = X.first;
+      unsigned NumIncludes = X.second;
+      RecordData::value_type Record[] = {INCLUDES, LocalSMID, NumIncludes};
+      Stream.EmitRecordWithBlob(IncludesAbbrev, Record, Filename);
+    }
+  }
 }
 
 void ASTWriter::WritePreprocessorDetail(PreprocessingRecord &PPRec,
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1888,10 +1888,6 @@
   HFI.isPragmaOnce |= (Flags >> 4) & 0x01;
   HFI.DirInfo = (Flags >> 1) & 0x07;
   HFI.IndexHeaderMapHeader = Flags & 0x01;
-  // FIXME: Find a better way to handle this. Maybe just store a
-  // "has been included" flag?
-  HFI.NumIncludes = std::max(endian::readNext<uint16_t, little, unaligned>(d),
-                             HFI.NumIncludes);
   HFI.ControllingMacroID = Reader.getGlobalIdentifierID(
       M, endian::readNext<uint32_t, little, unaligned>(d));
   if (unsigned FrameworkOffset =
@@ -3701,6 +3697,18 @@
       break;
     }
 
+    case INCLUDES: {
+      unsigned LocalSMID = Record[0];
+      unsigned NumIncludes = Record[1];
+      StringRef Filename = Blob;
+      // PCHs don't have submodules. We can add the includes right away.
+      if (PP.getPreprocessorOpts().ImplicitPCHInclude == F.FileName)
+        PP.addSubmoduleInclude(nullptr, Filename, NumIncludes);
+      else
+        SubmoduleImports[LocalSMID][Filename] = NumIncludes;
+      break;
+    }
+
     case LATE_PARSED_TEMPLATE:
       LateParsedTemplates.emplace_back(
           std::piecewise_construct, std::forward_as_tuple(&F),
@@ -5569,6 +5577,19 @@
       CurrentModule->IsUnimportable =
           ParentModule && ParentModule->IsUnimportable;
       CurrentModule->IsAvailable = !CurrentModule->IsUnimportable;
+
+      // Now that we can map the local submodule ID to an actual Module
+      // instance, pass the include information to the preprocessor.
+      for (const auto &Submodule : SubmoduleImports) {
+        unsigned LocalID = Submodule.getFirst();
+        if (GlobalID == getGlobalSubmoduleID(F, LocalID)) {
+          for (const auto &Include : Submodule.getSecond())
+            PP.addSubmoduleInclude(CurrentModule, Include.getKey(),
+                                   Include.getValue());
+          break;
+        }
+      }
+
       break;
     }
 
Index: clang/lib/Lex/Preprocessor.cpp
===================================================================
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -91,7 +91,7 @@
       // deferred to Preprocessor::Initialize().
       Identifiers(IILookup), PragmaHandlers(new PragmaNamespace(StringRef())),
       TUKind(TUKind), SkipMainFilePreamble(0, true),
-      CurSubmoduleState(&NullSubmoduleState) {
+      CurSubmoduleState(&NullSubmoduleState), CurIncludeMap(&RootIncludeMap) {
   OwnsHeaderSearch = OwnsHeaders;
 
   // Default to discarding comments.
@@ -549,7 +549,7 @@
     // Tell the header info that the main file was entered.  If the file is later
     // #imported, it won't be re-entered.
     if (const FileEntry *FE = SourceMgr.getFileEntryForID(MainFileID))
-      HeaderInfo.IncrementIncludeCount(FE);
+      IncrementIncludeCount(FE);
   }
 
   // Preprocess Predefines to populate the initial preprocessor state.
@@ -1466,3 +1466,27 @@
   Record = new PreprocessingRecord(getSourceManager());
   addPPCallbacks(std::unique_ptr<PPCallbacks>(Record));
 }
+
+bool Preprocessor::FirstTimeLexingFile(const FileEntry *File) const {
+  return NumIncludes(File) == 1;
+}
+
+unsigned Preprocessor::NumIncludes(const FileEntry *File) const {
+  return (CurIncludeMap->LocalIncludes[File] +
+          CurIncludeMap->OuterIncludes[File]);
+}
+
+void Preprocessor::IncrementIncludeCount(const FileEntry *File) {
+  CurIncludeMap->LocalIncludes[File] += 1;
+}
+
+void Preprocessor::addSubmoduleInclude(Module *M, StringRef Filename,
+                                       unsigned NumIncludes) {
+  if (auto MaybeFE = FileMgr.getFile(Filename)) {
+    if (M == nullptr) {
+      CurIncludeMap->OuterIncludes[*MaybeFE] += NumIncludes;
+    } else {
+      SubmoduleIncludeMaps[M].LocalIncludes[*MaybeFE] += NumIncludes;
+    }
+  }
+}
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -376,8 +376,7 @@
         if (const IdentifierInfo *DefinedMacro =
               CurPPLexer->MIOpt.GetDefinedMacro()) {
           if (!isMacroDefined(ControllingMacro) &&
-              DefinedMacro != ControllingMacro &&
-              HeaderInfo.FirstTimeLexingFile(FE)) {
+              DefinedMacro != ControllingMacro && FirstTimeLexingFile(FE)) {
 
             // If the edit distance between the two macros is more than 50%,
             // DefinedMacro may not be header guard, or can be header guard of
@@ -690,7 +689,20 @@
     // Just track that we entered this submodule.
     BuildingSubmoduleStack.push_back(
         BuildingSubmoduleInfo(M, ImportLoc, ForPragma, CurSubmoduleState,
-                              PendingModuleMacroNames.size()));
+                              PendingModuleMacroNames.size(), CurIncludeMap));
+
+    auto IncludeStateIt = SubmoduleIncludeMaps.emplace(M, IncludeMap());
+    auto &IncludeState = IncludeStateIt.first->second;
+
+    // Propagate all include information from the outer include state into the
+    // current.
+    for (const auto& Includes : CurIncludeMap->OuterIncludes)
+      IncludeState.OuterIncludes[Includes.getFirst()] += Includes.getSecond();
+    for (const auto& Includes : CurIncludeMap->LocalIncludes)
+      IncludeState.OuterIncludes[Includes.getFirst()] += Includes.getSecond();
+
+    CurIncludeMap = &IncludeState;
+
     if (Callbacks)
       Callbacks->EnteredSubmodule(M, ImportLoc, ForPragma);
     return;
@@ -707,7 +719,9 @@
 
   // If this is the first time we've entered this module, set up its state.
   auto R = Submodules.insert(std::make_pair(M, SubmoduleState()));
+  auto IncludeIt = SubmoduleIncludeMaps.emplace(M, IncludeMap());
   auto &State = R.first->second;
+  auto &IncludeState = IncludeIt.first->second;
   bool FirstTime = R.second;
   if (FirstTime) {
     // Determine the set of starting macros for this submodule; take these
@@ -735,7 +749,7 @@
   // Track that we entered this module.
   BuildingSubmoduleStack.push_back(
       BuildingSubmoduleInfo(M, ImportLoc, ForPragma, CurSubmoduleState,
-                            PendingModuleMacroNames.size()));
+                            PendingModuleMacroNames.size(), CurIncludeMap));
 
   if (Callbacks)
     Callbacks->EnteredSubmodule(M, ImportLoc, ForPragma);
@@ -743,6 +757,15 @@
   // Switch to this submodule as the current submodule.
   CurSubmoduleState = &State;
 
+  // Propagate all include information from the outer include state into the
+  // current.
+  for (const auto& Includes : CurIncludeMap->OuterIncludes)
+    IncludeState.OuterIncludes[Includes.getFirst()] += Includes.getSecond();
+  for (const auto& Includes : CurIncludeMap->LocalIncludes)
+    IncludeState.OuterIncludes[Includes.getFirst()] += Includes.getSecond();
+
+  CurIncludeMap = &IncludeState;
+
   // This module is visible to itself.
   if (FirstTime)
     makeModuleVisible(M, ImportLoc);
@@ -773,6 +796,13 @@
   Module *LeavingMod = Info.M;
   SourceLocation ImportLoc = Info.ImportLoc;
 
+  // Propagate local include information from the inner submodule include
+  // state into the outer.
+  for (const auto &Includes : CurIncludeMap->LocalIncludes)
+    Info.OuterSubmoduleIncludes->OuterIncludes[Includes.getFirst()] +=
+        Includes.getSecond();
+  CurIncludeMap = Info.OuterSubmoduleIncludes;
+
   if (!needModuleMacros() ||
       (!getLangOpts().ModulesLocalVisibility &&
        LeavingMod->getTopLevelModuleName() != getLangOpts().CurrentModule)) {
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2055,7 +2055,7 @@
   // include cycle. Don't enter already processed files again as it can lead to
   // reaching the max allowed include depth again.
   if (Action == Enter && HasReachedMaxIncludeDepth && File &&
-      HeaderInfo.getFileInfo(&File->getFileEntry()).NumIncludes)
+      NumIncludes(*File))
     Action = IncludeLimitReached;
 
   // Determine whether we should try to import the module for this #include, if
@@ -2309,6 +2309,10 @@
 
     makeModuleVisible(M, EndLoc);
 
+    // TODO: Consider storing this into "ImportedIncludes" or something.
+    for (const auto& Include : SubmoduleIncludeMaps[M].LocalIncludes)
+      CurIncludeMap->LocalIncludes[Include.getFirst()] += Include.getSecond();
+
     if (IncludeTok.getIdentifierInfo()->getPPKeywordID() ==
         tok::pp___include_macros)
       return {ImportAction::None};
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -89,16 +89,16 @@
 void HeaderSearch::PrintStats() {
   llvm::errs() << "\n*** HeaderSearch Stats:\n"
                << FileInfo.size() << " files tracked.\n";
-  unsigned NumOnceOnlyFiles = 0, MaxNumIncludes = 0, NumSingleIncludedFiles = 0;
-  for (unsigned i = 0, e = FileInfo.size(); i != e; ++i) {
-    NumOnceOnlyFiles += (FileInfo[i].isPragmaOnce || FileInfo[i].isImport);
-    if (MaxNumIncludes < FileInfo[i].NumIncludes)
-      MaxNumIncludes = FileInfo[i].NumIncludes;
-    NumSingleIncludedFiles += FileInfo[i].NumIncludes == 1;
-  }
-  llvm::errs() << "  " << NumOnceOnlyFiles << " #import/#pragma once files.\n"
-               << "  " << NumSingleIncludedFiles << " included exactly once.\n"
-               << "  " << MaxNumIncludes << " max times a file is included.\n";
+//  unsigned NumOnceOnlyFiles = 0, MaxNumIncludes = 0, NumSingleIncludedFiles = 0;
+//  for (unsigned i = 0, e = FileInfo.size(); i != e; ++i) {
+//    NumOnceOnlyFiles += (FileInfo[i].isPragmaOnce || FileInfo[i].isImport);
+//    if (MaxNumIncludes < FileInfo[i].NumIncludes)
+//      MaxNumIncludes = FileInfo[i].NumIncludes;
+//    NumSingleIncludedFiles += FileInfo[i].NumIncludes == 1;
+//  }
+//  llvm::errs() << "  " << NumOnceOnlyFiles << " #import/#pragma once files.\n"
+//               << "  " << NumSingleIncludedFiles << " included exactly once.\n"
+//               << "  " << MaxNumIncludes << " max times a file is included.\n";
 
   llvm::errs() << "  " << NumIncluded << " #include/#include_next/#import.\n"
                << "    " << NumMultiIncludeFileOptzn
@@ -1203,7 +1203,6 @@
   HFI.isImport |= OtherHFI.isImport;
   HFI.isPragmaOnce |= OtherHFI.isPragmaOnce;
   HFI.isModuleHeader |= OtherHFI.isModuleHeader;
-  HFI.NumIncludes += OtherHFI.NumIncludes;
 
   if (!HFI.ControllingMacro && !HFI.ControllingMacroID) {
     HFI.ControllingMacro = OtherHFI.ControllingMacro;
@@ -1361,7 +1360,7 @@
     FileInfo.isImport = true;
 
     // Has this already been #import'ed or #include'd?
-    if (FileInfo.NumIncludes && !TryEnterImported())
+    if (PP.NumIncludes(File) && !TryEnterImported())
       return false;
   } else {
     // Otherwise, if this is a #include of a file that was previously #import'd
@@ -1385,7 +1384,7 @@
   }
 
   // Increment the number of times this file has been included.
-  ++FileInfo.NumIncludes;
+  PP.IncrementIncludeCount(File);
 
   return true;
 }
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -855,6 +855,10 @@
   /// The pragma clang optimize location (if the pragma state is "off").
   SourceLocation OptimizeOffPragmaLocation;
 
+  /// The submodule include maps. Translates local submodule ID to the map that
+  /// contains the number of includes for a file.
+  llvm::DenseMap<unsigned, llvm::StringMap<unsigned>> SubmoduleImports;
+
   /// The PragmaMSStructKind pragma ms_struct state if set, or -1.
   int PragmaMSStructState = -1;
 
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -695,6 +695,9 @@
 
   /// Record code for \#pragma float_control options.
   FLOAT_CONTROL_PRAGMA_OPTIONS = 65,
+
+  /// Record code for submodule includes.
+  INCLUDES = 66,
 };
 
 /// Record types used within a source manager block.
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -717,6 +717,14 @@
   /// This mapping lives within the \p CurSubmoduleState.
   using MacroMap = llvm::DenseMap<const IdentifierInfo *, MacroState>;
 
+  /// For each included header file, we keep a mapping to the number of times it
+  /// was included.
+  struct IncludeMap {
+    // TODO: Store this in a vector indexed by the FileEntry UID.
+    llvm::DenseMap<const FileEntry *, unsigned> LocalIncludes;
+    llvm::DenseMap<const FileEntry *, unsigned> OuterIncludes;
+  };
+
   struct SubmoduleState;
 
   /// Information about a submodule that we're currently building.
@@ -736,12 +744,17 @@
     /// The number of pending module macro names when we started building this.
     unsigned OuterPendingModuleMacroNames;
 
+    /// The previous IncludeMap.
+    IncludeMap *OuterSubmoduleIncludes;
+
     BuildingSubmoduleInfo(Module *M, SourceLocation ImportLoc, bool IsPragma,
                           SubmoduleState *OuterSubmoduleState,
-                          unsigned OuterPendingModuleMacroNames)
+                          unsigned OuterPendingModuleMacroNames,
+                          IncludeMap *OuterSubmoduleIncludes)
         : M(M), ImportLoc(ImportLoc), IsPragma(IsPragma),
           OuterSubmoduleState(OuterSubmoduleState),
-          OuterPendingModuleMacroNames(OuterPendingModuleMacroNames) {}
+          OuterPendingModuleMacroNames(OuterPendingModuleMacroNames),
+          OuterSubmoduleIncludes(OuterSubmoduleIncludes) {}
   };
   SmallVector<BuildingSubmoduleInfo, 8> BuildingSubmoduleStack;
 
@@ -765,6 +778,16 @@
   /// in a submodule.
   SubmoduleState *CurSubmoduleState;
 
+  /// Include maps of each entered submodule.
+  std::map<Module *, IncludeMap> SubmoduleIncludeMaps;
+
+  /// The root include map.
+  IncludeMap RootIncludeMap;
+
+  /// The current include map. Will be \p RootIncludeMap if we're not in a
+  /// submodule.
+  IncludeMap *CurIncludeMap;
+
   /// The set of known macros exported from modules.
   llvm::FoldingSet<ModuleMacro> ModuleMacros;
 
@@ -1224,6 +1247,27 @@
 
   /// \}
 
+  /// Return true if this is the first time encountering this header.
+  bool FirstTimeLexingFile(const FileEntry *File) const;
+
+  /// Return the number of times the given file has been entered.
+  unsigned NumIncludes(const FileEntry *File) const;
+
+  /// Increment the count for the number of times the specified
+  /// FileEntry has been entered.
+  void IncrementIncludeCount(const FileEntry *File);
+
+  /// Get the submodule include information.
+  const std::map<Module *, IncludeMap> &getSubmoduleIncludes() const {
+    return SubmoduleIncludeMaps;
+  }
+
+  /// Get the root include information.
+  const IncludeMap &getRootIncludes() const { return RootIncludeMap; }
+
+  /// Add a submodule include.
+  void addSubmoduleInclude(Module *M, StringRef Filename, unsigned NumIncludes);
+
   /// Return the name of the macro defined before \p Loc that has
   /// spelling \p Tokens.  If there are multiple macros with same spelling,
   /// return the last one defined.
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -89,9 +89,6 @@
   /// Whether this file has been looked up as a header.
   unsigned IsValid : 1;
 
-  /// The number of times the file has been included already.
-  unsigned short NumIncludes = 0;
-
   /// The ID number of the controlling macro.
   ///
   /// This ID number will be non-zero when there is a controlling
@@ -130,13 +127,6 @@
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-    return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-      ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply
@@ -481,12 +471,6 @@
                             ModuleMap::ModuleHeaderRole Role,
                             bool isCompilingModuleHeader);
 
-  /// Increment the count for the number of times the specified
-  /// FileEntry has been entered.
-  void IncrementIncludeCount(const FileEntry *File) {
-    ++getFileInfo(File).NumIncludes;
-  }
-
   /// Mark the specified file as having a controlling macro.
   ///
   /// This is used by the multiple-include optimization to eliminate
@@ -496,11 +480,6 @@
     getFileInfo(File).ControllingMacro = ControllingMacro;
   }
 
-  /// Return true if this is the first time encountering this header.
-  bool FirstTimeLexingFile(const FileEntry *File) {
-    return getFileInfo(File).NumIncludes == 1;
-  }
-
   /// Determine whether this file is intended to be safe from
   /// multiple inclusions, e.g., it has \#pragma once or a controlling
   /// macro.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to