[cfe-commits] r172620 - in /cfe/trunk: include/clang/Lex/MacroInfo.h include/clang/Lex/Preprocessor.h include/clang/Serialization/ASTReader.h lib/Lex/PPMacroExpansion.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/PCH/macro-redef.c

Argyrios Kyrtzidis akyrtzi at gmail.com
Wed Jan 16 08:19:39 PST 2013


Author: akirtzidis
Date: Wed Jan 16 10:19:38 2013
New Revision: 172620

URL: http://llvm.org/viewvc/llvm-project?rev=172620&view=rev
Log:
[PCH/Modules] Change how macro [re]definitions are de/serialized.

Previously we would serialize the macro redefinitions as a list, part of
the identifier, and try to chain them together across modules individually
without having the info that they were already chained at definition time.

Change this by serializing the macro redefinition chain and then try
to synthesize the chain parts across modules. This allows us to correctly
pinpoint when 2 different definitions are ambiguous because they came from
unrelated modules.

Fixes bogus "ambiguous expansion of macro" warning when a macro in a PCH
is redefined without undef'ing it first.

rdar://13016031

Added:
    cfe/trunk/test/PCH/macro-redef.c
Modified:
    cfe/trunk/include/clang/Lex/MacroInfo.h
    cfe/trunk/include/clang/Lex/Preprocessor.h
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Lex/PPMacroExpansion.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Lex/MacroInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/MacroInfo.h (original)
+++ cfe/trunk/include/clang/Lex/MacroInfo.h Wed Jan 16 10:19:38 2013
@@ -169,6 +169,14 @@
   /// \brief Get previous definition of the macro with the same name.
   MacroInfo *getPreviousDefinition() { return PreviousDefinition; }
 
+  /// \brief Get the first definition in the chain.
+  MacroInfo *getFirstDefinition() {
+    MacroInfo *MI = this;
+    while (MI->PreviousDefinition)
+      MI = MI->PreviousDefinition;
+    return MI;
+  }
+
   /// \brief Find macro definition active in the specified source location. If
   /// this macro was not defined there, return NULL.
   const MacroInfo *findDefinitionAtLoc(SourceLocation L,

Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Wed Jan 16 10:19:38 2013
@@ -525,8 +525,7 @@
   /// \brief Specify a macro for this identifier.
   void setMacroInfo(IdentifierInfo *II, MacroInfo *MI);
   /// \brief Add a MacroInfo that was loaded from an AST file.
-  void addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI,
-                          MacroInfo *Hint = 0);
+  void addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI);
   /// \brief Make the given MacroInfo, that was loaded from an AST file and
   /// previously hidden, visible.
   void makeLoadedMacroInfoVisible(IdentifierInfo *II, MacroInfo *MI);

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Jan 16 10:19:38 2013
@@ -1535,7 +1535,7 @@
                                                     unsigned LocalID);
 
   /// \brief Retrieve the macro with the given ID.
-  MacroInfo *getMacro(serialization::MacroID ID, MacroInfo *Hint = 0);
+  MacroInfo *getMacro(serialization::MacroID ID);
 
   /// \brief Retrieve the global macro ID corresponding to the given local
   /// ID within the given module file.
@@ -1693,20 +1693,19 @@
   Expr *ReadSubExpr();
 
   /// \brief Reads the macro record located at the given offset.
-  void ReadMacroRecord(ModuleFile &F, uint64_t Offset, MacroInfo *Hint = 0);
+  void ReadMacroRecord(ModuleFile &F, uint64_t Offset);
 
   /// \brief Determine the global preprocessed entity ID that corresponds to
   /// the given local ID within the given module.
   serialization::PreprocessedEntityID
   getGlobalPreprocessedEntityID(ModuleFile &M, unsigned LocalID) const;
 
-  /// \brief Note that the identifier has a macro history.
+  /// \brief Add the macro ID assosiated with \c II for deserialization.
   ///
   /// \param II The name of the macro.
