<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 12 May 2017 at 14:19, Juergen Ributzka <span dir="ltr"><<a href="mailto:juergen@ributzka.de" target="_blank">juergen@ributzka.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Richard,<div><br></div><div>I think this broke modules.</div><div><br></div><div>If I try to compile a simple test program (@import Foundation;) I get the following error:</div><div><div>./bin/clang test.m -fmodules -c -isysroot /Applications/Xcode.app/<wbr>Contents/Developer/Platforms/<wbr>MacOSX.platform/Developer/<wbr>SDKs/MacOSX10.12.sdk </div><div>While building module 'Foundation' imported from test.m:1:</div><div>While building module 'CoreGraphics' imported from /Applications/Xcode.app/<wbr>Contents/Developer/Platforms/<wbr>MacOSX.platform/Developer/<wbr>SDKs/MacOSX10.12.sdk/System/<wbr>Library/Frameworks/Foundation.<wbr>framework/Headers/NSGeometry.<wbr>h:12:</div><div>While building module 'IOKit' imported from /Applications/Xcode.app/<wbr>Contents/Developer/Platforms/<wbr>MacOSX.platform/Developer/<wbr>SDKs/MacOSX10.12.sdk/System/<wbr>Library/Frameworks/<wbr>CoreGraphics.framework/<wbr>Headers/<wbr>CGDisplayConfiguration.h:12:</div><div>In file included from <module-includes>:40:</div><div>In file included from /Applications/Xcode.app/<wbr>Contents/Developer/Platforms/<wbr>MacOSX.platform/Developer/<wbr>SDKs/MacOSX10.12.sdk/System/<wbr>Library/Frameworks/IOKit.<wbr>framework/Headers/hid/<wbr>IOHIDDevicePlugIn.h:39:</div><div>/Applications/Xcode.app/<wbr>Contents/Developer/Platforms/<wbr>MacOSX.platform/Developer/<wbr>SDKs/MacOSX10.12.sdk/System/<wbr>Library/Frameworks/IOKit.<wbr>framework/Headers/hid/<wbr>IOHIDLibObsolete.h:41:8: error: redefinition of</div><div>      'IOHIDEventStruct'</div><div>struct IOHIDEventStruct</div></div></div></blockquote><div><br></div><div>Already reverted, I'm investigating.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Cheers,</div><div>Juergen</div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 12, 2017 at 11:56 AM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Fri May 12 13:56:03 2017<br>
New Revision: 302932<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=302932&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=302932&view=rev</a><br>
Log:<br>
[modules] Simplify module macro handling in non-local-submodule-visibility mode.<br>
<br>
When reaching the end of a module, we used to convert its macros to<br>
ModuleMacros but also leave them in the MacroDirective chain for the<br>
identifier. This meant that every lookup of such a macro would find two<br>
(identical) definitions. It also made it difficult to determine the correct<br>
owner for a macro when reaching the end of a module: the most recent<br>
MacroDirective in the chain could be from an #included submodule rather than<br>
the current module.<br>
<br>
Simplify this: whenever we convert a MacroDirective to a ModuleMacro when<br>
leaving a module, clear out the MacroDirective chain for that identifier, and<br>
just rely on the ModuleMacro to provide the macro definition information.<br>
<br>
(We don't want to do this for local submodule visibility mode, because in that<br>
mode we maintain a distinct MacroDirective chain for each submodule, and we<br>
need to keep around the prior MacroDirective in case we re-enter the submodule<br>
-- for instance, if its header is #included more than once in a module build,<br>
we need the include guard directive to stick around. But the problem doesn't<br>
arise in this case for the same reason: each submodule has its own<br>
MacroDirective chain, so the macros don't leak out of submodules in the first<br>
place.)<br>
<br>
Modified:<br>
    cfe/trunk/lib/Lex/PPLexerChang<wbr>e.cpp<br>
<br>
Modified: cfe/trunk/lib/Lex/PPLexerChang<wbr>e.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=302932&r1=302931&r2=302932&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Lex/PPLexe<wbr>rChange.cpp?rev=302932&r1=<wbr>302931&r2=302932&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Lex/PPLexerChang<wbr>e.cpp (original)<br>
+++ cfe/trunk/lib/Lex/PPLexerChang<wbr>e.cpp Fri May 12 13:56:03 2017<br>
@@ -731,7 +731,7 @@ Module *Preprocessor::LeaveSubmodule(<wbr>boo<br>
   Module *LeavingMod = Info.M;<br>
   SourceLocation ImportLoc = Info.ImportLoc;<br>
<br>
-  if (!needModuleMacros() ||<br>
+  if (!needModuleMacros() ||<br>
       (!getLangOpts().ModulesLocalV<wbr>isibility &&<br>
        LeavingMod->getTopLevelModuleN<wbr>ame() != getLangOpts().CurrentModule)) {<br>
     // If we don't need module macros, or this is not a module for which we<br>
@@ -777,17 +777,6 @@ Module *Preprocessor::LeaveSubmodule(<wbr>boo<br>
     for (auto *MD = Macro.getLatest(); MD != OldMD; MD = MD->getPrevious()) {<br>
       assert(MD && "broken macro directive chain");<br>
<br>
-      // Stop on macros defined in other submodules of this module that we<br>
-      // #included along the way. There's no point doing this if we're<br>
-      // tracking local submodule visibility, since there can be no such<br>
-      // directives in our list.<br>
-      if (!getLangOpts().ModulesLocalVi<wbr>sibility) {<br>
-        Module *Mod = getModuleContainingLocation(MD<wbr>->getLocation());<br>
-        if (Mod != LeavingMod &&<br>
-            Mod->getTopLevelModule() == LeavingMod->getTopLevelModule(<wbr>))<br>
-          break;<br>
-      }<br>
-<br>
       if (auto *VisMD = dyn_cast<VisibilityMacroDirect<wbr>ive>(MD)) {<br>
         // The latest visibility directive for a name in a submodule affects<br>
         // all the directives that come before it.<br>
@@ -809,6 +798,12 @@ Module *Preprocessor::LeaveSubmodule(<wbr>boo<br>
         if (Def || !Macro.getOverriddenMacros().e<wbr>mpty())<br>
           addModuleMacro(LeavingMod, II, Def,<br>
                          Macro.getOverriddenMacros(), IsNew);<br>
+<br>
+        if (!getLangOpts().ModulesLocalVi<wbr>sibility) {<br>
+          // This macro is exposed to the rest of this compilation as a<br>
+          // ModuleMacro; we don't need to track its MacroDirective any more.<br>
+          Macro.setLatest(nullptr);<br>
+        }<br>
         break;<br>
       }<br>
     }<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>