[modules] Add -fmodules-local-submodule-visibility flag.

This flag specifies that the normal visibility rules should be used even for
local submodules (submodules of the currently-being-built module). Thus names
will only be visible if a header / module that declares them has actually been
included / imported, and not merely because a submodule that happened to be
built earlier declared those names. This also removes the need to modularize
bottom-up: textually-included headers will be included into every submodule
that includes them, since their include guards will not leak between modules.

So far, this only governs visibility of macros, not of declarations, so is not
ready for real use yet.

llvm-svn: 236350

Author: zygoloid

clang/include/clang/Basic/LangOptions.def

@@ -130,6 +130,7 @@ COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "require declaration of module us
 LANGOPT(ModulesErrorRecovery, 1, 1, "automatically import modules as needed when performing error recovery")
 BENIGN_LANGOPT(ModulesImplicitMaps, 1, 1, "use files called module.modulemap implicitly as module maps")
 BENIGN_LANGOPT(ImplicitModules, 1, 1, "build modules that are not specified via -fmodule-file")
+COMPATIBLE_LANGOPT(ModulesLocalVisibility, 1, 0, "local submodule visibility")
 COMPATIBLE_LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
 COMPATIBLE_LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")
 LANGOPT(Static            , 1, 0, "__STATIC__ predefined macro (as opposed to __DYNAMIC__)")

clang/include/clang/Basic/Module.h

