]> granicus.if.org Git - clang/commitdiff
[PCH/Modules] Change how macro [re]definitions are de/serialized.
authorArgyrios Kyrtzidis <akyrtzi@gmail.com>
Wed, 16 Jan 2013 16:19:38 +0000 (16:19 +0000)
committerArgyrios Kyrtzidis <akyrtzi@gmail.com>
Wed, 16 Jan 2013 16:19:38 +0000 (16:19 +0000)
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

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

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 [new file with mode: 0644]

index aeedd735e399fc313897d13e6ae547032f07be80..c6b44a9422aedeb11c41a0c1ab0483e58d644dbb 100644 (file)
@@ -169,6 +169,14 @@ public:
   /// \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,
index 1789e81b0a216004c34084a1166206860dd7a779..e7bd0dfeec1ad94aad03606f97d11edfd4f04b3b 100644 (file)
@@ -525,8 +525,7 @@ public:
   /// \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);
index 07ccab470696c5a175f784d84a0e19988be1a81c..1212887c94e7d8b27862a031214afad6cdbad377 100644 (file)
@@ -1535,7 +1535,7 @@ public:
                                                     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 @@ public:
   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();
index 7947522572a66765ef52652002ec2fbd7e61cbc3..7b8871576a67e6d94d7c660c26b6bb2d04dfb3a5 100644 (file)
@@ -55,11 +55,9 @@ void Preprocessor::setMacroInfo(IdentifierInfo *II, MacroInfo *MI) {
     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 @@ void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI,
     // 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 @@ void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI,
       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,
index 81d3cea7ba70d11339d3f1e2d5022cbef48cf5f3..5b1c5cd6e53141e27aeab6cef180030b711add8d 100644 (file)
@@ -525,13 +525,9 @@ IdentifierInfo *ASTIdentifierLookupTrait::ReadData(const internal_key_type& k,
   // 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 @@ bool ASTReader::ReadBlockAbbrevs(llvm::BitstreamCursor &Cursor,
   }
 }
 
-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 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
   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 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
       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 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset,
       }
       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 @@ HeaderFileInfoTrait::ReadData(const internal_key_type, const unsigned char *d,
   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 @@ IdentifierID ASTReader::getGlobalIdentifierID(ModuleFile &M, unsigned LocalID) {
   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 @@ MacroInfo *ASTReader::getMacro(MacroID ID, MacroInfo *Hint) {
     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 @@ void ASTReader::finishPendingActions() {
     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();
index af736a4048a87e2512ec585407f6fe633d7376eb..67e03c23346bc11c3b0d8314c0430e7b772121cc 100644 (file)
@@ -1796,12 +1796,10 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
   // 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 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
       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 @@ public:
     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 @@ public:
     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
diff --git a/test/PCH/macro-redef.c b/test/PCH/macro-redef.c
new file mode 100644 (file)
index 0000000..7e25d7f
--- /dev/null
@@ -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@-6 {{previous}}
+
+#else
+
+// Use the error to verify it was parsed.
+int x = M1; // expected-note {{previous}}
+int x = M2; // expected-error {{redefinition}}
+
+#endif