From 5968b1b71f880582dd9cc57e8f3669ba100ee9f9 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Thu, 11 Oct 2012 21:07:39 +0000 Subject: [PATCH] Diagnose the expansion of ambiguous macro definitions. This can happen only with modules, when two disjoint modules #define the same identifier to different token sequences. llvm-svn: 165746 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticLexKinds.td | 6 +++++ clang/include/clang/Lex/MacroInfo.h | 11 ++++++++ clang/lib/Lex/MacroInfo.cpp | 6 +++-- clang/lib/Lex/PPMacroExpansion.cpp | 36 +++++++++++++++++++++---- clang/lib/Serialization/ASTReader.cpp | 23 ++++++++++++++-- clang/test/Modules/Inputs/macros_left.h | 4 ++- clang/test/Modules/Inputs/macros_right.h | 6 +++-- clang/test/Modules/Inputs/macros_top.h | 8 ++++++ clang/test/Modules/macros.c | 14 +++++++--- 10 files changed, 99 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 00a0dc7..4b0f4f1 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -212,6 +212,7 @@ def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">; // Preprocessor warnings. def : DiagGroup<"builtin-macro-redefined">; +def AmbiguousMacro : DiagGroup<"ambiguous-macro">; // Just silence warnings about -Wstrict-aliasing for now. def : DiagGroup<"strict-aliasing=0">; diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index fbdf56c..1652236 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -228,6 +228,12 @@ def pp_macro_not_used : Warning<"macro is not used">, DefaultIgnore, def warn_pp_undef_identifier : Warning< "%0 is not defined, evaluates to 0">, InGroup>, DefaultIgnore; +def warn_pp_ambiguous_macro : Warning< + "ambiguous expansion of macro %0">, InGroup; +def note_pp_ambiguous_macro_chosen : Note< + "expanding this definition of %0">; +def note_pp_ambiguous_macro_other : Note< + "other definition of %0">; def pp_invalid_string_literal : Warning< "invalid string literal, ignoring final '\\'">; diff --git a/clang/include/clang/Lex/MacroInfo.h b/clang/include/clang/Lex/MacroInfo.h index a7bcfb9..aba77d5 100644 --- a/clang/include/clang/Lex/MacroInfo.h +++ b/clang/include/clang/Lex/MacroInfo.h @@ -111,6 +111,10 @@ private: /// file. bool IsHidden : 1; + /// \brief Whether the definition of this macro is ambiguous, due to + /// multiple definitions coming in from multiple modules. + bool IsAmbiguous : 1; + ~MacroInfo() { assert(ArgumentList == 0 && "Didn't call destroy before dtor!"); } @@ -340,6 +344,13 @@ public: /// \brief Set whether this macro definition is hidden. void setHidden(bool Val) { IsHidden = Val; } + /// \brief Determine whether this macro definition is ambiguous with + /// other macro definitions. + bool isAmbiguous() const { return IsAmbiguous; } + + /// \brief Set whether this macro definition is ambiguous. + void setAmbiguous(bool Val) { IsAmbiguous = Val; } + private: unsigned getDefinitionLengthSlow(SourceManager &SM) const; }; diff --git a/clang/lib/Lex/MacroInfo.cpp b/clang/lib/Lex/MacroInfo.cpp index 7e538fa..904f04e 100644 --- a/clang/lib/Lex/MacroInfo.cpp +++ b/clang/lib/Lex/MacroInfo.cpp @@ -32,7 +32,8 @@ MacroInfo::MacroInfo(SourceLocation DefLoc) IsAllowRedefinitionsWithoutWarning(false), IsWarnIfUnused(false), IsPublic(true), - IsHidden(false) { + IsHidden(false), + IsAmbiguous(false) { } MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator) @@ -56,7 +57,8 @@ MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator) IsAllowRedefinitionsWithoutWarning(MI.IsAllowRedefinitionsWithoutWarning), IsWarnIfUnused(MI.IsWarnIfUnused), IsPublic(MI.IsPublic), - IsHidden(MI.IsHidden) { + IsHidden(MI.IsHidden), + IsAmbiguous(MI.IsAmbiguous) { setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator); } diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index c717816..dc3a456 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -89,15 +89,25 @@ void Preprocessor::addLoadedMacroInfo(IdentifierInfo *II, MacroInfo *MI, // Find the end of the definition chain. MacroInfo *Prev = StoredMI; MacroInfo *PrevPrev; - bool Ambiguous = false; + bool Ambiguous = StoredMI->isAmbiguous(); + bool MatchedOther = false; do { // If the macros are not identical, we have an ambiguity. - if (!Prev->isIdenticalTo(*MI, *this)) - Ambiguous = true; + if (!Prev->isIdenticalTo(*MI, *this)) { + if (!Ambiguous) { + Ambiguous = true; + StoredMI->setAmbiguous(true); + } + } else { + MatchedOther = true; + } } while ((PrevPrev = Prev->getPreviousDefinition()) && PrevPrev->isDefined()); - // FIXME: Actually use the ambiguity information for something. + // If there are ambiguous definitions, and we didn't match any other + // definition, then mark us as ambiguous. + if (Ambiguous && !MatchedOther) + MI->setAmbiguous(true); // Wire this macro information into the chain. MI->setPreviousDefinition(Prev->getPreviousDefinition()); @@ -360,7 +370,23 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier, } } } - + + // If the macro definition is ambiguous, complain. + if (MI->isAmbiguous()) { + Diag(Identifier, diag::warn_pp_ambiguous_macro) + << Identifier.getIdentifierInfo(); + Diag(MI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_chosen) + << Identifier.getIdentifierInfo(); + for (MacroInfo *PrevMI = MI->getPreviousDefinition(); + PrevMI && PrevMI->isDefined(); + PrevMI = PrevMI->getPreviousDefinition()) { + if (PrevMI->isAmbiguous()) { + Diag(PrevMI->getDefinitionLoc(), diag::note_pp_ambiguous_macro_other) + << Identifier.getIdentifierInfo(); + } + } + } + // If we started lexing a macro, enter the macro expansion body. // If this macro expands to no tokens, don't bother to push it onto the diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 5657ee2..8f18eb8 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -1254,6 +1254,24 @@ 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) { @@ -1370,8 +1388,9 @@ void ASTReader::ReadMacroRecord(ModuleFile &F, uint64_t Offset, } MI->setHidden(Hidden); - // Finally, install the macro. - PP.addLoadedMacroInfo(II, MI, Hint); + // 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. diff --git a/clang/test/Modules/Inputs/macros_left.h b/clang/test/Modules/Inputs/macros_left.h index 919dc20..a2f287e 100644 --- a/clang/test/Modules/Inputs/macros_left.h +++ b/clang/test/Modules/Inputs/macros_left.h @@ -7,6 +7,8 @@ #define LEFT_RIGHT_IDENTICAL int -#define LEFT_RIGHT_DIFFERENT float + #define LEFT_RIGHT_DIFFERENT2 float #define LEFT_RIGHT_DIFFERENT3 float + +#define LEFT_RIGHT_DIFFERENT float diff --git a/clang/test/Modules/Inputs/macros_right.h b/clang/test/Modules/Inputs/macros_right.h index 8e96882..a4a1e2e 100644 --- a/clang/test/Modules/Inputs/macros_right.h +++ b/clang/test/Modules/Inputs/macros_right.h @@ -1,8 +1,8 @@ #include "macros_top.h" #define RIGHT unsigned short -#undef TOP_RIGHT_REDEF -#define TOP_RIGHT_REDEF float + + @@ -13,3 +13,5 @@ #define LEFT_RIGHT_DIFFERENT2 int #define LEFT_RIGHT_DIFFERENT3 int +#undef TOP_RIGHT_REDEF +#define TOP_RIGHT_REDEF float diff --git a/clang/test/Modules/Inputs/macros_top.h b/clang/test/Modules/Inputs/macros_top.h index 1a21848..5264cea 100644 --- a/clang/test/Modules/Inputs/macros_top.h +++ b/clang/test/Modules/Inputs/macros_top.h @@ -2,4 +2,12 @@ #define TOP_LEFT_UNDEF 1 + + + + + + + + #define TOP_RIGHT_REDEF int diff --git a/clang/test/Modules/macros.c b/clang/test/Modules/macros.c index 168e2c6..8f6a20a 100644 --- a/clang/test/Modules/macros.c +++ b/clang/test/Modules/macros.c @@ -7,8 +7,14 @@ // RUN: %clang_cc1 -E -fmodules -x objective-c -fmodule-cache-path %t %s | FileCheck -check-prefix CHECK-PREPROCESSED %s // FIXME: When we have a syntax for modules in C, use that. // These notes come from headers in modules, and are bogus. + // FIXME: expected-note{{previous definition is here}} -// FIXME: expected-note{{previous definition is here}} +// expected-note{{other definition of 'LEFT_RIGHT_DIFFERENT'}} +// expected-note{{expanding this definition of 'TOP_RIGHT_REDEF'}} +// FIXME: expected-note{{previous definition is here}} \ +// expected-note{{expanding this definition of 'LEFT_RIGHT_DIFFERENT'}} + +// expected-note{{other definition of 'TOP_RIGHT_REDEF'}} @__experimental_modules_import macros; @@ -109,11 +115,11 @@ void test2() { int i; float f; double d; - TOP_RIGHT_REDEF *ip = &i; // FIXME: warning + TOP_RIGHT_REDEF *ip = &i; // expected-warning{{ambiguous expansion of macro 'TOP_RIGHT_REDEF'}} LEFT_RIGHT_IDENTICAL *ip2 = &i; - LEFT_RIGHT_DIFFERENT *fp = &f; // FIXME: warning - LEFT_RIGHT_DIFFERENT2 *dp = &d; // okay + LEFT_RIGHT_DIFFERENT *fp = &f; // expected-warning{{ambiguous expansion of macro 'LEFT_RIGHT_DIFFERENT'}} + LEFT_RIGHT_DIFFERENT2 *dp = &d; int LEFT_RIGHT_DIFFERENT3; } -- 2.7.4