<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Apr 3, 2013, at 12:44 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br><div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><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>#define SOME_CONSTANT (ULONG)0xffff</div><div>...</div><div>#define SOME_CONSTANT (u_long)0xffff<br></div><div><br></div><div>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></blockquote><div><br></div><div>Does MSVC not warn for these ?</div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Apr 3, 2013 at 1:39 PM, Argyrios Kyrtzidis<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank">akyrtzi@gmail.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Author: akirtzidis<br>Date: Wed Apr 3 12:39:30 2013<br>New Revision: 178671<br><br>URL:<span class="Apple-converted-space"> </span><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><a href="rdar://13562254">rdar://13562254</a><br><br>Modified:<br> <span class="Apple-converted-space"> </span>cfe/trunk/include/clang/Lex/MacroInfo.h<br> <span class="Apple-converted-space"> </span>cfe/trunk/lib/Frontend/CompilerInstance.cpp<br> <span class="Apple-converted-space"> </span>cfe/trunk/lib/Lex/MacroInfo.cpp<br> <span class="Apple-converted-space"> </span>cfe/trunk/lib/Lex/PPDirectives.cpp<br> <span class="Apple-converted-space"> </span>cfe/trunk/lib/Serialization/ASTReader.cpp<br> <span class="Apple-converted-space"> </span>cfe/trunk/test/Modules/Inputs/macros_left.h<br> <span class="Apple-converted-space"> </span>cfe/trunk/test/Modules/Inputs/macros_right.h<br> <span class="Apple-converted-space"> </span>cfe/trunk/test/Modules/macros.c<br> <span class="Apple-converted-space"> </span>cfe/trunk/test/Preprocessor/macro_misc.c<br><br>Modified: cfe/trunk/include/clang/Lex/MacroInfo.h<br>URL:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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:<span class="Apple-converted-space"> </span><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></blockquote></div></div></div></blockquote></div><br></body></html>