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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri May 12 14:53:12 PDT 2017


On 12 May 2017 at 14:19, Juergen Ributzka <juergen at ributzka.de> wrote:

> 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
>

Already reverted, I'm investigating.


> Cheers,
> Juergen
>
>
> On Fri, May 12, 2017 at 11:56 AM, Richard Smith via cfe-commits <
> cfe-commits at 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/PPLexe
>> rChange.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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170512/efb38507/attachment.html>


More information about the cfe-commits mailing list