r302932 - [modules] Simplify module macro handling in non-local-submodule-visibility mode.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri May 12 13:56:03 2017
New Revision: 302932

URL: http://llvm.org/viewvc/llvm-project?rev=302932&view=rev
Log:
[modules] Simplify module macro handling in non-local-submodule-visibility mode.

When reaching the end of a module, we used to convert its macros to
ModuleMacros but also leave them in the MacroDirective chain for the
identifier. This meant that every lookup of such a macro would find two
(identical) definitions. It also made it difficult to determine the correct
owner for a macro when reaching the end of a module: the most recent
MacroDirective in the chain could be from an #included submodule rather than
the current module.

Simplify this: whenever we convert a MacroDirective to a ModuleMacro when
leaving a module, clear out the MacroDirective chain for that identifier, and
just rely on the ModuleMacro to provide the macro definition information.

(We don't want to do this for local submodule visibility mode, because in that
mode we maintain a distinct MacroDirective chain for each submodule, and we
need to keep around the prior MacroDirective in case we re-enter the submodule
-- for instance, if its header is #included more than once in a module build,
we need the include guard directive to stick around. But the problem doesn't
arise in this case for the same reason: each submodule has its own
MacroDirective chain, so the macros don't leak out of submodules in the first
place.)

Modified:
    cfe/trunk/lib/Lex/PPLexerChange.cpp

Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=302932&r1=302931&r2=302932&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
+++ cfe/trunk/lib/Lex/PPLexerChange.cpp Fri May 12 13:56:03 2017
@@ -731,7 +731,7 @@ Module *Preprocessor::LeaveSubmodule(boo
   Module *LeavingMod = Info.M;
   SourceLocation ImportLoc = Info.ImportLoc;
 
-  if (!needModuleMacros() || 
+  if (!needModuleMacros() ||
       (!getLangOpts().ModulesLocalVisibility &&
        LeavingMod->getTopLevelModuleName() != getLangOpts().CurrentModule)) {
     // If we don't need module macros, or this is not a module for which we
@@ -777,17 +777,6 @@ Module *Preprocessor::LeaveSubmodule(boo
     for (auto *MD = Macro.getLatest(); MD != OldMD; MD = MD->getPrevious()) {
       assert(MD && "broken macro directive chain");
 
-      // Stop on macros defined in other submodules of this module that we
-      // #included along the way. There's no point doing this if we're
-      // tracking local submodule visibility, since there can be no such
-      // directives in our list.
-      if (!getLangOpts().ModulesLocalVisibility) {
-        Module *Mod = getModuleContainingLocation(MD->getLocation());
-        if (Mod != LeavingMod &&
-            Mod->getTopLevelModule() == LeavingMod->getTopLevelModule())
-          break;
-      }
-
       if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
         // The latest visibility directive for a name in a submodule affects
         // all the directives that come before it.
@@ -809,6 +798,12 @@ Module *Preprocessor::LeaveSubmodule(boo
         if (Def || !Macro.getOverriddenMacros().empty())
           addModuleMacro(LeavingMod, II, Def,
                          Macro.getOverriddenMacros(), IsNew);
+
+        if (!getLangOpts().ModulesLocalVisibility) {
+          // This macro is exposed to the rest of this compilation as a
+          // ModuleMacro; we don't need to track its MacroDirective any more.
+          Macro.setLatest(nullptr);
+        }
         break;
       }
     }