]> granicus.if.org Git - clang/commitdiff
If a module A exports a macro M, and a module B imports that macro and #undef's
authorRichard Smith <richard-llvm@metafoo.co.uk>
Sat, 1 Mar 2014 00:08:04 +0000 (00:08 +0000)
committerRichard Smith <richard-llvm@metafoo.co.uk>
Sat, 1 Mar 2014 00:08:04 +0000 (00:08 +0000)
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).

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@202560 91177308-0d34-0410-b5e6-96231b3b80d8

17 files changed:
docs/Modules.rst
include/clang/Basic/Module.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/Module.h
lib/Lex/MacroInfo.cpp
lib/Lex/PPMacroExpansion.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/Inputs/macros_other.h
test/Modules/Inputs/macros_right.h
test/Modules/Inputs/macros_right_undef.h
test/Modules/Inputs/macros_top.h
test/Modules/Inputs/module.map
test/Modules/macros.c
test/Modules/macros2.c [new file with mode: 0644]
test/PCH/macro-undef.cpp [new file with mode: 0644]

index 8d7eaa6fc2b3a7bc6eab36388ec6f7ec813003b9..037add7074e948a2d60b6aa1f5891db59a29d190 100644 (file)
@@ -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
 ===================
 
index a57f21b5f25246ca49bcfedd289da8dff92a7452..a880671bdd2ccc1fd88b5bf9f3ace6752b175585 100644 (file)
@@ -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;
 
index e4c2119e7f899510cb2e163ed58ec5ec330222e7..706c1d8e33fa868b86c8ecd03e15ec1d4bf60b99 100644 (file)
@@ -64,6 +64,7 @@ class ASTUnit; // FIXME: Layering violation and egregious hack.
 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.
   ///
index 821c9cdfb513fde4213f1d2003ec2ba0c5a1b55b..ce4a7cb1c5b53f5bb6a64473d3f93c1e15be352f 100644 (file)
@@ -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.
index a7b55156c5d7653d93d59e12aa64e732aed405c3..0325a80c3bea110600ea726547f4fe2010a04d19 100644 (file)
@@ -144,7 +144,7 @@ MacroDirective::DefInfo MacroDirective::getDefinition() {
       isPublic = VisMD->isPublic();
   }
 
-  return DefInfo();
+  return DefInfo(0, UndefLoc, !isPublic.hasValue() || isPublic.getValue());
 }
 
 const MacroDirective::DefInfo
index 721760afa0ee7f92ea4392a7ff7dc81936f1d94d..84f1ea5b1faf1ffc20dc7ea109053c85276cd3e1 100644 (file)
@@ -293,11 +293,11 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     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;
     }
   }
 
index f93a52d7fdfa5ff69d7381e3bd8681618bc979db..c0da65d583bb95fafb055b77cdb6ac765b7d661b 100644 (file)
@@ -579,11 +579,34 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
     }
 
     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_key_ref key, const unsigned char *d,
   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(IdentifierInfo *II) {
     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(IdentifierInfo *II,
                               PMInfo.PCHMacroData.MacroDirectivesOffset);
     return;
   }
-  
-  // Module Macro.
 
-  GlobalMacroID GMacID = PMInfo.ModuleMacroData.GMacID;
-  SourceLocation ImportLoc =
-      SourceLocation::getFromRawEncoding(PMInfo.ModuleMacroData.ImportLoc);
+  // Module Macro.
 
-  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(MacroInfo *PrevMI, MacroInfo *NewMI,
   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];
 
-  DefMacroDirective *DefMD = cast<DefMacroDirective>(MD);
+    // 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);
+  }
+}
+
+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(Sema &S, ObjCMethodDecl *Method) {
 }
 
 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 *Mod,
       // 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)
index 18dbb053b99b1f4b234af7c7efdf693e9d714476..958e18183c04bfae23d3c6a423d3ef094a1e2f3d 100644 (file)
@@ -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);
         }
       }
     }
index 11b57407c29d519cc47a585e05be18abc88e56a9..df73af43050222e7d46b15690cd99adbd106d8a0 100644 (file)
@@ -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;
+      // 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;
       }
-
-      if (isa<UndefMacroDirective>(MD)) {
-        isUndefined = true;
-        continue;
-      }
-
-      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);
       }
     }
