r178671 - [preprocessor] Allow comparing two macro definitions syntactically instead of only lexically.

Argyrios Kyrtzidis akyrtzi at gmail.com
Wed Apr 3 14:20:31 PDT 2013


On Apr 3, 2013, at 12:44 PM, Reid Kleckner <rnk at google.com> wrote:

> Nice!  I've also observed redefinition warnings from clang in various winsock headers that might require more work.
> 
> I see things like:
> #define SOME_CONSTANT (ULONG)0xffff
> ...
> #define SOME_CONSTANT (u_long)0xffff
> 
> We may want to avoid warning on that.  Or I could adjust my flags so that the winsock headers become "system" headers and the warning is suppressed.

Does MSVC not warn for these ?

> 
> 
> On Wed, Apr 3, 2013 at 1:39 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> Author: akirtzidis
> Date: Wed Apr  3 12:39:30 2013
> New Revision: 178671
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=178671&view=rev
> Log:
> [preprocessor] Allow comparing two macro definitions syntactically instead of only lexically.
> 
> Syntactically means the function macro parameter names do not need to use the same
> identifiers in order for the definitions to be considered identical.
> 
> Syntactic equivalence is a microsoft extension for macro redefinitions and we'll also
> use this kind of comparison to check for ambiguous macros coming from modules.
> 
> rdar://13562254
> 
> Modified:
>     cfe/trunk/include/clang/Lex/MacroInfo.h
>     cfe/trunk/lib/Frontend/CompilerInstance.cpp
>     cfe/trunk/lib/Lex/MacroInfo.cpp
>     cfe/trunk/lib/Lex/PPDirectives.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/macros.c
>     cfe/trunk/test/Preprocessor/macro_misc.c
> 
> Modified: cfe/trunk/include/clang/Lex/MacroInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/MacroInfo.h (original)
> +++ cfe/trunk/include/clang/Lex/MacroInfo.h Wed Apr  3 12:39:30 2013
> @@ -145,9 +145,12 @@ public:
>    /// \brief Return true if the specified macro definition is equal to
>    /// this macro in spelling, arguments, and whitespace.
>    ///
> -  /// This is used to emit duplicate definition warnings.  This implements the rules
> -  /// in C99 6.10.3.
> -  bool isIdenticalTo(const MacroInfo &Other, Preprocessor &PP) const;
> +  /// \param Syntactically if true, the macro definitions can be identical even
> +  /// if they use different identifiers for the function macro parameters.
> +  /// Otherwise the comparison is lexical and this implements the rules in
> +  /// C99 6.10.3.
> +  bool isIdenticalTo(const MacroInfo &Other, Preprocessor &PP,
> +                     bool Syntactically) const;
> 
>    /// \brief Set or clear the isBuiltinMacro flag.
>    void setIsBuiltinMacro(bool Val = true) {
> 
> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Apr  3 12:39:30 2013
> @@ -991,7 +991,8 @@ static void checkConfigMacro(Preprocesso
>    // If the current macro definition is the same as the predefined macro
>    // definition, it's okay.
>    if (LatestDef.getMacroInfo() == PredefinedDef.getMacroInfo() ||
> -      LatestDef.getMacroInfo()->isIdenticalTo(*PredefinedDef.getMacroInfo(),PP))
> +      LatestDef.getMacroInfo()->isIdenticalTo(*PredefinedDef.getMacroInfo(),PP,
> +                                              /*Syntactically=*/true))
>      return;
> 
>    // The macro definitions differ.
> 
> Modified: cfe/trunk/lib/Lex/MacroInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/MacroInfo.cpp (original)
> +++ cfe/trunk/lib/Lex/MacroInfo.cpp Wed Apr  3 12:39:30 2013
> @@ -61,11 +61,17 @@ unsigned MacroInfo::getDefinitionLengthS
>    return DefinitionLength;
>  }
> 
> -// isIdenticalTo - Return true if the specified macro definition is equal to
> -// this macro in spelling, arguments, and whitespace.  This is used to emit
> -// duplicate definition warnings.  This implements the rules in C99 6.10.3.
> -//
> -bool MacroInfo::isIdenticalTo(const MacroInfo &Other, Preprocessor &PP) const {
> +/// \brief Return true if the specified macro definition is equal to
> +/// this macro in spelling, arguments, and whitespace.
> +///
> +/// \param Syntactically if true, the macro definitions can be identical even
> +/// if they use different identifiers for the function macro parameters.
> +/// Otherwise the comparison is lexical and this implements the rules in
> +/// C99 6.10.3.
> +bool MacroInfo::isIdenticalTo(const MacroInfo &Other, Preprocessor &PP,
> +                              bool Syntactically) const {
> +  bool Lexically = !Syntactically;
> +
>    // Check # tokens in replacement, number of args, and various flags all match.
>    if (ReplacementTokens.size() != Other.ReplacementTokens.size() ||
>        getNumArgs() != Other.getNumArgs() ||
> @@ -74,10 +80,12 @@ bool MacroInfo::isIdenticalTo(const Macr
>        isGNUVarargs() != Other.isGNUVarargs())
>      return false;
> 
> -  // Check arguments.
> -  for (arg_iterator I = arg_begin(), OI = Other.arg_begin(), E = arg_end();
> -       I != E; ++I, ++OI)
> -    if (*I != *OI) return false;
> +  if (Lexically) {
> +    // Check arguments.
> +    for (arg_iterator I = arg_begin(), OI = Other.arg_begin(), E = arg_end();
> +         I != E; ++I, ++OI)
> +      if (*I != *OI) return false;
> +  }
> 
>    // Check all the tokens.
>    for (unsigned i = 0, e = ReplacementTokens.size(); i != e; ++i) {
> @@ -95,7 +103,15 @@ bool MacroInfo::isIdenticalTo(const Macr
> 
>      // If this is an identifier, it is easy.
>      if (A.getIdentifierInfo() || B.getIdentifierInfo()) {
> -      if (A.getIdentifierInfo() != B.getIdentifierInfo())
> +      if (A.getIdentifierInfo() == B.getIdentifierInfo())
> +        continue;
> +      if (Lexically)
> +        return false;
> +      // With syntactic equivalence the parameter names can be different as long
> +      // as they are used in the same place.
> +      int AArgNum = getArgumentNum(A.getIdentifierInfo());
> +      int BArgNum = Other.getArgumentNum(B.getIdentifierInfo());
> +      if (AArgNum == -1 || AArgNum != BArgNum)
>          return false;
>        continue;
>      }
> 
> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Wed Apr  3 12:39:30 2013
> @@ -1949,9 +1949,9 @@ void Preprocessor::HandleDefineDirective
>        if (OtherMI->isBuiltinMacro())
>          Diag(MacroNameTok, diag::ext_pp_redef_builtin_macro);
>        // Macros must be identical.  This means all tokens and whitespace
> -      // separation must be the same.  C99 6.10.3.2.
> +      // separation must be the same.  C99 6.10.3p2.
>        else if (!OtherMI->isAllowRedefinitionsWithoutWarning() &&
> -               !MI->isIdenticalTo(*OtherMI, *this)) {
> +               !MI->isIdenticalTo(*OtherMI, *this, /*Syntactic=*/LangOpts.MicrosoftExt)) {
>          Diag(MI->getDefinitionLoc(), diag::ext_pp_macro_redef)
>            << MacroNameTok.getIdentifierInfo();
>          Diag(OtherMI->getDefinitionLoc(), diag::note_previous_definition);
> 
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Apr  3 12:39:30 2013
> @@ -1604,7 +1604,8 @@ void ASTReader::installImportedMacro(Ide
>      MacroDirective::DefInfo PrevDef = Prev->getDefinition();
>      MacroInfo *PrevMI = PrevDef.getMacroInfo();
>      MacroInfo *NewMI = DefMD->getInfo();
> -    if (NewMI != PrevMI && !PrevMI->isIdenticalTo(*NewMI, PP)) {
> +    if (NewMI != PrevMI && !PrevMI->isIdenticalTo(*NewMI, PP,
> +                                                  /*Syntactically=*/true)) {
>        // Before marking the macros as ambiguous, check if this is a case where
>        // the system macro uses a not identical definition compared to a macro
>        // from the clang headers. For example:
> 
> 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=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/macros_left.h (original)
> +++ cfe/trunk/test/Modules/Inputs/macros_left.h Wed Apr  3 12:39:30 2013
> @@ -12,3 +12,5 @@
>  #define LEFT_RIGHT_DIFFERENT3 float
> 
>  #define LEFT_RIGHT_DIFFERENT float
> +
> +#define FN_ADD(a,b) (a+b)
> 
> 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=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/macros_right.h (original)
> +++ cfe/trunk/test/Modules/Inputs/macros_right.h Wed Apr  3 12:39:30 2013
> @@ -15,3 +15,5 @@
> 
>  #undef TOP_RIGHT_REDEF
>  #define TOP_RIGHT_REDEF float
> +
> +#define FN_ADD(x, y) (x+y)
> 
> Modified: cfe/trunk/test/Modules/macros.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/macros.c?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/test/Modules/macros.c (original)
> +++ cfe/trunk/test/Modules/macros.c Wed Apr  3 12:39:30 2013
> @@ -125,6 +125,7 @@ void test2() {
>  void test3() {
>    double d;
>    LEFT_RIGHT_DIFFERENT *dp = &d; // okay
> +  int x = FN_ADD(1,2);
>  }
> 
>  #ifndef TOP_RIGHT_UNDEF
> 
> Modified: cfe/trunk/test/Preprocessor/macro_misc.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/macro_misc.c?rev=178671&r1=178670&r2=178671&view=diff
> ==============================================================================
> --- cfe/trunk/test/Preprocessor/macro_misc.c (original)
> +++ cfe/trunk/test/Preprocessor/macro_misc.c Wed Apr  3 12:39:30 2013
> @@ -21,3 +21,17 @@
>  #define FUNC_LIKE3(a) ( a)  // expected-note {{previous definition is here}}
>  #define FUNC_LIKE3(a) (a) // expected-warning {{'FUNC_LIKE3' macro redefined}}
> 
> +// RUN: %clang_cc1 -fms-extensions -DMS_EXT %s -Eonly -verify
> +#ifndef MS_EXT
> +// This should under C99.
> +#define FUNC_LIKE4(a,b) (a+b)  // expected-note {{previous definition is here}}
> +#define FUNC_LIKE4(x,y) (x+y) // expected-warning {{'FUNC_LIKE4' macro redefined}}
> +#else
> +// This shouldn't under MS extensions.
> +#define FUNC_LIKE4(a,b) (a+b)
> +#define FUNC_LIKE4(x,y) (x+y)
> +
> +// This should.
> +#define FUNC_LIKE5(a,b) (a+b) // expected-note {{previous definition is here}}
> +#define FUNC_LIKE5(x,y) (y+x) // expected-warning {{'FUNC_LIKE5' macro redefined}}
> +#endif
> 
> 
> _______________________________________________
> 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/20130403/9f09c4eb/attachment.html>


More information about the cfe-commits mailing list