[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
Douglas Gregor
dgregor at apple.com
Thu Jan 17 20:35:09 PST 2013
Unfortunately, I had to revert this (and its follow-on) in r172783, because it was causing hangs when building complicated modules.
- Doug
On Jan 16, 2013, at 8:19 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list