@@ -480,14 +480,24 @@ class Module {
 class VisibleModuleSet {
 public:
   VisibleModuleSet() : Generation(0) {}
+  VisibleModuleSet(VisibleModuleSet &&O)
+      : ImportLocs(std::move(O.ImportLocs)), Generation(O.Generation ? 1 : 0) {
+    O.ImportLocs.clear();
+    ++O.Generation;
+  }
 
+  /// Move from another visible modules set. Guaranteed to leave the source
+  /// empty and bump the generation on both.
   VisibleModuleSet &operator=(VisibleModuleSet &&O) {
     ImportLocs = std::move(O.ImportLocs);
+    O.ImportLocs.clear();
+    ++O.Generation;
     ++Generation;
     return *this;
   }
 
-  /// \brief Get the current visibility generation.
+  /// \brief Get the current visibility generation. Incremented each time the
+  /// set of visible modules changes in any way.
   unsigned getGeneration() const { return Generation; }
 
   /// \brief Determine whether a module is visible.

clang/include/clang/Driver/CC1Options.td

@@ -347,6 +347,10 @@ def fmodule_map_file_home_is_cwd : Flag<["-"], "fmodule-map-file-home-is-cwd">,
 def fmodule_feature : Separate<["-"], "fmodule-feature">,
   MetaVarName<"<feature>">,
   HelpText<"Enable <feature> in module map requires declarations">;
+def fmodules_local_submodule_visibility :
+  Flag<["-"], "fmodules-local-submodule-visibility">,
+  HelpText<"Enforce name visibility rules across submodules of the same "
+           "top-level module.">;
 
 let Group = Action_Group in {
 

clang/include/clang/Lex/Preprocessor.h

@@ -466,12 +466,15 @@ class Preprocessor : public RefCountedBase<Preprocessor> {
         return Info->OverriddenMacros;
       return None;
     }
-    void setOverriddenMacros(ArrayRef<ModuleMacro*> Overrides) {
+    void setOverriddenMacros(Preprocessor &PP,
+                             ArrayRef<ModuleMacro *> Overrides) {
       auto *Info = State.dyn_cast<ModuleMacroInfo*>();
       if (!Info) {
-        assert(Overrides.empty() &&
-               "have overrides but never had module macro");
-        return;
+        if (Overrides.empty())
+          return;
+        Info = new (PP.getPreprocessorAllocator())
+            ModuleMacroInfo(State.get<MacroDirective *>());
+        State = Info;
       }
       Info->OverriddenMacros.clear();
       Info->OverriddenMacros.insert(Info->OverriddenMacros.end(),
@@ -498,16 +501,11 @@ class Preprocessor : public RefCountedBase<Preprocessor> {
     Module *M;
     /// The location at which the module was included.
     SourceLocation ImportLoc;
-
-    struct SavedMacroInfo {
-      SavedMacroInfo() : Latest(nullptr) {}
-      MacroDirective *Latest;
-      llvm::TinyPtrVector<ModuleMacro*> Overridden;
-    };
     /// The macros that were visible before we entered the module.
-    llvm::DenseMap<const IdentifierInfo*, SavedMacroInfo> Macros;
+    MacroMap Macros;
+    /// The set of modules that was visible in the surrounding submodule.
+    VisibleModuleSet VisibleModules;
 
-    // FIXME: VisibleModules?
     // FIXME: CounterValue?
     // FIXME: PragmaPushMacroInfo?
   };
@@ -662,6 +660,7 @@ class Preprocessor : public RefCountedBase<Preprocessor> {
   HeaderSearch &getHeaderSearchInfo() const { return HeaderInfo; }
 
   IdentifierTable &getIdentifierTable() { return Identifiers; }
+  const IdentifierTable &getIdentifierTable() const { return Identifiers; }
   SelectorTable &getSelectorTable() { return Selectors; }
   Builtin::Context &getBuiltinInfo() { return BuiltinInfo; }
   llvm::BumpPtrAllocator &getPreprocessorAllocator() { return BP; }

clang/lib/Frontend/CompilerInvocation.cpp

@@ -1508,6 +1508,8 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
   Opts.ModulesStrictDeclUse = Args.hasArg(OPT_fmodules_strict_decluse);
   Opts.ModulesDeclUse =
       Args.hasArg(OPT_fmodules_decluse) || Opts.ModulesStrictDeclUse;
+  Opts.ModulesLocalVisibility =
+      Args.hasArg(OPT_fmodules_local_submodule_visibility);
   Opts.ModulesSearchAll = Opts.Modules &&
     !Args.hasArg(OPT_fno_modules_search_all) &&
     Args.hasArg(OPT_fmodules_search_all);

clang/lib/Lex/PPLexerChange.cpp

@@ -612,16 +612,25 @@ void Preprocessor::HandleMicrosoftCommentPaste(Token &Tok) {
 void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) {
   // Save the current state for future imports.
   BuildingSubmoduleStack.push_back(BuildingSubmoduleInfo(M, ImportLoc));
-
   auto &Info = BuildingSubmoduleStack.back();
-  // Copy across our macros and start the submodule with the current state.
-  // FIXME: We should start each submodule with just the predefined macros.
-  for (auto &M : Macros) {
-    BuildingSubmoduleInfo::SavedMacroInfo SMI;
-    SMI.Latest = M.second.getLatest();
-    auto O = M.second.getOverriddenMacros();
-    SMI.Overridden.insert(SMI.Overridden.end(), O.begin(), O.end());
-    Info.Macros.insert(std::make_pair(M.first, SMI));
+  Info.Macros.swap(Macros);
+  // Save our visible modules set. This is guaranteed to clear the set.
+  if (getLangOpts().ModulesLocalVisibility)
+    Info.VisibleModules = std::move(VisibleModules);
+
+  // 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)
+                             ? BuildingSubmoduleStack[0].Macros
+                             : Info.Macros;
+
+  // Restore to the starting state.
+  // FIXME: Do this lazily, when each macro name is first referenced.
+  for (auto &Macro : StartingMacros) {
+    MacroState MS(Macro.second.getLatest());
+    MS.setOverriddenMacros(*this, MS.getOverriddenMacros());
+    Macros.insert(std::make_pair(Macro.first, std::move(MS)));
   }
 }
 
@@ -631,19 +640,35 @@ void Preprocessor::LeaveSubmodule() {
   // Create ModuleMacros for any macros defined in this submodule.
   for (auto &Macro : Macros) {
     auto *II = const_cast<IdentifierInfo*>(Macro.first);
-    auto SavedInfo = Info.Macros.lookup(II);
+    auto &OuterInfo = Info.Macros[II];
+
+    // Find the starting point for the MacroDirective chain in this submodule.
+    auto *OldMD = OuterInfo.getLatest();
+    if (getLangOpts().ModulesLocalVisibility &&
+        BuildingSubmoduleStack.size() > 1) {
+      auto &PredefMacros = BuildingSubmoduleStack[0].Macros;
+      auto PredefMacroIt = PredefMacros.find(Macro.first);
+      if (PredefMacroIt == PredefMacros.end())
+        OldMD = nullptr;
+      else
+        OldMD = PredefMacroIt->second.getLatest();
+    }
 
     // This module may have exported a new macro. If so, create a ModuleMacro
     // representing that fact.
     bool ExplicitlyPublic = false;
-    for (auto *MD = Macro.second.getLatest(); MD != SavedInfo.Latest;
+    for (auto *MD = Macro.second.getLatest(); MD != OldMD;
          MD = MD->getPrevious()) {
       assert(MD && "broken macro directive chain");
 
       // Skip macros defined in other submodules we #included along the way.
-      Module *Mod = getModuleContainingLocation(MD->getLocation());
-      if (Mod != Info.M)
-        continue;
+      // There's no point doing this if we're tracking local submodule
+      // visibiltiy, since there can be no such directives in our list.
+      if (!getLangOpts().ModulesLocalVisibility) {
+        Module *Mod = getModuleContainingLocation(MD->getLocation());
+        if (Mod != Info.M)
+          continue;
+      }
 
       if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
         // The latest visibility directive for a name in a submodule affects
@@ -667,11 +692,21 @@ void Preprocessor::LeaveSubmodule() {
       }
     }
 
-    // Restore the macro's overrides list.
-    Macro.second.setOverriddenMacros(SavedInfo.Overridden);
+    // Maintain a single macro directive chain if we're not tracking
+    // per-submodule macro visibility.
+    if (!getLangOpts().ModulesLocalVisibility)
+      OuterInfo.setLatest(Macro.second.getLatest());
   }
 
-  makeModuleVisible(Info.M, Info.ImportLoc);
+  // Put back the old macros.
+  std::swap(Info.Macros, Macros);
+
+  if (getLangOpts().ModulesLocalVisibility)
+    VisibleModules = std::move(Info.VisibleModules);
+
+  // A nested #include makes the included submodule visible.
+  if (BuildingSubmoduleStack.size() > 1)
+    makeModuleVisible(Info.M, Info.ImportLoc);
 
   BuildingSubmoduleStack.pop_back();
 }

clang/lib/Serialization/ASTWriter.cpp

@@ -2019,37 +2019,23 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
   // Loop over all the macro directives that are live at the end of the file,
   // emitting each to the PP section.
 
-  // Construct the list of macro directives that need to be serialized.
-  typedef std::pair<const IdentifierInfo *, MacroDirective *> MacroChain;
-  SmallVector<MacroChain, 2> MacroDirectives;
-  for (Preprocessor::macro_iterator
-         I = PP.macro_begin(/*IncludeExternalMacros=*/false),
-         E = PP.macro_end(/*IncludeExternalMacros=*/false);
-       I != E; ++I) {
-    MacroDirectives.push_back(std::make_pair(I->first, I->second.getLatest()));
-  }
-
+  // Construct the list of identifiers with macro directives that need to be
+  // serialized.
+  SmallVector<const IdentifierInfo *, 128> MacroIdentifiers;
+  for (auto &Id : PP.getIdentifierTable())
+    if (Id.second->hadMacroDefinition() &&
+        (!Id.second->isFromAST() ||
+         Id.second->hasChangedSinceDeserialization()))
+      MacroIdentifiers.push_back(Id.second);
   // Sort the set of macro definitions that need to be serialized by the
   // name of the macro, to provide a stable ordering.
-  int (*Cmp)(const MacroChain*, const MacroChain*) =
-    [](const MacroChain *A, const MacroChain *B) -> int {
-      return A->first->getName().compare(B->first->getName());
-    };
-  llvm::array_pod_sort(MacroDirectives.begin(), MacroDirectives.end(), Cmp);
+  std::sort(MacroIdentifiers.begin(), MacroIdentifiers.end(),
+            llvm::less_ptr<IdentifierInfo>());
 
   // Emit the macro directives as a list and associate the offset with the
   // identifier they belong to.
-  for (auto &Chain : MacroDirectives) {
-    const IdentifierInfo *Name = Chain.first;
-    MacroDirective *MD = Chain.second;
-
-    // If the macro or identifier need no updates, don't write the macro history
-    // for this one.
-    // FIXME: Chain the macro history instead of re-writing it.
-    if (MD && MD->isFromPCH() &&
-        Name->isFromAST() && !Name->hasChangedSinceDeserialization())
-      continue;
-
+  for (const IdentifierInfo *Name : MacroIdentifiers) {
+    MacroDirective *MD = PP.getLocalMacroDirectiveHistory(Name);
     auto StartOffset = Stream.GetCurrentBitNo();
 
     // Emit the macro directives in reverse source order.
@@ -2069,6 +2055,7 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
     }
 
     // Write out any exported module macros.
+    bool EmittedModuleMacros = false;
     if (IsModule) {
       auto Leafs = PP.getLeafModuleMacros(Name);
       SmallVector<ModuleMacro*, 8> Worklist(Leafs.begin(), Leafs.end());
@@ -2079,7 +2066,7 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
         // Emit a record indicating this submodule exports this macro.
         ModuleMacroRecord.push_back(
             getSubmoduleID(Macro->getOwningModule()));
-        ModuleMacroRecord.push_back(getMacroID(Macro->getMacroInfo()));
+        ModuleMacroRecord.push_back(getMacroRef(Macro->getMacroInfo(), Name));
         for (auto *M : Macro->overrides())
           ModuleMacroRecord.push_back(getSubmoduleID(M->getOwningModule()));
 
@@ -2090,10 +2077,12 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
         for (auto *M : Macro->overrides())
           if (++Visits[M] == M->getNumOverridingMacros())
             Worklist.push_back(M);
+
+        EmittedModuleMacros = true;
       }
     }
 
-    if (Record.empty())
+    if (Record.empty() && !EmittedModuleMacros)
       continue;
 
     IdentMacroDirectivesOffsetMap[Name] = StartOffset;

clang/test/Modules/macro-ambiguity.cpp

@@ -57,6 +57,27 @@
 // RUN:   -fmodule-file=%t/c.pcm \
 // RUN:   -fmodule-file=%t/d.pcm \
 // RUN:   -Wambiguous-macro -verify macro-ambiguity.cpp
+//
+// RUN: %clang_cc1 -fmodules -x c++ -fmodules-cache-path=%t \
+// RUN:   -v -fmodules-local-submodule-visibility \
+// RUN:   -iquote Inputs/macro-ambiguity/a/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/a/system \
+// RUN:   -iquote Inputs/macro-ambiguity/b/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/b/system \
+// RUN:   -iquote Inputs/macro-ambiguity/c/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/c/system \
+// RUN:   -iquote Inputs/macro-ambiguity/d/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/d/system \
+// RUN:   -iquote Inputs/macro-ambiguity/e/quote \
+// RUN:   -isystem Inputs/macro-ambiguity/e/system \
+// RUN:   -fno-implicit-modules -fno-modules-implicit-maps \
+// RUN:   -fmodule-map-file-home-is-cwd \
+// RUN:   -fmodule-map-file=Inputs/macro-ambiguity/module.modulemap \
+// RUN:   -fmodule-file=%t/a.pcm \
+// RUN:   -fmodule-file=%t/b.pcm \
+// RUN:   -fmodule-file=%t/c.pcm \
+// RUN:   -fmodule-file=%t/d.pcm \
+// RUN:   -Wambiguous-macro -verify macro-ambiguity.cpp
 
 // Include the textual headers first to maximize the ways in which things can
 // become ambiguous.

clang/test/Modules/macro-reexport.cpp

@@ -7,6 +7,15 @@
 // RUN: %clang_cc1 -fsyntax-only -DD2 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
 // RUN: %clang_cc1 -fsyntax-only -DF1 -I%S/Inputs/macro-reexport %s -fmodules-cache-path=%t -verify
 // RUN: %clang_cc1 -fsyntax-only -DF1 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
+//
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DC1 -I%S/Inputs/macro-reexport %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DC1 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DD1 -I%S/Inputs/macro-reexport %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DD1 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DD2 -I%S/Inputs/macro-reexport %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DD2 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DF1 -I%S/Inputs/macro-reexport %s -fmodules-cache-path=%t -verify
+// RUN: %clang_cc1 -fmodules-local-submodule-visibility -fsyntax-only -DF1 -I%S/Inputs/macro-reexport -fmodules %s -fmodules-cache-path=%t -verify
 
 #if defined(F1)
 #include "f1.h"

clang/test/Modules/macros.c

@@ -1,9 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_top %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_left %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_right %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros %S/Inputs/module.map
 // RUN: %clang_cc1 -fmodules -x objective-c -verify -fmodules-cache-path=%t -I %S/Inputs %s
+// RUN: %clang_cc1 -fmodules -DLOCAL_VISIBILITY -fmodules-local-submodule-visibility -x objective-c -verify -fmodules-cache-path=%t -I %S/Inputs %s
 // RUN: not %clang_cc1 -E -fmodules -x objective-c -fmodules-cache-path=%t -I %S/Inputs %s | FileCheck -check-prefix CHECK-PREPROCESSED %s
 // FIXME: When we have a syntax for modules in C, use that.
 // These notes come from headers in modules, and are bogus.
@@ -143,11 +140,20 @@ TOP_DEF_RIGHT_UNDEF *TDRUf() { return TDRUp; }
 
 int TOP_DEF_RIGHT_UNDEF; // ok, no longer defined
 
-// FIXME: When macros_right.undef is built, macros_top is visible because
-// the state from building macros_right leaks through, so macros_right.undef
-// undefines macros_top's macro.
-#ifdef TOP_RIGHT_UNDEF
-# error TOP_RIGHT_UNDEF should not be defined
+#ifdef LOCAL_VISIBILITY
+// TOP_RIGHT_UNDEF should not be undefined, because macros_right.undef does
+// not undefine macros_right's macro.
+# ifndef TOP_RIGHT_UNDEF
+#  error TOP_RIGHT_UNDEF should still be defined
+# endif
+#else
+// When macros_right.undef is built and local submodule visibility is not
+// enabled, macros_top is visible because the state from building
+// macros_right leaks through, so macros_right.undef undefines macros_top's
+// macro.
+# ifdef TOP_RIGHT_UNDEF
+#  error TOP_RIGHT_UNDEF should not be defined
+# endif
 #endif
 
 @import macros_other;

clang/test/Modules/macros2.c

@@ -1,9 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_top %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_left %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_right %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros %S/Inputs/module.map
 // RUN: %clang_cc1 -fmodules -x objective-c -verify -fmodules-cache-path=%t -I %S/Inputs %s
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -x objective-c -verify -fmodules-cache-path=%t -I %S/Inputs %s -DLOCAL_VISIBILITY
 
 // This test checks some of the same things as macros.c, but imports modules in
 // a different order.
@@ -49,9 +46,15 @@ void test() {
 
 @import macros_right.undef;
 
-// FIXME: See macros.c.
-#ifdef TOP_RIGHT_UNDEF
-# error TOP_RIGHT_UNDEF should not be defined
+// See macros.c.
+#ifdef LOCAL_VISIBILITY
+# ifndef TOP_RIGHT_UNDEF
+#  error TOP_RIGHT_UNDEF should still be defined
+# endif
+#else
+# ifdef TOP_RIGHT_UNDEF
+#  error TOP_RIGHT_UNDEF should not be defined
+# endif
 #endif
 
 #ifndef TOP_OTHER_UNDEF1