<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""></div><div class=""><br class=""></div><div class="">- Fariborz</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 23, 2014, at 5:25 PM, Richard Trieu <<a href="mailto:rtrieu@google.com" class="">rtrieu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Oct 23, 2014 at 10:24 AM, jahanian <span dir="ltr" class=""><<a href="mailto:fjahanian@apple.com" target="_blank" class="">fjahanian@apple.com</a>></span> wrote:<br class=""><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">I would like to warn on logical negation of operands with known non-null value; as in "if (!array)…", just as we warn on "if (array == 0)…"<br class="">
Note that this is a c-only patch. c++ already has a warning when such non-null pointers are implicitly converted to bool.<br class="">
Is this ok for check-in?<br class="">
<br class="">
- Fariborz<br class=""><br class=""></blockquote><div class=""><br class=""></div><div class=""> Index: include/clang/AST/Expr.h</div><div class="">===================================================================</div><div class="">--- include/clang/AST/Expr.h<span class="" style="white-space:pre"> </span>(revision 220486)</div><div class="">+++ include/clang/AST/Expr.h<span class="" style="white-space:pre"> </span>(working copy)</div><div class="">@@ -641,7 +641,10 @@</div><div class=""> NPCK_CXX11_nullptr,</div><div class=""> </div><div class=""> /// \brief Expression is a GNU-style __null constant.</div><div class="">- NPCK_GNUNull</div><div class="">+ NPCK_GNUNull,</div><div class="">+ </div><div class="">+ /// \brief Expression is operand of a logical negation operator</div><div class="">+ NPCK_LNEG_ZeroLiteral</div><div class=""> };</div><div class=""><br class=""></div><div class="">Don't add another element to enum NullPointerConstantKind. This is used for determining what kind of null pointer an expression is, meaning it should only be extended if the evaluate for null functions in Expr are updated to return it. Requiring another value for a parameter in Sema function is not a good reason to extend the enum.<br class=""></div><div class=""><br class=""></div><div class=""><div class="">Index: include/clang/Basic/DiagnosticSemaKinds.td</div><div class="">===================================================================</div><div class="">--- include/clang/Basic/DiagnosticSemaKinds.td<span class="" style="white-space:pre"> </span>(revision 220486)</div><div class="">+++ include/clang/Basic/DiagnosticSemaKinds.td<span class="" style="white-space:pre"> </span>(working copy)</div><div class="">@@ -2512,6 +2512,10 @@</div><div class=""> "comparison of %select{address of|function|array}0 '%1' %select{not |}2"</div><div class=""> "equal to a null pointer is always %select{true|false}2">,</div><div class=""> InGroup<TautologicalPointerCompare>;</div><div class="">+def warn_null_logical_negation_compare : Warning<</div><div class="">+ "logical negation of %select{address of|function|array}0 '%1' is "</div><div class="">+ "always %select{true|false}2">,</div><div class="">+ InGroup<TautologicalPointerCompare>;</div><div class=""> def warn_this_null_compare : Warning<</div><div class=""> "'this' pointer cannot be null in well-defined C++ code; comparison may be "</div><div class=""> "assumed to always evaluate to %select{true|false}0">,</div></div><div class=""><br class=""></div><div class="">There should not be two different messages for the same warning in C versus C++ mode. Use the old diagnostic message to consistent.</div><div class=""><br class=""></div><div class=""><div class="">Index: lib/Sema/SemaExpr.cpp</div><div class="">===================================================================</div><div class="">--- lib/Sema/SemaExpr.cpp<span class="" style="white-space:pre"> </span>(revision 220486)</div><div class="">+++ lib/Sema/SemaExpr.cpp<span class="" style="white-space:pre"> </span>(working copy)</div><div class="">@@ -9952,7 +9952,10 @@</div><div class=""> // that are explicitly defined as valid by the standard).</div><div class=""> if (Opc != UO_AddrOf && Opc != UO_Deref)</div><div class=""> CheckArrayAccess(Input.get());</div><div class="">-</div><div class="">+ if (Opc == UO_LNot && Input.get()->getType()->isPointerType() &&</div><div class="">+ !getLangOpts().CPlusPlus)</div><div class="">+ DiagnoseAlwaysNonNullPointer(Input.get(), Expr::NPCK_LNEG_ZeroLiteral,</div><div class="">+ true, SourceRange());</div><div class=""> return new (Context)</div><div class=""> UnaryOperator(Input.get(), Opc, resultType, VK, OK, OpLoc);</div><div class=""> }</div></div><div class=""><br class=""></div><div class="">DiagnoseAlwaysNonNullPointer is currently called from the CheckImplicitConversion and AnalyzeImplicitConversion set of functions. A better way is to find where a logical not expression is processed during the implicit conversion checks, then add a special case to check the sub-expression for conversions to bool. Also, the LangOptions should be checked for Bool, not CPlusPlus, and extra checks for logical and and logical or should happen as well.</div></div></div></div>
</div></blockquote></div><br class=""></div></body></html>