[modules] If a module #includes a modular header that #undef's its macro, it

should not export the macro.

... at least, not unless we have local submodule visibility enabled.

llvm-svn: 236369

8 files changed, 30 insertions, 11 deletions

diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 2ce880ee7b3..6414c94a46a 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -205,7 +205,7 @@ public:
 
   /// \brief The set of modules imported by this module, and on which this
   /// module depends.
-  SmallVector<Module *, 2> Imports;
+  llvm::SmallSetVector<Module *, 2> Imports;
   
   /// \brief Describes an exported module.
   ///
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index c506aa7f6f0..d4defdd71a7 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -620,8 +620,7 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) {
 
   // Determine the set of starting macros for this submodule.
   // FIXME: If we re-enter a submodule, should we restore its MacroDirectives?
-  auto &StartingMacros = (getLangOpts().ModulesLocalVisibility &&
-                          BuildingSubmoduleStack.size() > 1)
+  auto &StartingMacros = getLangOpts().ModulesLocalVisibility
                              ? BuildingSubmoduleStack[0].Macros
                              : Info.Macros;
 
@@ -661,13 +660,13 @@ void Preprocessor::LeaveSubmodule() {
          MD = MD->getPrevious()) {
       assert(MD && "broken macro directive chain");
 
-      // Skip macros defined in other submodules we #included along the way.
+      // Stop on macros defined in other submodules we #included along the way.
       // There's no point doing this if we're tracking local submodule
-      // visibiltiy, since there can be no such directives in our list.
+      // visibility, since there can be no such directives in our list.
       if (!getLangOpts().ModulesLocalVisibility) {
         Module *Mod = getModuleContainingLocation(MD->getLocation());
         if (Mod != Info.M)
-          continue;
+          break;
       }
 
       if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 2db109565af..1251e982533 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -763,9 +763,6 @@ void Preprocessor::LexAfterModuleImport(Token &Result) {
 }
 
 void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
-  if (VisibleModules.isVisible(M))
-    return;
-
   VisibleModules.setVisible(
       M, Loc, [](Module *) {},
       [&](ArrayRef<Module *> Path, Module *Conflict, StringRef Message) {
@@ -779,7 +776,7 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
 
   // Add this module to the imports list of the currently-built submodule.
   if (!BuildingSubmoduleStack.empty())
-    BuildingSubmoduleStack.back().M->Imports.push_back(M);
+    BuildingSubmoduleStack.back().M->Imports.insert(M);
 }
 
 bool Preprocessor::FinishLexStringLiteral(Token &Result, std::string &String,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index d67d79b52df..150306176eb 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3442,7 +3442,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
 
     case UnresolvedModuleRef::Import:
       if (ResolvedMod)
-        Unresolved.Mod->Imports.push_back(ResolvedMod);
+        Unresolved.Mod->Imports.insert(ResolvedMod);
       continue;
 
     case UnresolvedModuleRef::Export:
diff --git a/clang/test/Modules/Inputs/macro-masking/a.h b/clang/test/Modules/Inputs/macro-masking/a.h
new file mode 100644
index 00000000000..dbc17b8cbcf
--- /dev/null
+++ b/clang/test/Modules/Inputs/macro-masking/a.h
@@ -0,0 +1,2 @@
+#define MACRO
+#include "b.h"
diff --git a/clang/test/Modules/Inputs/macro-masking/b.h b/clang/test/Modules/Inputs/macro-masking/b.h
new file mode 100644
index 00000000000..0d14daf13af
--- /dev/null
+++ b/clang/test/Modules/Inputs/macro-masking/b.h
@@ -0,0 +1 @@
+#undef MACRO
diff --git a/clang/test/Modules/Inputs/macro-masking/module.modulemap b/clang/test/Modules/Inputs/macro-masking/module.modulemap
new file mode 100644
index 00000000000..63e1014dda8
--- /dev/null
+++ b/clang/test/Modules/Inputs/macro-masking/module.modulemap
@@ -0,0 +1,4 @@
+module X {
+  module A { header "a.h" export * }
+  module B { header "b.h" export * }
+}
diff --git a/clang/test/Modules/macro-masking.cpp b/clang/test/Modules/macro-masking.cpp
new file mode 100644
index 00000000000..3d4e8e0ba49
--- /dev/null
+++ b/clang/test/Modules/macro-masking.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules %s -fmodules-cache-path=%t -verify -I%S/Inputs/macro-masking
+// RxN: %clang_cc1 -fsyntax-only -fmodules -fmodules-local-submodule-visibility %s -fmodules-cache-path=%t -verify -I%S/Inputs/macro-masking -DLOCAL_VISIBILITY
+// expected-no-diagnostics
+
+#include "a.h"
+
+#ifdef LOCAL_VISIBILITY
+# ifndef MACRO
+#  error should still be defined, undef does not override define
+# endif
+#else
+# ifdef MACRO
+#  error should have been undefined!
+# endif
+#endif