<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 4 November 2014 02:15, Richtarsky, Martin <span dir="ltr"><<a href="mailto:martin.richtarsky@sap.com" target="_blank">martin.richtarsky@sap.com</a>></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">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.<br></blockquote><div><br></div><div>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?</div><div><br></div><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">How are others dealing with this problem e.g. at Google and Apple with large codebases likely affected by this?<br></blockquote><div><br></div><div>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.</div><div><br></div><div>One alternative is to build with -fsanitize=null,nonnull-attribute,returns-nonnull-attribute and run their tests.</div><div><br></div><div>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.</div><div><br></div><div><div>clang-query> match implicitCastExpr(hasImplicitDestinationType(asString("_Bool")), hasSourceExpression(thisExpr()))</div><div><br></div><div>Match #1:</div><div><br></div><div>/usr/local/google/home/nlewycky/sap.cc:3:12: note: "root" binds here</div><div> return this;</div><div> ^~~~</div><div>1 match.</div></div><div><br></div><div><br></div><div>Hrm. I tried writing the obvious thing for address of reference, but this doesn't work:</div><div><br></div><div><div>clang-query> match implicitCastExpr(hasImplicitDestinationType(asString("_Bool")), hasSourceExpression(unaryOperator(hasOperatorName("&"), hasUnaryOperand(hasType(referenceType())))))</div><div>0 matches.</div></div><div><br></div><div>because apparently nothing has referenceType():</div><div><br></div><div><div>clang-query> match expr(hasType(referenceType()))</div><div>0 matches.</div></div><div><div>clang-query> match expr(hasType(qualType(referenceType())))</div><div>0 matches.</div></div><div><br></div><div>The testcase is:</div><div><br></div><div><div>bool test2(int &i) {</div><div> return &i;</div><div>}</div></div><div><br></div><div>Not sure what I'm doing wrong with clang-query.</div><div><br></div><div>Nick</div><div><br></div><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">$ 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><br>
</blockquote></div><br></div></div>