[modules] When checking the include guard for a header, check whether it's

visible in the module we're considering entering. Previously we assumed that if
we knew the include guard for a modular header, we'd already parsed it, but
that need not be the case if a header is present in the current module and one
of its dependencies; the result of getting this wrong was that the current
module's submodule for the header would end up empty.

llvm-svn: 241953

8 files changed, 41 insertions, 8 deletions

diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index bba0c38cec7..b2f58ead0e7 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -786,6 +786,22 @@ public:
            (!getLangOpts().Modules || (bool)getMacroDefinition(II));
   }
 
+  /// \brief Determine whether II is defined as a macro within the module M,
+  /// if that is a module that we've already preprocessed. Does not check for
+  /// macros imported into M.
+  bool isMacroDefinedInLocalModule(const IdentifierInfo *II, Module *M) {
+    if (!II->hasMacroDefinition())
+      return false;
+    auto I = Submodules.find(M);
+    if (I == Submodules.end())
+      return false;
+    auto J = I->second.Macros.find(II);
+    if (J == I->second.Macros.end())
+      return false;
+    auto *MD = J->second.getLatest();
+    return MD && MD->isDefined();
+  }
+
   MacroDefinition getMacroDefinition(const IdentifierInfo *II) {
     if (!II->hasMacroDefinition())
       return MacroDefinition();
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 6c5c64bd266..b805990eecb 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -995,7 +995,8 @@ HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) {
   return HFI;
 }
 
-bool HeaderSearch::tryGetFileInfo(const FileEntry *FE, HeaderFileInfo &Result) const {
+bool HeaderSearch::tryGetFileInfo(const FileEntry *FE,
+                                  HeaderFileInfo &Result) const {
   if (FE->getUID() >= FileInfo.size())
     return false;
   const HeaderFileInfo &HFI = FileInfo[FE->getUID()];
@@ -1028,7 +1029,7 @@ void HeaderSearch::MarkFileModuleHeader(const FileEntry *FE,
 
   HeaderFileInfo &HFI = FileInfo[FE->getUID()];
   HFI.isModuleHeader = true;
-  HFI.isCompilingModuleHeader = isCompilingModuleHeader;
+  HFI.isCompilingModuleHeader |= isCompilingModuleHeader;
   HFI.setHeaderRole(Role);
 }
 
@@ -1058,15 +1059,16 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
   // Next, check to see if the file is wrapped with #ifndef guards.  If so, and
   // if the macro that guards it is defined, we know the #include has no effect.
   if (const IdentifierInfo *ControllingMacro
-      = FileInfo.getControllingMacro(ExternalLookup))
-    // If the include file is part of a module, and we already know what its
-    // controlling macro is, then we've already parsed it and can safely just
-    // make it visible. This saves us needing to switch into the visibility
-    // state of the module just to check whether the macro is defined within it.
-    if (M || PP.isMacroDefined(ControllingMacro)) {
+      = FileInfo.getControllingMacro(ExternalLookup)) {
+    // If the header corresponds to a module, check whether the macro is already
+    // defined in that module rather than checking in the current set of visible
+    // modules.
+    if (M ? PP.isMacroDefinedInLocalModule(ControllingMacro, M)
+          : PP.isMacroDefined(ControllingMacro)) {
       ++NumMultiIncludeFileOptzn;
       return false;
     }
+  }
 
   // Increment the number of times this file has been included.
   ++FileInfo.NumIncludes;
diff --git a/clang/test/Modules/Inputs/multiple-include/a.h b/clang/test/Modules/Inputs/multiple-include/a.h
new file mode 100644
index 00000000000..82661908588
--- /dev/null
+++ b/clang/test/Modules/Inputs/multiple-include/a.h
@@ -0,0 +1 @@
+#include "x.h"
diff --git a/clang/test/Modules/Inputs/multiple-include/b.h b/clang/test/Modules/Inputs/multiple-include/b.h
new file mode 100644
index 00000000000..f56ab64da6b
--- /dev/null
+++ b/clang/test/Modules/Inputs/multiple-include/b.h
@@ -0,0 +1,3 @@
+#pragma clang __debug macro C_H
+#include "c.h"
+inline int get() { return c; }
diff --git a/clang/test/Modules/Inputs/multiple-include/c.h b/clang/test/Modules/Inputs/multiple-include/c.h
new file mode 100644
index 00000000000..4e7d4b742dc
--- /dev/null
+++ b/clang/test/Modules/Inputs/multiple-include/c.h
@@ -0,0 +1,4 @@
+#ifndef C_H
+#define C_H
+extern int c;
+#endif
diff --git a/clang/test/Modules/Inputs/multiple-include/module.modulemap b/clang/test/Modules/Inputs/multiple-include/module.modulemap
new file mode 100644
index 00000000000..1228ae6cbb7
--- /dev/null
+++ b/clang/test/Modules/Inputs/multiple-include/module.modulemap
@@ -0,0 +1,2 @@
+module A { module a { header "a.h" } module b { header "b.h" } module c { header "c.h" } }
+module X { module x { header "x.h" } module c { header "c.h" } }
diff --git a/clang/test/Modules/Inputs/multiple-include/x.h b/clang/test/Modules/Inputs/multiple-include/x.h
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/clang/test/Modules/Inputs/multiple-include/x.h
diff --git a/clang/test/Modules/multiple-include.cpp b/clang/test/Modules/multiple-include.cpp
new file mode 100644
index 00000000000..7cbeefc8178
--- /dev/null
+++ b/clang/test/Modules/multiple-include.cpp
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -I%S/Inputs/multiple-include -fmodules-cache-path=%t -fimplicit-module-maps -verify %s -fmodules-local-submodule-visibility
+// expected-no-diagnostics
+#include "b.h"
+int c = get();