<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 12 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Text bubliny Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.StylE-mailovZprvy17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.TextbublinyChar
{mso-style-name:"Text bubliny Char";
mso-style-priority:99;
mso-style-link:"Text bubliny";
font-family:"Tahoma","sans-serif";}
span.shorttext
{mso-style-name:short_text;}
span.hps
{mso-style-name:hps;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 70.85pt 70.85pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=CS link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Hi,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>I think Martin is right. Preprocessor is only textual replacement, compiler </span><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'>should not know</span></span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> about this step and compiles code resulting from preprocess step.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>I used Martin</span><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>’s source code I have got different number of warnings if preprocess step was made as extra step before compilation, in my opinion it is a bug.<o:p></o:p></span></p><p class=MsoNormal><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'>Common sense</span></span><span class=shorttext><span lang=EN style='font-family:"Calibri","sans-serif"'> </span></span><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'>tells me that</span></span><span class=shorttext><span lang=EN style='font-family:"Calibri","sans-serif"'> </span></span><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'>this “equation” should be true every time: clang++ -c = clang++ -E + clang++ -c.<o:p></o:p></span></span></p><p class=MsoNormal><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'><o:p> </o:p></span></span></p><p class=MsoNormal><span class=hps><span lang=EN style='font-family:"Calibri","sans-serif"'>Mira<o:p></o:p></span></span></p><p class=MsoNormal><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri","sans-serif"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>d:\Tmp>clang++ -v<o:p></o:p></span></p><p class=MsoNormal><span lang=DE style='font-size:10.0pt;font-family:"Courier New"'>clang version 3.6.0 (220682)<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>Target: i686-pc-windows-gnu<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>Thread model: posix<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>d:\Tmp>clang++ -c foo.cpp<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:9:13: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> [-Wundefined-bool-conversion]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (!(this)); // warns<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> </span><span lang=DE style='font-size:10.0pt;font-family:"Courier New"'>~ ^~~~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:23:12: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> evaluate to true [-Wtautological-undefined-compare]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (&r != 0); // warns<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ^ ~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:30:8: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> evaluate to true [-Wtautological-undefined-compare]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> RET(&r != 0); // does NOT warn<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ^ ~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:2:13: note: expanded from macro 'RET'<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (!(expr))<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ^<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>3 warnings generated.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>d:\Tmp>clang++ -E foo.cpp > foo2.cpp<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>d:\Tmp>clang++ -c foo2.cpp<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:9:13: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> [-Wundefined-bool-conversion]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (!(this));<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ~ ^~~~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:14:13: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> [-Wundefined-bool-conversion]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (!(this));<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ~ ^~~~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:23:12: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> evaluate to true [-Wtautological-undefined-compare]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (&r != 0);<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ^ ~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>foo.cpp:30:14: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> evaluate to true [-Wtautological-undefined-compare]<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> return (!(&r != 0));<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'> ^ ~<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:10.0pt;font-family:"Courier New"'>4 warnings generated.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> <a href="mailto:cfe-dev-bounces@cs.uiuc.edu">cfe-dev-bounces@cs.uiuc.edu</a> [<a href="mailto:cfe-dev-bounces@cs.uiuc.edu">mailto:cfe-dev-bounces@cs.uiuc.edu</a>] <b>On Behalf Of </b>Nick Lewycky<br><b>Sent:</b> Wednesday, November 05, 2014 8:05 AM<br><b>To:</b> Richtarsky, Martin<br><b>Cc:</b> <a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br><b>Subject:</b> Re: [cfe-dev] Warnings not raised from a macro: -Wundefined-bool-conversion/-Wtautological-undefined-compare<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><div><div><p class=MsoNormal>On 4 November 2014 02:15, Richtarsky, Martin <<a href="mailto:martin.richtarsky@sap.com" target="_blank">martin.richtarsky@sap.com</a>> wrote:<o:p></o:p></p><p class=MsoNormal>Hi,<br><br>while cleaning some code of "-Wundefined-bool-conversion"/"-Wtautological-undefined-compare" warnings I found that these are never raised when coming from a macro. I know this is intentional in clang, but I wonder whether the benefits of this really outweigh the risks.<br><br>clang 3.5 optimizes these checks away, so it is crucial to find all places that rely on them in the codebase. But when the code comes from a macro there is suddenly no way to do so at compile time. See functions f2() below.<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I think this is correct behaviour. How is the macro supposed to know whether the pointer you gave it is the magically non-nullable kind or the regular flavour? Are you going to ask people to use two different macros depending on whether the argument is nullable or not?<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt'><p class=MsoNormal>How are others dealing with this problem e.g. at Google and Apple with large codebases likely affected by this?<o:p></o:p></p></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>With a similar macro, CHECK, I've been seeing callers doing "CHECK(this != nullptr);" and that gets flagged by the warning. I haven't seen any failures due to a pattern like you describe, and only something like a dozen or so code changes at Google.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>One alternative is to build with -fsanitize=null,nonnull-attribute,returns-nonnull-attribute and run their tests.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>But maybe you don't have tests. You could gin up a patched clang that comments out the "Don't warn inside macros" logic in Sema::DiagnoseAlwaysNonNullPointer. But maybe you can't easily vend a deliberately-non-production compiler. The last trick up my sleeve is clang-query.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><div><p class=MsoNormal>clang-query> match implicitCastExpr(hasImplicitDestinationType(asString("_Bool")), hasSourceExpression(thisExpr()))<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Match #1:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>/usr/local/google/home/nlewycky/sap.cc:3:12: note: "root" binds here<o:p></o:p></p></div><div><p class=MsoNormal> return this;<o:p></o:p></p></div><div><p class=MsoNormal> ^~~~<o:p></o:p></p></div><div><p class=MsoNormal>1 match.<o:p></o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Hrm. I tried writing the obvious thing for address of reference, but this doesn't work:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><div><p class=MsoNormal>clang-query> match implicitCastExpr(hasImplicitDestinationType(asString("_Bool")), hasSourceExpression(unaryOperator(hasOperatorName("&"), hasUnaryOperand(hasType(referenceType())))))<o:p></o:p></p></div><div><p class=MsoNormal>0 matches.<o:p></o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>because apparently nothing has referenceType():<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><div><p class=MsoNormal>clang-query> match expr(hasType(referenceType()))<o:p></o:p></p></div><div><p class=MsoNormal>0 matches.<o:p></o:p></p></div></div><div><div><p class=MsoNormal>clang-query> match expr(hasType(qualType(referenceType())))<o:p></o:p></p></div><div><p class=MsoNormal>0 matches.<o:p></o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The testcase is:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><div><p class=MsoNormal>bool test2(int &i) {<o:p></o:p></p></div><div><p class=MsoNormal> return &i;<o:p></o:p></p></div><div><p class=MsoNormal>}<o:p></o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Not sure what I'm doing wrong with clang-query.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Nick<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt'><p class=MsoNormal>$ cat undefined+macro.cpp<br>----<br>#define RET(expr) \<br> return (!(expr))<br><br>class A<br>{<br><br>int f1()<br>{<br> return (!this); // warns<br>}<br><br>int f2()<br>{<br> RET(this); // does NOT warn<br>}<br><br>};<br><br>int f1()<br>{<br> int a = 1;<br> int &r = a;<br> return (&r != 0); // warns<br>}<br><br>int f2()<br>{<br> int a = 1;<br> int &r = a;<br> RET(&r != 0); // does NOT warn<br>}<br>----<br><br>$ clang++ -c undefined+macro.cpp<br><br><br>undefined+macro.cpp:9:12: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true [-Wundefined-bool-conversion]<br> return (!this); // warns<br> ~^~~~<br>undefined+macro.cpp:23:12: warning: reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true [-Wtautological-undefined-compare]<br> return (&r != 0); // warns<br> ^ ~<br>2 warnings generated.<br><br><br>Best regards,<br>Martin<br><br>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><o:p></o:p></p></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></div></body></html>