-  ///
-  /// \param IDs The global macro IDs that are associated with this identifier.
-  void setIdentifierIsMacro(IdentifierInfo *II,
-                            ArrayRef<serialization::MacroID> IDs);
+  /// \param ID The global macro ID that is associated with this identifier.
+  void addMacroIDForDeserialization(IdentifierInfo *II,
+                                    serialization::MacroID ID);
 
   /// \brief Read the set of macros defined by this external macro source.
   virtual void ReadDefinedMacros();

Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Wed Jan 16 10:19:38 2013
@@ -55,11 +55,9 @@
     II->setChangedSinceDeserialization();
 }
 
-void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI,
-                                      MacroInfo *Hint) {
+void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI) {
   assert(MI && "Missing macro?");
   assert(MI->isFromAST() && "Macro is not from an AST?");
-  assert(!MI->getPreviousDefinition() && "Macro already in chain?");
   
   MacroInfo *&StoredMI = Macros[II];
 
@@ -79,7 +77,7 @@
     // Simple case: if this is the first actual definition, just put it at
     // th beginning.
     if (!StoredMI->isDefined()) {
-      MI->setPreviousDefinition(StoredMI);
+      MI->getFirstDefinition()->setPreviousDefinition(StoredMI);
       StoredMI = MI;
 
       II->setHasMacroDefinition(true);
@@ -112,16 +110,14 @@
       MI->setAmbiguous(true);
 
     // Wire this macro information into the chain.
-    MI->setPreviousDefinition(Prev->getPreviousDefinition());
+    MI->getFirstDefinition()->setPreviousDefinition(
+                                                 Prev->getPreviousDefinition());
     Prev->setPreviousDefinition(MI);
     return;
   }
 
   // The macro is not a definition; put it at the end of the list.
-  MacroInfo *Prev = Hint? Hint : StoredMI;
-  while (Prev->getPreviousDefinition())
-    Prev = Prev->getPreviousDefinition();
-  Prev->setPreviousDefinition(MI);
+  StoredMI->getFirstDefinition()->setPreviousDefinition(MI);
 }
 
 void Preprocessor::makeLoadedMacroInfoVisible(IdentifierInfo *II,

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jan 16 10:19:38 2013
@@ -525,13 +525,9 @@
   // If this identifier is a macro, deserialize the macro
   // definition.
   if (hadMacroDefinition) {
-    SmallVector<MacroID, 4> MacroIDs;
-    while (uint32_t LocalID = ReadUnalignedLE32(d)) {
-      MacroIDs.push_back(Reader.getGlobalMacroID(F, LocalID));
-      DataLen -= 4;
-    }
     DataLen -= 4;
-    Reader.setIdentifierIsMacro(II, MacroIDs);
+    uint32_t LocalID = ReadUnalignedLE32(d);
+    Reader.addMacroIDForDeserialization(II, Reader.getGlobalMacroID(F,LocalID));
   }
 
   Reader.SetIdentifierInfo(ID, II);
@@ -1061,8 +1057,7 @@
   }
 }
 
