Hi Richard, I think this broke modules.
If I try to compile a simple test program (@import Foundation;) I get the following error: ./bin/clang test.m -fmodules -c -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk While building module 'Foundation' imported from test.m:1: While building module 'CoreGraphics' imported from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSGeometry.h:12: While building module 'IOKit' imported from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/CoreGraphics.framework/Headers/CGDisplayConfiguration.h:12: In file included from <module-includes>:40: In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/IOKit.framework/Headers/hid/IOHIDDevicePlugIn.h:39: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/System/Library/Frameworks/IOKit.framework/Headers/hid/IOHIDLibObsolete.h:41:8: error: redefinition of 'IOHIDEventStruct' struct IOHIDEventStruct Cheers, Juergen On Fri, May 12, 2017 at 11:56 AM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > 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; > } > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits