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