index ea686bfc558aa832c5883d58129155d184c2e602..4923a7fe56aa872161ad22384fc50be1d52a3f4a 100644 (file)
@@ -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
index dbbd2c364350339e9ff2b04614f6e7d40c8bc0f2..a70c3501e5c9f84aae2f410d4df2664940faccdc 100644 (file)
@@ -17,3 +17,5 @@
 #define TOP_RIGHT_REDEF float
 
 #define FN_ADD(x, y) (x+y)
+
+#undef TOP_OTHER_DEF_RIGHT_UNDEF
index 49473e36f0cbba1b430db5937eb0b5f6a367a7cb..15a83666a136909ceb0a5607483f7e101bb89233 100644 (file)
@@ -1 +1,4 @@
 #undef TOP_RIGHT_UNDEF
+
+@import macros_top;
+#undef TOP_OTHER_DEF_RIGHT_UNDEF
index dd303ffee4e1511fa914c98b73db57c54784205a..2955471a8fa0e1d25ed2dd5fea70a288f23a7c1f 100644 (file)
@@ -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
index c0cdf76cdc3874033b80f8c22f2830e02b83eb06..0ce16fbd3e74ec7140e378c65a4b8140ccae145f 100644 (file)
@@ -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" 
index 954181372a6092e31b60ae80fae330c89050720d..7a7e570ca256b3359fa37584425439754c6d1d5e 100644 (file)
 // FIXME: expected-note@Inputs/macros_left.h:11{{previous definition is here}}
 // FIXME: expected-note@Inputs/macros_right.h:12{{previous definition is here}}
 // expected-note@Inputs/macros_right.h:12{{expanding this definition of 'LEFT_RIGHT_DIFFERENT'}}
-// expected-note@Inputs/macros_top.h:13{{other definition of 'TOP_RIGHT_REDEF'}}
 // expected-note@Inputs/macros_right.h:13{{expanding this definition of 'LEFT_RIGHT_DIFFERENT2'}}
 // expected-note@Inputs/macros_left.h:14{{other definition of 'LEFT_RIGHT_DIFFERENT'}}
-// expected-note@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
+
+@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@macros_top.h:19 {{expanding this definition}}
+// expected-note@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
diff --git a/test/Modules/macros2.c b/test/Modules/macros2.c
new file mode 100644 (file)
index 0000000..87b4c97
--- /dev/null
@@ -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.
+
+@import macros_other;
+
+int n0 = TOP_OTHER_DEF_RIGHT_UNDEF; // ok
+
+@import macros_top;
+
+TOP_OTHER_DEF_RIGHT_UNDEF *n0b; // expected-warning{{ambiguous expansion of macro 'TOP_OTHER_DEF_RIGHT_UNDEF'}}
+// expected-note@macros_top.h:22 {{expanding this definition of 'TOP_OTHER_DEF_RIGHT_UNDEF'}}
+// expected-note@macros_other.h:6 {{other definition of 'TOP_OTHER_DEF_RIGHT_UNDEF'}}
+
+@import macros_right;
+@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@macros_left.h:14 {{expanding this}}
+  // expected-note@macros_right.h:12 {{other}}
+  LEFT_RIGHT_DIFFERENT2 *ip3 = &f; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT2}}
+  // expected-note@macros_left.h:11 {{expanding this}}
+  // expected-note@macros_right.h:13 {{other}}
+#undef LEFT_RIGHT_DIFFERENT3
+  int LEFT_RIGHT_DIFFERENT3;
+}
+
+@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@macros_top.h:19 {{expanding this definition}}
+// expected-note@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
diff --git a/test/PCH/macro-undef.cpp b/test/PCH/macro-undef.cpp
new file mode 100644 (file)
index 0000000..c0ce2de
--- /dev/null
@@ -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@12 {{uninitialized}}
+// expected-note@11 {{initialize}}
+// CHECK: fix-it:"{{.*}}":{11:10-11:10}:" = NULL"
+
+// expected-warning@18 {{uninitialized}}
+// expected-note@17 {{initialize}}
+// CHECK: fix-it:"{{.*}}":{17:10-17:10}:" = 0"
+
+int main() {
+  f<int>(); // expected-note {{instantiation}}
+  g<int>(); // expected-note {{instantiation}}
+}
+
+#endif