r202560 - If a module A exports a macro M, and a module B imports that macro and #undef's

Richard Smith richard at metafoo.co.uk
Thu Mar 20 17:41:37 PDT 2014


On Thu, Mar 20, 2014 at 3:40 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Thu, Mar 20, 2014 at 3:20 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>> Hi Richard,
>>
>> 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
>> http://llvm.org/PR19215.  Could you take a look?
>>
>
> Thanks for the heads-up, investigating...
>

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.


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


More information about the cfe-commits mailing list