From: Argyrios Kyrtzidis Date: Wed, 16 Jan 2013 16:19:38 +0000 (+0000) Subject: [PCH/Modules] Change how macro [re]definitions are de/serialized. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7f69b1ab4f63ded511dddd7fb7e1cd8860757016;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172620 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Lex/MacroInfo.h b/include/clang/Lex/MacroInfo.h index aeedd735e3..c6b44a9422 100644 --- a/include/clang/Lex/MacroInfo.h +++ b/include/clang/Lex/MacroInfo.h @@ -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, diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 1789e81b0a..e7bd0dfeec 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -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); diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 07ccab4706..1212887c94 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -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 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(); diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp index 7947522572..7b8871576a 100644 --- a/lib/Lex/PPMacroExpansion.cpp +++ b/lib/Lex/PPMacroExpansion.cpp @@ -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, diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 81d3cea7ba..5b1c5cd6e5 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -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 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 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 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 &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 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 &MacroIDs = PMIt->second; + for (SmallVectorImpl::iterator + MIt = MacroIDs.begin(), ME = MacroIDs.end(); MIt != ME; ++MIt) { + MacroInfo *MI = getMacro(*MIt); + PP.addLoadedMacroInfo(PMIt->first, MI); } } PendingMacroIDs.clear(); diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index af736a4048..67e03c2334 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -1796,12 +1796,10 @@ void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) { // Construct the list of macro definitions that need to be serialized. SmallVector, 2> MacrosToEmit; - llvm::SmallPtrSet 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 index 0000000000..7e25d7f5a2 --- /dev/null +++ b/test/PCH/macro-redef.c @@ -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