[cfe-commits] r165746 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td include/clang/Lex/MacroInfo.h lib/Lex/MacroInfo.cpp lib/Lex/PPMacroExpansion.cpp lib/Serialization/ASTReader.cpp test/Modules
Matthieu Monrocq
matthieu.monrocq at gmail.com
Fri Oct 12 09:52:54 PDT 2012
On Thu, Oct 11, 2012 at 11:07 PM, Douglas Gregor <dgregor at apple.com> wrote:
> Author: dgregor
> Date: Thu Oct 11 16:07:39 2012
> New Revision: 165746
>
> URL: http://llvm.org/viewvc/llvm-project?rev=165746&view=rev
> Log:
> 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.
>
>
Would it make sense to diagnose an ambiguity even if the macros both expand
to the same sequence of tokens ?
I am concerned with the future actually. If I am linking in two modules and
they "work for now" but later on one of them change its macro definition
and not the other, I'll get the warning very late.
(I understand it's somewhat of a subjective opinion, but since there is a
precedent with ODR for inline functions...)
-- Matthieu
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> cfe/trunk/include/clang/Lex/MacroInfo.h
> cfe/trunk/lib/Lex/MacroInfo.cpp
> cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/test/Modules/Inputs/macros_left.h
> cfe/trunk/test/Modules/Inputs/macros_right.h
> cfe/trunk/test/Modules/Inputs/macros_top.h
> cfe/trunk/test/Modules/macros.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Oct 11 16:07:39
> 2012
> @@ -212,6 +212,7 @@
>
> // 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">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Thu Oct 11
> 16:07:39 2012
> @@ -228,6 +228,12 @@
> def warn_pp_undef_identifier : Warning<
> "%0 is not defined, evaluates to 0">,
> InGroup<DiagGroup<"undef">>, DefaultIgnore;
> +def warn_pp_ambiguous_macro : Warning<
> + "ambiguous expansion of macro %0">, InGroup<AmbiguousMacro>;
> +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 '\\'">;
>
> Modified: cfe/trunk/include/clang/Lex/MacroInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/MacroInfo.h (original)
> +++ cfe/trunk/include/clang/Lex/MacroInfo.h Thu Oct 11 16:07:39 2012
> @@ -111,6 +111,10 @@
> /// 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 @@
> /// \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;
> };
>
> Modified: cfe/trunk/lib/Lex/MacroInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/MacroInfo.cpp (original)
> +++ cfe/trunk/lib/Lex/MacroInfo.cpp Thu Oct 11 16:07:39 2012
> @@ -32,7 +32,8 @@
> IsAllowRedefinitionsWithoutWarning(false),
> IsWarnIfUnused(false),
> IsPublic(true),
> - IsHidden(false) {
> + IsHidden(false),
> + IsAmbiguous(false) {
> }
>
> MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator
> &PPAllocator)
> @@ -56,7 +57,8 @@
>
> IsAllowRedefinitionsWithoutWarning(MI.IsAllowRedefinitionsWithoutWarning),
> IsWarnIfUnused(MI.IsWarnIfUnused),
> IsPublic(MI.IsPublic),
> - IsHidden(MI.IsHidden) {
> + IsHidden(MI.IsHidden),
> + IsAmbiguous(MI.IsAmbiguous) {
> setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator);
> }
>
>
> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Thu Oct 11 16:07:39 2012
> @@ -89,15 +89,25 @@
> // 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 @@
> }
> }
> }
> -
> +
> + // 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
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Oct 11 16:07:39 2012
> @@ -1254,6 +1254,24 @@
> 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) {
> @@ -1370,8 +1388,9 @@
> }
> 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.
>
> Modified: cfe/trunk/test/Modules/Inputs/macros_left.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_left.h?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/macros_left.h (original)
> +++ cfe/trunk/test/Modules/Inputs/macros_left.h Thu Oct 11 16:07:39 2012
> @@ -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
>
> Modified: cfe/trunk/test/Modules/Inputs/macros_right.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_right.h?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/macros_right.h (original)
> +++ cfe/trunk/test/Modules/Inputs/macros_right.h Thu Oct 11 16:07:39 2012
> @@ -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
>
> Modified: cfe/trunk/test/Modules/Inputs/macros_top.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macros_top.h?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/macros_top.h (original)
> +++ cfe/trunk/test/Modules/Inputs/macros_top.h Thu Oct 11 16:07:39 2012
> @@ -2,4 +2,12 @@
>
> #define TOP_LEFT_UNDEF 1
>
> +
> +
> +
> +
> +
> +
> +
> +
> #define TOP_RIGHT_REDEF int
>
> Modified: cfe/trunk/test/Modules/macros.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros.c?rev=165746&r1=165745&r2=165746&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Modules/macros.c (original)
> +++ cfe/trunk/test/Modules/macros.c Thu Oct 11 16:07:39 2012
> @@ -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 @@
> 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;
> }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121012/6c8944d7/attachment.html>
More information about the cfe-commits
mailing list