<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Mar 20, 2014, at 5:41 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Mar 20, 2014 at 3:40 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div class="">On Thu, Mar 20, 2014 at 3:20 PM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto;">Hi Richard,<br>
<br>
I’'m seeing an assertion failure that I think is caused by this commit. A macro that is undef’d in a module imported by a pch seems to get an invalid location, which then triggers an assertion in UndefMacroDirective’s constructor. The details, as well as a test case, are at <a href="http://llvm.org/PR19215" target="_blank">http://llvm.org/PR19215</a>. Could you take a look?<br>
</blockquote><div><br></div></div><div>Thanks for the heads-up, investigating...</div></div></div></div></blockquote><div><br></div><div>Just a shallow bug: we were failing to propagate the import location for the module from the PCH build into the user of the PCH. Fixed in r204417.<br></div></div></div></div></blockquote><div><br></div>Great, thanks!</div><div><br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><div><span style="color:rgb(80,0,80)"> </span></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Thanks,<br>
<br>
Ben<br>
<div><br>
> Author: rsmith<br>
> Date: Fri Feb 28 18:08:04 2014<br>
> New Revision: 202560<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=202560&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=202560&view=rev</a><br>
> Log:<br>
> If a module A exports a macro M, and a module B imports that macro and #undef's<br>
> it, importers of B should not see the macro. This is complicated by the fact<br>
> that A's macro could also be visible through a different path. The rules (as<br>
> hashed out on cfe-commits) are included as a documentation update in this<br>
> change.<br>
><br>
> With this, the number of regressions in libc++'s testsuite when modules are<br>
> enabled drops from 47 to 7. Those remaining 7 are also macro-related, and are<br>
> due to remaining bugs in this change (in particular, the handling of submodules<br>
> is imperfect).<br>
><br>
> Added:<br>
> cfe/trunk/test/Modules/macros2.c<br>
> cfe/trunk/test/PCH/macro-undef.cpp<br>
> Modified:<br>
> cfe/trunk/docs/Modules.rst<br>
> cfe/trunk/include/clang/Basic/Module.h<br>
> cfe/trunk/include/clang/Serialization/ASTReader.h<br>
> cfe/trunk/include/clang/Serialization/Module.h<br>
> cfe/trunk/lib/Lex/MacroInfo.cpp<br>
> cfe/trunk/lib/Lex/PPMacroExpansion.cpp<br>
> cfe/trunk/lib/Serialization/ASTReader.cpp<br>
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
> cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
> cfe/trunk/test/Modules/Inputs/macros_other.h<br>
> cfe/trunk/test/Modules/Inputs/macros_right.h<br>
> cfe/trunk/test/Modules/Inputs/macros_right_undef.h<br>
> cfe/trunk/test/Modules/Inputs/macros_top.h<br>
> cfe/trunk/test/Modules/Inputs/module.map<br>
> cfe/trunk/test/Modules/macros.c<br>
><br>
> Modified: cfe/trunk/docs/Modules.rst<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/docs/Modules.rst (original)<br>
> +++ cfe/trunk/docs/Modules.rst Fri Feb 28 18:08:04 2014<br>
> @@ -198,6 +198,40 @@ Command-line parameters<br>
> ``-fmodule-map-file=<file>``<br>
> Load the given module map file if a header from its directory or one of its subdirectories is loaded.<br>
><br>
> +Module Semantics<br>
> +================<br>
> +<br>
> +Modules are modeled as if each submodule were a separate translation unit, and a module import makes names from the other translation unit visible. Each submodule starts with a new preprocessor state and an empty translation unit.<br>
> +<br>
> +.. note::<br>
> +<br>
> + This behavior is currently only approximated when building a module. Entities within a submodule that has already been built are visible when building later submodules in that module. This can lead to fragile modules that depend on the build order used for the submodules of the module, and should not be relied upon.<br>
> +<br>
> +As an example, in C, this implies that if two structs are defined in different submodules with the same name, those two types are distinct types (but may be *compatible* types if their definitions match. In C++, two structs defined with the same name in different submodules are the *same* type, and must be equivalent under C++'s One Definition Rule.<br>
> +<br>
> +.. note::<br>
> +<br>
> + Clang currently only performs minimal checking for violations of the One Definition Rule.<br>
> +<br>
> +Macros<br>
> +------<br>
> +<br>
> +The C and C++ preprocessor assumes that the input text is a single linear buffer, but with modules this is not the case. It is possible to import two modules that have conflicting definitions for a macro (or where one ``#define``\s a macro and the other ``#undef``\ines it). The rules for handling macro definitions in the presence of modules are as follows:<br>
> +<br>
> +* Each definition and undefinition of a macro is considered to be a distinct entity.<br>
> +* Such entities are *visible* if they are from the current submodule or translation unit, or if they were exported from a submodule that has been imported.<br>
> +* A ``#define X`` or ``#undef X`` directive *overrides* all definitions of ``X`` that are visible at the point of the directive.<br>
> +* A ``#define`` or ``#undef`` directive is *active* if it is visible and no visible directive overrides it.<br>
> +* A set of macro directives is *consistent* if it consists of only ``#undef`` directives, or if all ``#define`` directives in the set define the macro name to the same sequence of tokens (following the usual rules for macro redefinitions).<br>
> +* If a macro name is used and the set of active directives is not consistent, the program is ill-formed. Otherwise, the (unique) meaning of the macro name is used.<br>
> +<br>
> +For example, suppose:<br>
> +<br>
> +* ``<stdio.h>`` defines a macro ``getc`` (and exports its ``#define``)<br>
> +* ``<cstdio>`` imports the ``<stdio.h>`` module and undefines the macro (and exports its ``#undef``)<br>
> +<br>
> +The ``#undef`` overrides the ``#define``, and a source file that imports both modules *in any order* will not see ``getc`` defined as a macro.<br>
> +<br>
> Module Map Language<br>
> ===================<br>
><br>
><br>
> Modified: cfe/trunk/include/clang/Basic/Module.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/Module.h (original)<br>
> +++ cfe/trunk/include/clang/Basic/Module.h Fri Feb 28 18:08:04 2014<br>
> @@ -160,11 +160,14 @@ public:<br>
> MacrosVisible,<br>
> /// \brief All of the names in this module are visible.<br>
> AllVisible<br>
> - };<br>
> -<br>
> - ///\ brief The visibility of names within this particular module.<br>
> + };<br>
> +<br>
> + /// \brief The visibility of names within this particular module.<br>
> NameVisibilityKind NameVisibility;<br>
><br>
> + /// \brief The location at which macros within this module became visible.<br>
> + SourceLocation MacroVisibilityLoc;<br>
> +<br>
> /// \brief The location of the inferred submodule.<br>
> SourceLocation InferredSubmoduleLoc;<br>
><br>
><br>
> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original)<br>
> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Feb 28 18:08:04 2014<br>
> @@ -64,6 +64,7 @@ class ASTUnit; // FIXME: Layering violat<br>
> class Attr;<br>
> class Decl;<br>
> class DeclContext;<br>
> +class DefMacroDirective;<br>
> class DiagnosticOptions;<br>
> class NestedNameSpecifier;<br>
> class CXXBaseSpecifier;<br>
> @@ -471,18 +472,22 @@ private:<br>
> /// global submodule ID to produce a local ID.<br>
> GlobalSubmoduleMapType GlobalSubmoduleMap;<br>
><br>
> + /// \brief Information on a macro definition or undefinition that is visible<br>
> + /// at the end of a submodule.<br>
> + struct ModuleMacroInfo;<br>
> +<br>
> /// \brief An entity that has been hidden.<br>
> class HiddenName {<br>
> public:<br>
> enum NameKind {<br>
> Declaration,<br>
> - MacroVisibility<br>
> + Macro<br>
> } Kind;<br>
><br>
> private:<br>
> union {<br>
> Decl *D;<br>
> - MacroDirective *MD;<br>
> + ModuleMacroInfo *MMI;<br>
> };<br>
><br>
> IdentifierInfo *Id;<br>
> @@ -490,8 +495,8 @@ private:<br>
> public:<br>
> HiddenName(Decl *D) : Kind(Declaration), D(D), Id() { }<br>
><br>
> - HiddenName(IdentifierInfo *II, MacroDirective *MD)<br>
> - : Kind(MacroVisibility), MD(MD), Id(II) { }<br>
> + HiddenName(IdentifierInfo *II, ModuleMacroInfo *MMI)<br>
> + : Kind(Macro), MMI(MMI), Id(II) { }<br>
><br>
> NameKind getKind() const { return Kind; }<br>
><br>
> @@ -500,15 +505,21 @@ private:<br>
> return D;<br>
> }<br>
><br>
> - std::pair<IdentifierInfo *, MacroDirective *> getMacro() const {<br>
> - assert(getKind() == MacroVisibility && "Hidden name is not a macro!");<br>
> - return std::make_pair(Id, MD);<br>
> + std::pair<IdentifierInfo *, ModuleMacroInfo *> getMacro() const {<br>
> + assert(getKind() == Macro && "Hidden name is not a macro!");<br>
> + return std::make_pair(Id, MMI);<br>
> }<br>
> };<br>
><br>
> + typedef llvm::SmallDenseMap<IdentifierInfo*,<br>
> + ModuleMacroInfo*> HiddenMacrosMap;<br>
> +<br>
> /// \brief A set of hidden declarations.<br>
> - typedef SmallVector<HiddenName, 2> HiddenNames;<br>
> -<br>
> + struct HiddenNames {<br>
> + SmallVector<Decl*, 2> HiddenDecls;<br>
> + HiddenMacrosMap HiddenMacros;<br>
> + };<br>
> +<br>
> typedef llvm::DenseMap<Module *, HiddenNames> HiddenNamesMapType;<br>
><br>
> /// \brief A mapping from each of the hidden submodules to the deserialized<br>
> @@ -564,8 +575,8 @@ private:<br>
> ModuleFile *M;<br>
><br>
> struct ModuleMacroDataTy {<br>
> - serialization::GlobalMacroID GMacID;<br>
> - unsigned ImportLoc;<br>
> + uint32_t MacID;<br>
> + serialization::SubmoduleID *Overrides;<br>
> };<br>
> struct PCHMacroDataTy {<br>
> uint64_t MacroDirectivesOffset;<br>
> @@ -577,10 +588,10 @@ private:<br>
> };<br>
><br>
> PendingMacroInfo(ModuleFile *M,<br>
> - serialization::GlobalMacroID GMacID,<br>
> - SourceLocation ImportLoc) : M(M) {<br>
> - ModuleMacroData.GMacID = GMacID;<br>
> - ModuleMacroData.ImportLoc = ImportLoc.getRawEncoding();<br>
> + uint32_t MacID,<br>
> + serialization::SubmoduleID *Overrides) : M(M) {<br>
> + ModuleMacroData.MacID = MacID;<br>
> + ModuleMacroData.Overrides = Overrides;<br>
> }<br>
><br>
> PendingMacroInfo(ModuleFile *M, uint64_t MacroDirectivesOffset) : M(M) {<br>
> @@ -1253,14 +1264,14 @@ public:<br>
> /// \param ImportLoc The location at which the import occurs.<br>
> ///<br>
> /// \param Complain Whether to complain about conflicting module imports.<br>
> - void makeModuleVisible(Module *Mod,<br>
> + void makeModuleVisible(Module *Mod,<br>
> Module::NameVisibilityKind NameVisibility,<br>
> SourceLocation ImportLoc,<br>
> bool Complain);<br>
> -<br>
> +<br>
> /// \brief Make the names within this set of hidden names visible.<br>
> void makeNamesVisible(const HiddenNames &Names, Module *Owner);<br>
> -<br>
> +<br>
> /// \brief Set the AST callbacks listener.<br>
> void setListener(ASTReaderListener *listener) {<br>
> Listener.reset(listener);<br>
> @@ -1687,14 +1698,27 @@ public:<br>
> serialization::IdentifierID getGlobalIdentifierID(ModuleFile &M,<br>
> unsigned LocalID);<br>
><br>
> + ModuleMacroInfo *getModuleMacro(const PendingMacroInfo &PMInfo);<br>
> +<br>
> void resolvePendingMacro(IdentifierInfo *II, const PendingMacroInfo &PMInfo);<br>
><br>
> void installPCHMacroDirectives(IdentifierInfo *II,<br>
> ModuleFile &M, uint64_t Offset);<br>
><br>
> - void installImportedMacro(IdentifierInfo *II, MacroDirective *MD,<br>
> + void installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,<br>
> Module *Owner);<br>
><br>
> + typedef llvm::SmallVector<DefMacroDirective*, 1> AmbiguousMacros;<br>
> + llvm::DenseMap<IdentifierInfo*, AmbiguousMacros> AmbiguousMacroDefs;<br>
> +<br>
> + void<br>
> + removeOverriddenMacros(IdentifierInfo *II, AmbiguousMacros &Ambig,<br>
> + llvm::ArrayRef<serialization::SubmoduleID> Overrides);<br>
> +<br>
> + AmbiguousMacros *<br>
> + removeOverriddenMacros(IdentifierInfo *II,<br>
> + llvm::ArrayRef<serialization::SubmoduleID> Overrides);<br>
> +<br>
> /// \brief Retrieve the macro with the given ID.<br>
> MacroInfo *getMacro(serialization::MacroID ID);<br>
><br>
> @@ -1875,7 +1899,7 @@ public:<br>
> void addPendingMacroFromModule(IdentifierInfo *II,<br>
> ModuleFile *M,<br>
> serialization::GlobalMacroID GMacID,<br>
> - SourceLocation ImportLoc);<br>
> + llvm::ArrayRef<serialization::SubmoduleID>);<br>
><br>
> /// \brief Add a macro to deserialize its macro directive history from a PCH.<br>
> ///<br>
><br>
> Modified: cfe/trunk/include/clang/Serialization/Module.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Serialization/Module.h (original)<br>
> +++ cfe/trunk/include/clang/Serialization/Module.h Fri Feb 28 18:08:04 2014<br>
> @@ -171,6 +171,9 @@ public:<br>
> /// If module A depends on and imports module B, both modules will have the<br>
> /// same DirectImportLoc, but different ImportLoc (B's ImportLoc will be a<br>
> /// source location inside module A).<br>
> + ///<br>
> + /// WARNING: This is largely useless. It doesn't tell you when a module was<br>
> + /// made visible, just when the first submodule of that module was imported.<br>
> SourceLocation DirectImportLoc;<br>
><br>
> /// \brief The source location where this module was first imported.<br>
><br>
> Modified: cfe/trunk/lib/Lex/MacroInfo.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Lex/MacroInfo.cpp (original)<br>
> +++ cfe/trunk/lib/Lex/MacroInfo.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -144,7 +144,7 @@ MacroDirective::DefInfo MacroDirective::<br>
> isPublic = VisMD->isPublic();<br>
> }<br>
><br>
> - return DefInfo();<br>
> + return DefInfo(0, UndefLoc, !isPublic.hasValue() || isPublic.getValue());<br>
> }<br>
><br>
> const MacroDirective::DefInfo<br>
><br>
> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)<br>
> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -293,11 +293,11 @@ bool Preprocessor::HandleMacroExpandedId<br>
> for (MacroDirective::DefInfo PrevDef = Def.getPreviousDefinition();<br>
> PrevDef && !PrevDef.isUndefined();<br>
> PrevDef = PrevDef.getPreviousDefinition()) {<br>
> - if (PrevDef.getDirective()->isAmbiguous()) {<br>
> - Diag(PrevDef.getMacroInfo()->getDefinitionLoc(),<br>
> - diag::note_pp_ambiguous_macro_other)<br>
> - << Identifier.getIdentifierInfo();<br>
> - }<br>
> + Diag(PrevDef.getMacroInfo()->getDefinitionLoc(),<br>
> + diag::note_pp_ambiguous_macro_other)<br>
> + << Identifier.getIdentifierInfo();<br>
> + if (!PrevDef.getDirective()->isAmbiguous())<br>
> + break;<br>
> }<br>
> }<br>
><br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -579,11 +579,34 @@ IdentifierInfo *ASTIdentifierLookupTrait<br>
> }<br>
><br>
> if (F.Kind == MK_Module) {<br>
> + // Macro definitions are stored from newest to oldest, so reverse them<br>
> + // before registering them.<br>
> + llvm::SmallVector<unsigned, 8> MacroSizes;<br>
> for (SmallVectorImpl<uint32_t>::iterator<br>
> - I = LocalMacroIDs.begin(), E = LocalMacroIDs.end(); I != E; ++I) {<br>
> - MacroID MacID = Reader.getGlobalMacroID(F, *I);<br>
> - Reader.addPendingMacroFromModule(II, &F, MacID, F.DirectImportLoc);<br>
> + I = LocalMacroIDs.begin(), E = LocalMacroIDs.end(); I != E; /**/) {<br>
> + unsigned Size = 1;<br>
> +<br>
> + static const uint32_t HasOverridesFlag = 0x80000000U;<br>
> + if (I + 1 != E && (I[1] & HasOverridesFlag))<br>
> + Size += 1 + (I[1] & ~HasOverridesFlag);<br>
> +<br>
> + MacroSizes.push_back(Size);<br>
> + I += Size;<br>
> + }<br>
> +<br>
> + SmallVectorImpl<uint32_t>::iterator I = LocalMacroIDs.end();<br>
> + for (SmallVectorImpl<unsigned>::reverse_iterator SI = MacroSizes.rbegin(),<br>
> + SE = MacroSizes.rend();<br>
> + SI != SE; ++SI) {<br>
> + I -= *SI;<br>
> +<br>
> + uint32_t LocalMacroID = *I;<br>
> + llvm::ArrayRef<uint32_t> Overrides;<br>
> + if (*SI != 1)<br>
> + Overrides = llvm::makeArrayRef(&I[2], *SI - 2);<br>
> + Reader.addPendingMacroFromModule(II, &F, LocalMacroID, Overrides);<br>
> }<br>
> + assert(I == LocalMacroIDs.begin());<br>
> } else {<br>
> Reader.addPendingMacroFromPCH(II, &F, MacroDirectivesOffset);<br>
> }<br>
> @@ -1323,12 +1346,19 @@ HeaderFileInfoTrait::ReadData(internal_k<br>
> return HFI;<br>
> }<br>
><br>
> -void ASTReader::addPendingMacroFromModule(IdentifierInfo *II,<br>
> - ModuleFile *M,<br>
> - GlobalMacroID GMacID,<br>
> - SourceLocation ImportLoc) {<br>
> +void<br>
> +ASTReader::addPendingMacroFromModule(IdentifierInfo *II, ModuleFile *M,<br>
> + GlobalMacroID GMacID,<br>
> + llvm::ArrayRef<SubmoduleID> Overrides) {<br>
> assert(NumCurrentElementsDeserializing > 0 &&"Missing deserialization guard");<br>
> - PendingMacroIDs[II].push_back(PendingMacroInfo(M, GMacID, ImportLoc));<br>
> + SubmoduleID *OverrideData = 0;<br>
> + if (!Overrides.empty()) {<br>
> + OverrideData = new (Context) SubmoduleID[Overrides.size() + 1];<br>
> + OverrideData[0] = Overrides.size();<br>
> + for (unsigned I = 0; I != Overrides.size(); ++I)<br>
> + OverrideData[I + 1] = getGlobalSubmoduleID(*M, Overrides[I]);<br>
> + }<br>
> + PendingMacroIDs[II].push_back(PendingMacroInfo(M, GMacID, OverrideData));<br>
> }<br>
><br>
> void ASTReader::addPendingMacroFromPCH(IdentifierInfo *II,<br>
> @@ -1477,6 +1507,59 @@ void ASTReader::markIdentifierUpToDate(I<br>
> IdentifierGeneration[II] = CurrentGeneration;<br>
> }<br>
><br>
> +struct ASTReader::ModuleMacroInfo {<br>
> + SubmoduleID SubModID;<br>
> + MacroInfo *MI;<br>
> + SubmoduleID *Overrides;<br>
> + // FIXME: Remove this.<br>
> + ModuleFile *F;<br>
> +<br>
> + bool isDefine() const { return MI; }<br>
> +<br>
> + SubmoduleID getSubmoduleID() const { return SubModID; }<br>
> +<br>
> + llvm::ArrayRef<SubmoduleID> getOverriddenSubmodules() const {<br>
> + if (!Overrides)<br>
> + return llvm::ArrayRef<SubmoduleID>();<br>
> + return llvm::makeArrayRef(Overrides + 1, *Overrides);<br>
> + }<br>
> +<br>
> + DefMacroDirective *import(Preprocessor &PP, SourceLocation ImportLoc) const {<br>
> + if (!MI)<br>
> + return 0;<br>
> + return PP.AllocateDefMacroDirective(MI, ImportLoc, /*isImported=*/true);<br>
> + }<br>
> +};<br>
> +<br>
> +ASTReader::ModuleMacroInfo *<br>
> +ASTReader::getModuleMacro(const PendingMacroInfo &PMInfo) {<br>
> + ModuleMacroInfo Info;<br>
> +<br>
> + uint32_t ID = PMInfo.ModuleMacroData.MacID;<br>
> + if (ID & 1) {<br>
> + // Macro undefinition.<br>
> + Info.SubModID = getGlobalSubmoduleID(*PMInfo.M, ID >> 1);<br>
> + Info.MI = 0;<br>
> + } else {<br>
> + // Macro definition.<br>
> + GlobalMacroID GMacID = getGlobalMacroID(*PMInfo.M, ID >> 1);<br>
> + assert(GMacID);<br>
> +<br>
> + // If this macro has already been loaded, don't do so again.<br>
> + // FIXME: This is highly dubious. Multiple macro definitions can have the<br>
> + // same MacroInfo (and hence the same GMacID) due to #pragma push_macro etc.<br>
> + if (MacrosLoaded[GMacID - NUM_PREDEF_MACRO_IDS])<br>
> + return 0;<br>
> +<br>
> + Info.MI = getMacro(GMacID);<br>
> + Info.SubModID = Info.MI->getOwningModuleID();<br>
> + }<br>
> + Info.Overrides = PMInfo.ModuleMacroData.Overrides;<br>
> + Info.F = PMInfo.M;<br>
> +<br>
> + return new (Context) ModuleMacroInfo(Info);<br>
> +}<br>
> +<br>
> void ASTReader::resolvePendingMacro(IdentifierInfo *II,<br>
> const PendingMacroInfo &PMInfo) {<br>
> assert(II);<br>
> @@ -1486,42 +1569,21 @@ void ASTReader::resolvePendingMacro(Iden<br>
> PMInfo.PCHMacroData.MacroDirectivesOffset);<br>
> return;<br>
> }<br>
> -<br>
> +<br>
> // Module Macro.<br>
><br>
> - GlobalMacroID GMacID = PMInfo.ModuleMacroData.GMacID;<br>
> - SourceLocation ImportLoc =<br>
> - SourceLocation::getFromRawEncoding(PMInfo.ModuleMacroData.ImportLoc);<br>
> -<br>
> - assert(GMacID);<br>
> - // If this macro has already been loaded, don't do so again.<br>
> - if (MacrosLoaded[GMacID - NUM_PREDEF_MACRO_IDS])<br>
> + ModuleMacroInfo *MMI = getModuleMacro(PMInfo);<br>
> + if (!MMI)<br>
> return;<br>
><br>
> - MacroInfo *MI = getMacro(GMacID);<br>
> - SubmoduleID SubModID = MI->getOwningModuleID();<br>
> - MacroDirective *MD = PP.AllocateDefMacroDirective(MI, ImportLoc,<br>
> - /*isImported=*/true);<br>
> -<br>
> - // Determine whether this macro definition is visible.<br>
> - bool Hidden = false;<br>
> - Module *Owner = 0;<br>
> - if (SubModID) {<br>
> - if ((Owner = getSubmodule(SubModID))) {<br>
> - if (Owner->NameVisibility == Module::Hidden) {<br>
> - // The owning module is not visible, and this macro definition<br>
> - // should not be, either.<br>
> - Hidden = true;<br>
> -<br>
> - // Note that this macro definition was hidden because its owning<br>
> - // module is not yet visible.<br>
> - HiddenNamesMap[Owner].push_back(HiddenName(II, MD));<br>
> - }<br>
> - }<br>
> + Module *Owner = getSubmodule(MMI->getSubmoduleID());<br>
> + if (Owner && Owner->NameVisibility == Module::Hidden) {<br>
> + // Macros in the owning module are hidden. Just remember this macro to<br>
> + // install if we make this module visible.<br>
> + HiddenNamesMap[Owner].HiddenMacros.insert(std::make_pair(II, MMI));<br>
> + } else {<br>
> + installImportedMacro(II, MMI, Owner);<br>
> }<br>
> -<br>
> - if (!Hidden)<br>
> - installImportedMacro(II, MD, Owner);<br>
> }<br>
><br>
> void ASTReader::installPCHMacroDirectives(IdentifierInfo *II,<br>
> @@ -1606,31 +1668,138 @@ static bool areDefinedInSystemModules(Ma<br>
> return PrevInSystem && NewInSystem;<br>
> }<br>
><br>
> -void ASTReader::installImportedMacro(IdentifierInfo *II, MacroDirective *MD,<br>
> - Module *Owner) {<br>
> - assert(II && MD);<br>
> +void ASTReader::removeOverriddenMacros(IdentifierInfo *II,<br>
> + AmbiguousMacros &Ambig,<br>
> + llvm::ArrayRef<SubmoduleID> Overrides) {<br>
> + for (unsigned OI = 0, ON = Overrides.size(); OI != ON; ++OI) {<br>
> + SubmoduleID OwnerID = Overrides[OI];<br>
> +<br>
> + // If this macro is not yet visible, remove it from the hidden names list.<br>
> + Module *Owner = getSubmodule(OwnerID);<br>
> + HiddenNames &Hidden = HiddenNamesMap[Owner];<br>
> + HiddenMacrosMap::iterator HI = Hidden.HiddenMacros.find(II);<br>
> + if (HI != Hidden.HiddenMacros.end()) {<br>
> + removeOverriddenMacros(II, Ambig, HI->second->getOverriddenSubmodules());<br>
> + Hidden.HiddenMacros.erase(HI);<br>
> + }<br>
> +<br>
> + // If this macro is already in our list of conflicts, remove it from there.<br>
> + for (unsigned AI = 0, AN = Ambig.size(); AI != AN; ++AI)<br>
> + if (Ambig[AI]->getInfo()->getOwningModuleID() == OwnerID)<br>
> + Ambig.erase(Ambig.begin() + AI);<br>
> + }<br>
> +}<br>
><br>
> - DefMacroDirective *DefMD = cast<DefMacroDirective>(MD);<br>
> +ASTReader::AmbiguousMacros *<br>
> +ASTReader::removeOverriddenMacros(IdentifierInfo *II,<br>
> + llvm::ArrayRef<SubmoduleID> Overrides) {<br>
> MacroDirective *Prev = PP.getMacroDirective(II);<br>
> - if (Prev) {<br>
> - MacroDirective::DefInfo PrevDef = Prev->getDefinition();<br>
> - MacroInfo *PrevMI = PrevDef.getMacroInfo();<br>
> - MacroInfo *NewMI = DefMD->getInfo();<br>
> - if (NewMI != PrevMI && !PrevMI->isIdenticalTo(*NewMI, PP,<br>
> - /*Syntactically=*/true)) {<br>
> - // Before marking the macros as ambiguous, check if this is a case where<br>
> - // both macros are in system headers. If so, we trust that the system<br>
> - // did not get it wrong. This also handles cases where Clang's own<br>
> - // headers have a different spelling of certain system macros:<br>
> - // #define LONG_MAX __LONG_MAX__ (clang's limits.h)<br>
> - // #define LONG_MAX 0x7fffffffffffffffL (system's limits.h)<br>
> - if (!areDefinedInSystemModules(PrevMI, NewMI, Owner, *this)) {<br>
> - PrevDef.getDirective()->setAmbiguous(true);<br>
> - DefMD->setAmbiguous(true);<br>
> - }<br>
> + if (!Prev && Overrides.empty())<br>
> + return 0;<br>
> +<br>
> + DefMacroDirective *PrevDef = Prev ? Prev->getDefinition().getDirective() : 0;<br>
> + if (PrevDef && PrevDef->isAmbiguous()) {<br>
> + // We had a prior ambiguity. Check whether we resolve it (or make it worse).<br>
> + AmbiguousMacros &Ambig = AmbiguousMacroDefs[II];<br>
> + Ambig.push_back(PrevDef);<br>
> +<br>
> + removeOverriddenMacros(II, Ambig, Overrides);<br>
> +<br>
> + if (!Ambig.empty())<br>
> + return &Ambig;<br>
> +<br>
> + AmbiguousMacroDefs.erase(II);<br>
> + } else {<br>
> + // There's no ambiguity yet. Maybe we're introducing one.<br>
> + llvm::SmallVector<DefMacroDirective*, 1> Ambig;<br>
> + if (PrevDef)<br>
> + Ambig.push_back(PrevDef);<br>
> +<br>
> + removeOverriddenMacros(II, Ambig, Overrides);<br>
> +<br>
> + if (!Ambig.empty()) {<br>
> + AmbiguousMacros &Result = AmbiguousMacroDefs[II];<br>
> + Result.swap(Ambig);<br>
> + return &Result;<br>
> }<br>
> }<br>
> -<br>
> +<br>
> + // We ended up with no ambiguity.<br>
> + return 0;<br>
> +}<br>
> +<br>
> +void ASTReader::installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,<br>
> + Module *Owner) {<br>
> + assert(II && Owner);<br>
> +<br>
> + SourceLocation ImportLoc = Owner->MacroVisibilityLoc;<br>
> + if (ImportLoc.isInvalid()) {<br>
> + // FIXME: If we made macros from this module visible but didn't provide a<br>
> + // source location for the import, we don't have a location for the macro.<br>
> + // Use the location at which the containing module file was first imported<br>
> + // for now.<br>
> + ImportLoc = MMI->F->DirectImportLoc;<br>
> + }<br>
> +<br>
> + llvm::SmallVectorImpl<DefMacroDirective*> *Prev =<br>
> + removeOverriddenMacros(II, MMI->getOverriddenSubmodules());<br>
> +<br>
> +<br>
> + // Create a synthetic macro definition corresponding to the import (or null<br>
> + // if this was an undefinition of the macro).<br>
> + DefMacroDirective *MD = MMI->import(PP, ImportLoc);<br>
> +<br>
> + // If there's no ambiguity, just install the macro.<br>
> + if (!Prev) {<br>
> + if (MD)<br>
> + PP.appendMacroDirective(II, MD);<br>
> + else<br>
> + PP.appendMacroDirective(II, PP.AllocateUndefMacroDirective(ImportLoc));<br>
> + return;<br>
> + }<br>
> + assert(!Prev->empty());<br>
> +<br>
> + if (!MD) {<br>
> + // We imported a #undef that didn't remove all prior definitions. The most<br>
> + // recent prior definition remains, and we install it in the place of the<br>
> + // imported directive.<br>
> + MacroInfo *NewMI = Prev->back()->getInfo();<br>
> + Prev->pop_back();<br>
> + MD = PP.AllocateDefMacroDirective(NewMI, ImportLoc, /*Imported*/true);<br>
> + }<br>
> +<br>
> + // We're introducing a macro definition that creates or adds to an ambiguity.<br>
> + // We can resolve that ambiguity if this macro is token-for-token identical to<br>
> + // all of the existing definitions.<br>
> + MacroInfo *NewMI = MD->getInfo();<br>
> + assert(NewMI && "macro definition with no MacroInfo?");<br>
> + while (!Prev->empty()) {<br>
> + MacroInfo *PrevMI = Prev->back()->getInfo();<br>
> + assert(PrevMI && "macro definition with no MacroInfo?");<br>
> +<br>
> + // Before marking the macros as ambiguous, check if this is a case where<br>
> + // both macros are in system headers. If so, we trust that the system<br>
> + // did not get it wrong. This also handles cases where Clang's own<br>
> + // headers have a different spelling of certain system macros:<br>
> + // #define LONG_MAX __LONG_MAX__ (clang's limits.h)<br>
> + // #define LONG_MAX 0x7fffffffffffffffL (system's limits.h)<br>
> + //<br>
> + // FIXME: Remove the defined-in-system-headers check. clang's limits.h<br>
> + // overrides the system limits.h's macros, so there's no conflict here.<br>
> + if (NewMI != PrevMI &&<br>
> + !PrevMI->isIdenticalTo(*NewMI, PP, /*Syntactically=*/true) &&<br>
> + !areDefinedInSystemModules(PrevMI, NewMI, Owner, *this))<br>
> + break;<br>
> +<br>
> + // The previous definition is the same as this one (or both are defined in<br>
> + // system modules so we can assume they're equivalent); we don't need to<br>
> + // track it any more.<br>
> + Prev->pop_back();<br>
> + }<br>
> +<br>
> + if (!Prev->empty())<br>
> + MD->setAmbiguous(true);<br>
> +<br>
> PP.appendMacroDirective(II, MD);<br>
> }<br>
><br>
> @@ -2828,30 +2997,25 @@ static void moveMethodToBackOfGlobalList<br>
> }<br>
><br>
> void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) {<br>
> - for (unsigned I = 0, N = Names.size(); I != N; ++I) {<br>
> - switch (Names[I].getKind()) {<br>
> - case HiddenName::Declaration: {<br>
> - Decl *D = Names[I].getDecl();<br>
> - bool wasHidden = D->Hidden;<br>
> - D->Hidden = false;<br>
> -<br>
> - if (wasHidden && SemaObj) {<br>
> - if (ObjCMethodDecl *Method = dyn_cast<ObjCMethodDecl>(D)) {<br>
> - moveMethodToBackOfGlobalList(*SemaObj, Method);<br>
> - }<br>
> + for (unsigned I = 0, N = Names.HiddenDecls.size(); I != N; ++I) {<br>
> + Decl *D = Names.HiddenDecls[I];<br>
> + bool wasHidden = D->Hidden;<br>
> + D->Hidden = false;<br>
> +<br>
> + if (wasHidden && SemaObj) {<br>
> + if (ObjCMethodDecl *Method = dyn_cast<ObjCMethodDecl>(D)) {<br>
> + moveMethodToBackOfGlobalList(*SemaObj, Method);<br>
> }<br>
> - break;<br>
> - }<br>
> - case HiddenName::MacroVisibility: {<br>
> - std::pair<IdentifierInfo *, MacroDirective *> Macro = Names[I].getMacro();<br>
> - installImportedMacro(Macro.first, Macro.second, Owner);<br>
> - break;<br>
> - }<br>
> }<br>
> }<br>
> +<br>
> + for (HiddenMacrosMap::const_iterator I = Names.HiddenMacros.begin(),<br>
> + E = Names.HiddenMacros.end();<br>
> + I != E; ++I)<br>
> + installImportedMacro(I->first, I->second, Owner);<br>
> }<br>
><br>
> -void ASTReader::makeModuleVisible(Module *Mod,<br>
> +void ASTReader::makeModuleVisible(Module *Mod,<br>
> Module::NameVisibilityKind NameVisibility,<br>
> SourceLocation ImportLoc,<br>
> bool Complain) {<br>
> @@ -2866,15 +3030,18 @@ void ASTReader::makeModuleVisible(Module<br>
> // there is nothing more to do.<br>
> continue;<br>
> }<br>
> -<br>
> +<br>
> if (!Mod->isAvailable()) {<br>
> // Modules that aren't available cannot be made visible.<br>
> continue;<br>
> }<br>
><br>
> // Update the module's name visibility.<br>
> + if (NameVisibility >= Module::MacrosVisible &&<br>
> + Mod->NameVisibility < Module::MacrosVisible)<br>
> + Mod->MacroVisibilityLoc = ImportLoc;<br>
> Mod->NameVisibility = NameVisibility;<br>
> -<br>
> +<br>
> // If we've already deserialized any names from this module,<br>
> // mark them as visible.<br>
> HiddenNamesMapType::iterator Hidden = HiddenNamesMap.find(Mod);<br>
> @@ -7457,14 +7624,14 @@ void ASTReader::finishPendingActions() {<br>
> SmallVector<PendingMacroInfo, 2> GlobalIDs;<br>
> GlobalIDs.swap(PendingMacroIDs.begin()[I].second);<br>
> // Initialize the macro history from chained-PCHs ahead of module imports.<br>
> - for (unsigned IDIdx = 0, NumIDs = GlobalIDs.size(); IDIdx != NumIDs;<br>
> + for (unsigned IDIdx = 0, NumIDs = GlobalIDs.size(); IDIdx != NumIDs;<br>
> ++IDIdx) {<br>
> const PendingMacroInfo &Info = GlobalIDs[IDIdx];<br>
> if (Info.M->Kind != MK_Module)<br>
> resolvePendingMacro(II, Info);<br>
> }<br>
> // Handle module imports.<br>
> - for (unsigned IDIdx = 0, NumIDs = GlobalIDs.size(); IDIdx != NumIDs;<br>
> + for (unsigned IDIdx = 0, NumIDs = GlobalIDs.size(); IDIdx != NumIDs;<br>
> ++IDIdx) {<br>
> const PendingMacroInfo &Info = GlobalIDs[IDIdx];<br>
> if (Info.M->Kind == MK_Module)<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -412,7 +412,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {<br>
><br>
> // Note that this declaration was hidden because its owning module is<br>
> // not yet visible.<br>
> - Reader.HiddenNamesMap[Owner].push_back(D);<br>
> + Reader.HiddenNamesMap[Owner].HiddenDecls.push_back(D);<br>
> }<br>
> }<br>
> }<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -2962,80 +2962,91 @@ class ASTIdentifierTableTrait {<br>
> return false;<br>
> }<br>
><br>
> - DefMacroDirective *getFirstPublicSubmoduleMacro(MacroDirective *MD,<br>
> - SubmoduleID &ModID) {<br>
> + typedef llvm::SmallVectorImpl<SubmoduleID> OverriddenList;<br>
> +<br>
> + MacroDirective *<br>
> + getFirstPublicSubmoduleMacro(MacroDirective *MD, SubmoduleID &ModID) {<br>
> ModID = 0;<br>
> - if (DefMacroDirective *DefMD = getPublicSubmoduleMacro(MD, ModID))<br>
> - if (!shouldIgnoreMacro(DefMD, IsModule, PP))<br>
> - return DefMD;<br>
> + llvm::SmallVector<SubmoduleID, 1> Overridden;<br>
> + if (MacroDirective *NextMD = getPublicSubmoduleMacro(MD, ModID, Overridden))<br>
> + if (!shouldIgnoreMacro(NextMD, IsModule, PP))<br>
> + return NextMD;<br>
> return 0;<br>
> }<br>
><br>
> - DefMacroDirective *getNextPublicSubmoduleMacro(DefMacroDirective *MD,<br>
> - SubmoduleID &ModID) {<br>
> - if (DefMacroDirective *<br>
> - DefMD = getPublicSubmoduleMacro(MD->getPrevious(), ModID))<br>
> - if (!shouldIgnoreMacro(DefMD, IsModule, PP))<br>
> - return DefMD;<br>
> + MacroDirective *<br>
> + getNextPublicSubmoduleMacro(MacroDirective *MD, SubmoduleID &ModID,<br>
> + OverriddenList &Overridden) {<br>
> + if (MacroDirective *NextMD =<br>
> + getPublicSubmoduleMacro(MD->getPrevious(), ModID, Overridden))<br>
> + if (!shouldIgnoreMacro(NextMD, IsModule, PP))<br>
> + return NextMD;<br>
> return 0;<br>
> }<br>
><br>
> /// \brief Traverses the macro directives history and returns the latest<br>
> - /// macro that is public and not undefined in the same submodule.<br>
> - /// A macro that is defined in submodule A and undefined in submodule B,<br>
> + /// public macro definition or undefinition that is not in ModID.<br>
> + /// A macro that is defined in submodule A and undefined in submodule B<br>
> /// will still be considered as defined/exported from submodule A.<br>
> - DefMacroDirective *getPublicSubmoduleMacro(MacroDirective *MD,<br>
> - SubmoduleID &ModID) {<br>
> + /// ModID is updated to the module containing the returned directive.<br>
> + ///<br>
> + /// FIXME: This process breaks down if a module defines a macro, imports<br>
> + /// another submodule that changes the macro, then changes the<br>
> + /// macro again itself.<br>
> + MacroDirective *getPublicSubmoduleMacro(MacroDirective *MD,<br>
> + SubmoduleID &ModID,<br>
> + OverriddenList &Overridden) {<br>
> if (!MD)<br>
> return 0;<br>
><br>
> + Overridden.clear();<br>
> SubmoduleID OrigModID = ModID;<br>
> - bool isUndefined = false;<br>
> - Optional<bool> isPublic;<br>
> + Optional<bool> IsPublic;<br>
> for (; MD; MD = MD->getPrevious()) {<br>
> SubmoduleID ThisModID = getSubmoduleID(MD);<br>
> if (ThisModID == 0) {<br>
> - isUndefined = false;<br>
> - isPublic = Optional<bool>();<br>
> + IsPublic = Optional<bool>();<br>
> continue;<br>
> }<br>
> - if (ThisModID != ModID){<br>
> + if (ThisModID != ModID) {<br>
> ModID = ThisModID;<br>
> - isUndefined = false;<br>
> - isPublic = Optional<bool>();<br>
> + IsPublic = Optional<bool>();<br>
> }<br>
> +<br>
> + // If this is a definition from a submodule import, that submodule's<br>
> + // definition is overridden by the definition or undefinition that we<br>
> + // started with.<br>
> + // FIXME: This should only apply to macros defined in OrigModID.<br>
> + // We can't do that currently, because a #include of a different submodule<br>
> + // of the same module just leaks through macros instead of providing new<br>
> + // DefMacroDirectives for them.<br>
> + if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))<br>
> + if (SubmoduleID SourceID = DefMD->getInfo()->getOwningModuleID())<br>
> + Overridden.push_back(SourceID);<br>
> +<br>
> // We are looking for a definition in a different submodule than the one<br>
> // that we started with. If a submodule has re-definitions of the same<br>
> // macro, only the last definition will be used as the "exported" one.<br>
> if (ModID == OrigModID)<br>
> continue;<br>
><br>
> - if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD)) {<br>
> - if (!isUndefined && (!isPublic.hasValue() || isPublic.getValue()))<br>
> - return DefMD;<br>
> - continue;<br>
> - }<br>
> -<br>
> - if (isa<UndefMacroDirective>(MD)) {<br>
> - isUndefined = true;<br>
> - continue;<br>
> + // The latest visibility directive for a name in a submodule affects all<br>
> + // the directives that come before it.<br>
> + if (VisibilityMacroDirective *VisMD =<br>
> + dyn_cast<VisibilityMacroDirective>(MD)) {<br>
> + if (!IsPublic.hasValue())<br>
> + IsPublic = VisMD->isPublic();<br>
> + } else if (!IsPublic.hasValue() || IsPublic.getValue()) {<br>
> + // FIXME: If we find an imported macro, we should include its list of<br>
> + // overrides in our export.<br>
> + return MD;<br>
> }<br>
> -<br>
> - VisibilityMacroDirective *VisMD = cast<VisibilityMacroDirective>(MD);<br>
> - if (!isPublic.hasValue())<br>
> - isPublic = VisMD->isPublic();<br>
> }<br>
><br>
> return 0;<br>
> }<br>
><br>
> SubmoduleID getSubmoduleID(MacroDirective *MD) {<br>
> - if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD)) {<br>
> - MacroInfo *MI = DefMD->getInfo();<br>
> - if (unsigned ID = MI->getOwningModuleID())<br>
> - return ID;<br>
> - return Writer.inferSubmoduleIDFromLocation(MI->getDefinitionLoc());<br>
> - }<br>
> return Writer.inferSubmoduleIDFromLocation(MD->getLocation());<br>
> }<br>
><br>
> @@ -3066,11 +3077,18 @@ public:<br>
> DataLen += 4; // MacroDirectives offset.<br>
> if (IsModule) {<br>
> SubmoduleID ModID;<br>
> - for (DefMacroDirective *<br>
> - DefMD = getFirstPublicSubmoduleMacro(Macro, ModID);<br>
> - DefMD; DefMD = getNextPublicSubmoduleMacro(DefMD, ModID)) {<br>
> - DataLen += 4; // MacroInfo ID.<br>
> + llvm::SmallVector<SubmoduleID, 4> Overridden;<br>
> + for (MacroDirective *<br>
> + MD = getFirstPublicSubmoduleMacro(Macro, ModID);<br>
> + MD; MD = getNextPublicSubmoduleMacro(MD, ModID, Overridden)) {<br>
> + // Previous macro's overrides.<br>
> + if (!Overridden.empty())<br>
> + DataLen += 4 * (1 + Overridden.size());<br>
> + DataLen += 4; // MacroInfo ID or ModuleID.<br>
> }<br>
> + // Previous macro's overrides.<br>
> + if (!Overridden.empty())<br>
> + DataLen += 4 * (1 + Overridden.size());<br>
> DataLen += 4;<br>
> }<br>
> }<br>
> @@ -3096,6 +3114,15 @@ public:<br>
> Out.write(II->getNameStart(), KeyLen);<br>
> }<br>
><br>
> + static void emitMacroOverrides(raw_ostream &Out,<br>
> + llvm::ArrayRef<SubmoduleID> Overridden) {<br>
> + if (!Overridden.empty()) {<br>
> + clang::io::Emit32(Out, Overridden.size() | 0x80000000U);<br>
> + for (unsigned I = 0, N = Overridden.size(); I != N; ++I)<br>
> + clang::io::Emit32(Out, Overridden[I]);<br>
> + }<br>
> + }<br>
> +<br>
> void EmitData(raw_ostream& Out, IdentifierInfo* II,<br>
> IdentID ID, unsigned) {<br>
> MacroDirective *Macro = 0;<br>
> @@ -3123,13 +3150,22 @@ public:<br>
> if (IsModule) {<br>
> // Write the IDs of macros coming from different submodules.<br>
> SubmoduleID ModID;<br>
> - for (DefMacroDirective *<br>
> - DefMD = getFirstPublicSubmoduleMacro(Macro, ModID);<br>
> - DefMD; DefMD = getNextPublicSubmoduleMacro(DefMD, ModID)) {<br>
> - MacroID InfoID = Writer.getMacroID(DefMD->getInfo());<br>
> - assert(InfoID);<br>
> - clang::io::Emit32(Out, InfoID);<br>
> + llvm::SmallVector<SubmoduleID, 4> Overridden;<br>
> + for (MacroDirective *<br>
> + MD = getFirstPublicSubmoduleMacro(Macro, ModID);<br>
> + MD; MD = getNextPublicSubmoduleMacro(MD, ModID, Overridden)) {<br>
> + MacroID InfoID = 0;<br>
> + emitMacroOverrides(Out, Overridden);<br>
> + if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD)) {<br>
> + InfoID = Writer.getMacroID(DefMD->getInfo());<br>
> + assert(InfoID);<br>
> + clang::io::Emit32(Out, InfoID << 1);<br>
> + } else {<br>
> + assert(isa<UndefMacroDirective>(MD));<br>
> + clang::io::Emit32(Out, (ModID << 1) | 1);<br>
> + }<br>
> }<br>
> + emitMacroOverrides(Out, Overridden);<br>
> clang::io::Emit32(Out, 0);<br>
> }<br>
> }<br>
><br>
> Modified: cfe/trunk/test/Modules/Inputs/macros_other.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_other.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_other.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/macros_other.h (original)<br>
> +++ cfe/trunk/test/Modules/Inputs/macros_other.h Fri Feb 28 18:08:04 2014<br>
> @@ -1 +1,6 @@<br>
> -#define OTHER_INTEGER int<br>
> +#undef TOP_OTHER_UNDEF1<br>
> +#define TOP_OTHER_UNDEF2 42<br>
> +#define TOP_OTHER_REDEF1 1<br>
> +#define TOP_OTHER_REDEF1 3<br>
> +<br>
> +#define TOP_OTHER_DEF_RIGHT_UNDEF 4<br>
><br>
> Modified: cfe/trunk/test/Modules/Inputs/macros_right.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_right.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_right.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/macros_right.h (original)<br>
> +++ cfe/trunk/test/Modules/Inputs/macros_right.h Fri Feb 28 18:08:04 2014<br>
> @@ -17,3 +17,5 @@<br>
> #define TOP_RIGHT_REDEF float<br>
><br>
> #define FN_ADD(x, y) (x+y)<br>
> +<br>
> +#undef TOP_OTHER_DEF_RIGHT_UNDEF<br>
><br>
> Modified: cfe/trunk/test/Modules/Inputs/macros_right_undef.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_right_undef.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_right_undef.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/macros_right_undef.h (original)<br>
> +++ cfe/trunk/test/Modules/Inputs/macros_right_undef.h Fri Feb 28 18:08:04 2014<br>
> @@ -1 +1,4 @@<br>
> #undef TOP_RIGHT_UNDEF<br>
> +<br>
> +@import macros_top;<br>
> +#undef TOP_OTHER_DEF_RIGHT_UNDEF<br>
><br>
> Modified: cfe/trunk/test/Modules/Inputs/macros_top.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_top.h?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_top.h?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/macros_top.h (original)<br>
> +++ cfe/trunk/test/Modules/Inputs/macros_top.h Fri Feb 28 18:08:04 2014<br>
> @@ -14,3 +14,9 @@<br>
><br>
> #define TOP_RIGHT_UNDEF int<br>
><br>
> +#define TOP_OTHER_UNDEF1 42<br>
> +#undef TOP_OTHER_UNDEF2<br>
> +#define TOP_OTHER_REDEF1 1<br>
> +#define TOP_OTHER_REDEF2 2<br>
> +<br>
> +#define TOP_OTHER_DEF_RIGHT_UNDEF void<br>
><br>
> Modified: cfe/trunk/test/Modules/Inputs/module.map<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module.map?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/module.map (original)<br>
> +++ cfe/trunk/test/Modules/Inputs/module.map Fri Feb 28 18:08:04 2014<br>
> @@ -33,6 +33,7 @@ module macros_right {<br>
> }<br>
> }<br>
> module macros { header "macros.h" }<br>
> +module macros_other { header "macros_other.h" }<br>
> module category_top { header "category_top.h" }<br>
> module category_left {<br>
> header "category_left.h"<br>
><br>
> Modified: cfe/trunk/test/Modules/macros.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros.c?rev=202560&r1=202559&r2=202560&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros.c?rev=202560&r1=202559&r2=202560&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/macros.c (original)<br>
> +++ cfe/trunk/test/Modules/macros.c Fri Feb 28 18:08:04 2014<br>
> @@ -11,10 +11,8 @@<br>
> // FIXME: expected-note@Inputs/macros_left.h:11{{previous definition is here}}<br>
> // FIXME: expected-note@Inputs/macros_right.h:12{{previous definition is here}}<br>
> // expected-note@Inputs/macros_right.h:12{{expanding this definition of 'LEFT_RIGHT_DIFFERENT'}}<br>
> -// expected-note@Inputs/macros_top.h:13{{other definition of 'TOP_RIGHT_REDEF'}}<br>
> // expected-note@Inputs/macros_right.h:13{{expanding this definition of 'LEFT_RIGHT_DIFFERENT2'}}<br>
> // expected-note@Inputs/macros_left.h:14{{other definition of 'LEFT_RIGHT_DIFFERENT'}}<br>
> -// expected-note@Inputs/macros_right.h:17{{expanding this definition of 'TOP_RIGHT_REDEF'}}<br>
><br>
> @import macros;<br>
><br>
> @@ -79,8 +77,8 @@ void f() {<br>
> # error TOP should be visible<br>
> #endif<br>
><br>
> -#ifndef TOP_LEFT_UNDEF<br>
> -# error TOP_LEFT_UNDEF should still be defined<br>
> +#ifdef TOP_LEFT_UNDEF<br>
> +# error TOP_LEFT_UNDEF should not be defined<br>
> #endif<br>
><br>
> void test1() {<br>
> @@ -112,7 +110,7 @@ void test2() {<br>
> int i;<br>
> float f;<br>
> double d;<br>
> - TOP_RIGHT_REDEF *fp = &f; // expected-warning{{ambiguous expansion of macro 'TOP_RIGHT_REDEF'}}<br>
> + TOP_RIGHT_REDEF *fp = &f; // ok, right's definition overrides top's definition<br>
><br>
> LEFT_RIGHT_IDENTICAL *ip = &i;<br>
> LEFT_RIGHT_DIFFERENT *ip2 = &i; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT'}}<br>
> @@ -134,6 +132,33 @@ void test3() {<br>
><br>
> @import macros_right.undef;<br>
><br>
> -#ifndef TOP_RIGHT_UNDEF<br>
> -# error TOP_RIGHT_UNDEF should still be defined<br>
> +// FIXME: When macros_right.undef is built, macros_top is visible because<br>
> +// the state from building macros_right leaks through, so macros_right.undef<br>
> +// undefines macros_top's macro.<br>
> +#ifdef TOP_RIGHT_UNDEF<br>
> +# error TOP_RIGHT_UNDEF should not be defined<br>
> +#endif<br>
> +<br>
> +@import macros_other;<br>
> +<br>
> +#ifndef TOP_OTHER_UNDEF1<br>
> +# error TOP_OTHER_UNDEF1 should still be defined<br>
> +#endif<br>
> +<br>
> +#ifndef TOP_OTHER_UNDEF2<br>
> +# error TOP_OTHER_UNDEF2 should still be defined<br>
> #endif<br>
> +<br>
> +#ifndef TOP_OTHER_REDEF1<br>
> +# error TOP_OTHER_REDEF1 should still be defined<br>
> +#endif<br>
> +int n1 = TOP_OTHER_REDEF1; // expected-warning{{ambiguous expansion of macro 'TOP_OTHER_REDEF1'}}<br>
> +// expected-note@macros_top.h:19 {{expanding this definition}}<br>
> +// expected-note@macros_other.h:4 {{other definition}}<br>
> +<br>
> +#ifndef TOP_OTHER_REDEF2<br>
> +# error TOP_OTHER_REDEF2 should still be defined<br>
> +#endif<br>
> +int n2 = TOP_OTHER_REDEF2; // ok<br>
> +<br>
> +int n3 = TOP_OTHER_DEF_RIGHT_UNDEF; // ok<br>
><br>
> Added: cfe/trunk/test/Modules/macros2.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros2.c?rev=202560&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros2.c?rev=202560&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/macros2.c (added)<br>
> +++ cfe/trunk/test/Modules/macros2.c Fri Feb 28 18:08:04 2014<br>
> @@ -0,0 +1,77 @@<br>
> +// RUN: rm -rf %t<br>
> +// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_top %S/Inputs/module.map<br>
> +// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_left %S/Inputs/module.map<br>
> +// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros_right %S/Inputs/module.map<br>
> +// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodules-cache-path=%t -fmodule-name=macros %S/Inputs/module.map<br>
> +// RUN: %clang_cc1 -fmodules -x objective-c -verify -fmodules-cache-path=%t -I %S/Inputs %s<br>
> +<br>
> +// This test checks some of the same things as macros.c, but imports modules in<br>
> +// a different order.<br>
> +<br>
> +@import macros_other;<br>
> +<br>
> +int n0 = TOP_OTHER_DEF_RIGHT_UNDEF; // ok<br>
> +<br>
> +@import macros_top;<br>
> +<br>
> +TOP_OTHER_DEF_RIGHT_UNDEF *n0b; // expected-warning{{ambiguous expansion of macro 'TOP_OTHER_DEF_RIGHT_UNDEF'}}<br>
> +// expected-note@macros_top.h:22 {{expanding this definition of 'TOP_OTHER_DEF_RIGHT_UNDEF'}}<br>
> +// expected-note@macros_other.h:6 {{other definition of 'TOP_OTHER_DEF_RIGHT_UNDEF'}}<br>
> +<br>
> +@import macros_right;<br>
> +@import macros_left;<br>
> +<br>
> +#ifdef TOP_LEFT_UNDEF<br>
> +# error TOP_LEFT_UNDEF should not be defined<br>
> +#endif<br>
> +<br>
> +#ifndef TOP_RIGHT_UNDEF<br>
> +# error TOP_RIGHT_UNDEF should still be defined<br>
> +#endif<br>
> +<br>
> +void test() {<br>
> + float f;<br>
> + TOP_RIGHT_REDEF *fp = &f; // ok, right's definition overrides top's definition<br>
> +<br>
> + // Note, left's definition wins here, whereas right's definition wins in<br>
> + // macros.c.<br>
> + int i;<br>
> + LEFT_RIGHT_IDENTICAL *ip = &i;<br>
> + LEFT_RIGHT_DIFFERENT *ip2 = &f; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT'}}<br>
> + // expected-note@macros_left.h:14 {{expanding this}}<br>
> + // expected-note@macros_right.h:12 {{other}}<br>
> + LEFT_RIGHT_DIFFERENT2 *ip3 = &f; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT2}}<br>
> + // expected-note@macros_left.h:11 {{expanding this}}<br>
> + // expected-note@macros_right.h:13 {{other}}<br>
> +#undef LEFT_RIGHT_DIFFERENT3<br>
> + int LEFT_RIGHT_DIFFERENT3;<br>
> +}<br>
> +<br>
> +@import macros_right.undef;<br>
> +<br>
> +// FIXME: See macros.c.<br>
> +#ifdef TOP_RIGHT_UNDEF<br>
> +# error TOP_RIGHT_UNDEF should not be defined<br>
> +#endif<br>
> +<br>
> +#ifndef TOP_OTHER_UNDEF1<br>
> +# error TOP_OTHER_UNDEF1 should still be defined<br>
> +#endif<br>
> +<br>
> +#ifndef TOP_OTHER_UNDEF2<br>
> +# error TOP_OTHER_UNDEF2 should still be defined<br>
> +#endif<br>
> +<br>
> +#ifndef TOP_OTHER_REDEF1<br>
> +# error TOP_OTHER_REDEF1 should still be defined<br>
> +#endif<br>
> +int n1 = TOP_OTHER_REDEF1; // expected-warning{{ambiguous expansion of macro 'TOP_OTHER_REDEF1'}}<br>
> +// expected-note@macros_top.h:19 {{expanding this definition}}<br>
> +// expected-note@macros_other.h:4 {{other definition}}<br>
> +<br>
> +#ifndef TOP_OTHER_REDEF2<br>
> +# error TOP_OTHER_REDEF2 should still be defined<br>
> +#endif<br>
> +int n2 = TOP_OTHER_REDEF2; // ok<br>
> +<br>
> +int n3 = TOP_OTHER_DEF_RIGHT_UNDEF; // ok<br>
><br>
> Added: cfe/trunk/test/PCH/macro-undef.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/macro-undef.cpp?rev=202560&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/macro-undef.cpp?rev=202560&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/PCH/macro-undef.cpp (added)<br>
> +++ cfe/trunk/test/PCH/macro-undef.cpp Fri Feb 28 18:08:04 2014<br>
> @@ -0,0 +1,36 @@<br>
> +// RUN: %clang_cc1 -emit-pch -o %t %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -include-pch %t %s -Wuninitialized -verify<br>
> +// RUN: %clang_cc1 -fsyntax-only -include-pch %t %s -Wuninitialized -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s<br>
> +<br>
> +#ifndef HEADER<br>
> +#define HEADER<br>
> +<br>
> +#define NULL 0<br>
> +template<typename T><br>
> +void *f() {<br>
> + void *p; // @11<br>
> + return p; // @12<br>
> +}<br>
> +#undef NULL<br>
> +template<typename T><br>
> +void *g() {<br>
> + void *p; // @17<br>
> + return p; // @18<br>
> +}<br>
> +<br>
> +#else<br>
> +<br>
> +// expected-warning@12 {{uninitialized}}<br>
> +// expected-note@11 {{initialize}}<br>
> +// CHECK: fix-it:"{{.*}}":{11:10-11:10}:" = NULL"<br>
> +<br>
> +// expected-warning@18 {{uninitialized}}<br>
> +// expected-note@17 {{initialize}}<br>
> +// CHECK: fix-it:"{{.*}}":{17:10-17:10}:" = 0"<br>
> +<br>
> +int main() {<br>
> + f<int>(); // expected-note {{instantiation}}<br>
> + g<int>(); // expected-note {{instantiation}}<br>
> +}<br>
> +<br>
> +#endif<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>
</blockquote></div><br></body></html>