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