-void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
-                                MacroInfo *Hint) {
+void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset) {
   llvm::BitstreamCursor &Stream = F.MacroCursor;
 
   // Keep track of where we are in the stream, then jump back there
@@ -1074,24 +1069,6 @@
   SmallVector<IdentifierInfo*, 16> MacroArgs;
   MacroInfo *Macro = 0;
 
-  // RAII object to add the loaded macro information once we're done
-  // adding tokens.
-  struct AddLoadedMacroInfoRAII {
-    Preprocessor &PP;
-    MacroInfo *Hint;
-    MacroInfo *MI;
-    IdentifierInfo *II;
-
-    AddLoadedMacroInfoRAII(Preprocessor &PP, MacroInfo *Hint)
-      : PP(PP), Hint(Hint), MI(), II() { }
-    ~AddLoadedMacroInfoRAII( ) {
-      if (MI) {
-        // Finally, install the macro.
-        PP.addLoadedMacroInfo(II, MI, Hint);
-      }
-    }
-  } AddLoadedMacroInfo(PP, Hint);
-
   while (true) {
     unsigned Code = Stream.ReadCode();
     switch (Code) {
@@ -1146,6 +1123,8 @@
       SourceLocation Loc = ReadSourceLocation(F, Record, NextIndex);
       MacroInfo *MI = PP.AllocateMacroInfo(Loc);
       MI->setDefinitionEndLoc(ReadSourceLocation(F, Record, NextIndex));
+      MacroInfo *PrevMI = getMacro(getGlobalMacroID(F, Record[NextIndex++]));
+      MI->setPreviousDefinition(PrevMI);
 
       // Record this macro.
       MacrosLoaded[GlobalID - NUM_PREDEF_MACRO_IDS] = MI;
@@ -1230,10 +1209,6 @@
       }
       MI->setHidden(Hidden);
 
-      // Make sure we install the macro once we're done.
-      AddLoadedMacroInfo.MI = MI;
-      AddLoadedMacroInfo.II = II;
-
       // Remember that we saw this macro last so that we add the tokens that
       // form its body to it.
       Macro = MI;
@@ -1341,10 +1316,13 @@
   return HFI;
 }
 
-void ASTReader::setIdentifierIsMacro(IdentifierInfo *II, ArrayRef<MacroID> IDs){
+void ASTReader::addMacroIDForDeserialization(IdentifierInfo *II, MacroID ID){
   II->setHadMacroDefinition(true);
   assert(NumCurrentElementsDeserializing > 0 &&"Missing deserialization guard");
-  PendingMacroIDs[II].append(IDs.begin(), IDs.end());
+  SmallVector<serialization::MacroID, 2> &MacroIDs = PendingMacroIDs[II];
+  assert(std::find(MacroIDs.begin(), MacroIDs.end(), ID) == MacroIDs.end() &&
+         "Already added the macro ID for deserialization");
+  MacroIDs.push_back(ID);
 }
 
 void ASTReader::ReadDefinedMacros() {
@@ -6158,7 +6136,7 @@
   return LocalID + I->second;
 }
 
-MacroInfo *ASTReader::getMacro(MacroID ID, MacroInfo *Hint) {
+MacroInfo *ASTReader::getMacro(MacroID ID) {
   if (ID == 0)
     return 0;
 
@@ -6174,7 +6152,7 @@
     assert(I != GlobalMacroMap.end() && "Corrupted global macro map");
     ModuleFile *M = I->second;
     unsigned Index = ID - M->BaseMacroID;
-    ReadMacroRecord(*M, M->MacroOffsets[Index], Hint);
+    ReadMacroRecord(*M, M->MacroOffsets[Index]);
   }
 
   return MacrosLoaded[ID];
@@ -6873,13 +6851,14 @@
     PendingDeclChains.clear();
 
     // Load any pending macro definitions.
+    // Note that new macros may be added while deserializing a macro.
     for (unsigned I = 0; I != PendingMacroIDs.size(); ++I) {
-      // FIXME: std::move here
-      SmallVector<MacroID, 2> GlobalIDs = PendingMacroIDs.begin()[I].second;
-      MacroInfo *Hint = 0;
-      for (unsigned IDIdx = 0, NumIDs = GlobalIDs.size(); IDIdx !=  NumIDs;
-           ++IDIdx) {
-        Hint = getMacro(GlobalIDs[IDIdx], Hint);
+      PendingMacroIDsMap::iterator PMIt = PendingMacroIDs.begin() + I;
+      SmallVector<serialization::MacroID, 2> &MacroIDs = PMIt->second;
+      for (SmallVectorImpl<serialization::MacroID>::iterator
+             MIt = MacroIDs.begin(), ME = MacroIDs.end(); MIt != ME; ++MIt) {
+        MacroInfo *MI = getMacro(*MIt);
+        PP.addLoadedMacroInfo(PMIt->first, MI);
       }
     }
     PendingMacroIDs.clear();

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=172620&r1=172619&r2=172620&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Jan 16 10:19:38 2013
@@ -1796,12 +1796,10 @@
   // Construct the list of macro definitions that need to be serialized.
   SmallVector<std::pair<const IdentifierInfo *, MacroInfo *>, 2> 
     MacrosToEmit;
-  llvm::SmallPtrSet<const IdentifierInfo*, 4> MacroDefinitionsSeen;
   for (Preprocessor::macro_iterator I = PP.macro_begin(Chain == 0),
                                     E = PP.macro_end(Chain == 0);
        I != E; ++I) {
     if (!IsModule || I->second->isPublic()) {
-      MacroDefinitionsSeen.insert(I->first);
       MacrosToEmit.push_back(std::make_pair(I->first, I->second));
     }
   }
@@ -1854,6 +1852,12 @@
       Record.push_back(inferSubmoduleIDFromLocation(MI->getDefinitionLoc()));
       AddSourceLocation(MI->getDefinitionLoc(), Record);
       AddSourceLocation(MI->getDefinitionEndLoc(), Record);
+      MacroInfo *PrevMI = MI->getPreviousDefinition();
+      // Serialize only the part of the definition chain that is local.
+      // The chain will be synthesized across modules by the ASTReader.
+      if (Chain && PrevMI && PrevMI->isFromAST())
+        PrevMI = 0;
+      addMacroRef(PrevMI, Record);
       AddSourceLocation(MI->getUndefLoc(), Record);
       Record.push_back(MI->isUsed());
       Record.push_back(MI->isPublic());
@@ -2735,14 +2739,8 @@
     if (isInterestingIdentifier(II, Macro)) {
       DataLen += 2; // 2 bytes for builtin ID
       DataLen += 2; // 2 bytes for flags
-      if (hadMacroDefinition(II, Macro)) {
-        for (MacroInfo *M = Macro; M; M = M->getPreviousDefinition()) {
-          if (Writer.getMacroRef(M) != 0)
-            DataLen += 4;
-        }
-
+      if (hadMacroDefinition(II, Macro))
         DataLen += 4;
-      }
 
       for (IdentifierResolver::iterator D = IdResolver.begin(II),
                                      DEnd = IdResolver.end();
@@ -2787,13 +2785,8 @@
     clang::io::Emit16(Out, Bits);
 
     if (HadMacroDefinition) {
-      // Write all of the macro IDs associated with this identifier.
-      for (MacroInfo *M = Macro; M; M = M->getPreviousDefinition()) {
-        if (MacroID ID = Writer.getMacroRef(M))
-          clang::io::Emit32(Out, ID);
-      }
-
-      clang::io::Emit32(Out, 0);
+      // Write the macro ID associated with this identifier.
+      clang::io::Emit32(Out, Writer.getMacroRef(Macro));
     }
 
     // Emit the declaration IDs in reverse order, because the

Added: cfe/trunk/test/PCH/macro-redef.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/macro-redef.c?rev=172620&view=auto
==============================================================================
--- cfe/trunk/test/PCH/macro-redef.c (added)
+++ cfe/trunk/test/PCH/macro-redef.c Wed Jan 16 10:19:38 2013
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 %s -emit-pch -o %t1.pch -verify
+// RUN: %clang_cc1 %s -emit-pch -o %t2.pch -include-pch %t1.pch -verify
+// RUN: %clang_cc1 -fsyntax-only %s -include-pch %t2.pch -verify
+
+// Test that a redefinition inside the PCH won't manifest as an ambiguous macro.
+// rdar://13016031
+
+#ifndef HEADER1
+#define HEADER1
+
+#define M1 0 // expected-note {{previous}}
+#define M1 1 // expected-warning {{redefined}}
+
+#define M2 3
+
+#elif !defined(HEADER2)
+#define HEADER2
+
+#define M2 4 // expected-warning {{redefined}}
+ // expected-note at -6 {{previous}}
+
+#else
+
+// Use the error to verify it was parsed.
+int x = M1; // expected-note {{previous}}
+int x = M2; // expected-error {{redefinition}}
+
+#endif





More information about the cfe-commits mailing list