<div dir="ltr">> <span style="font-size:12.8px">I like static analysis, but I do not think the static analysis available in clang today for null checking is suitable for -Wall. One of the reasons is because it is difficult to silence false positives (as you mentioned). More importantly though, the impact to build time is quite substantial. Nullability checks are path sensitive, and path sensitive checks are super-exponential. If a file takes seconds to compile, it is fairly common for it to take minutes to analyze.</span><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Sorry, I didn't express myself clearly. The idea was more to make a new pass check like the uninitialized value check, but checking for nulls rather than "has this been assigned?" -- it's going to be a low-quality analysis, but I'd think it should be able to catch trivial cases without eating too many cycles.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">> For item 3., I was under the impression that most smart pointer classes inlined well enough that the static analyzer was still effective at finding null issues. Do you have a small example that causes a null pointer warning with a raw pointer, but doesn't cause a null pointer warning with a smart pointer?</span><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px"><br></span></div><div>Looks like you're right! :) I assumed that because we didn't give warnings on code like this:</div><div><br></div><div>static int *f() { return nullptr; }</div><div>int g() { return *f(); }</div><div><br></div><div>...We wouldn't do much to look through function calls/etc. After playing around a bit, I've found that this was not a good assumption, and our handling for smart pointers/pointer types where we use the lower N bits for flags/etc. is actually quite good. Thanks for correcting me :)</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 6, 2015 at 2:06 PM, Craig, Ben via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
I like static analysis, but I do not think the static analysis
available in clang today for null checking is suitable for -Wall.
One of the reasons is because it is difficult to silence false
positives (as you mentioned). More importantly though, the impact
to build time is quite substantial. Nullability checks are path
sensitive, and path sensitive checks are super-exponential. If a
file takes seconds to compile, it is fairly common for it to take
minutes to analyze.<br>
<br>
I'm fine with the check being under a different flag, but lumping it
in with -Wall would cause a lot of developer pain. In fact, I would
very much like to be able to run the analyzer at the same time that
I do a compile. That idea has been discussed before though, and
there was resistance:<br>
<a href="http://lists.llvm.org/pipermail/cfe-dev/2013-July/031097.html" target="_blank">http://lists.llvm.org/pipermail/cfe-dev/2013-July/031097.html</a><br>
<br>
For item 3., I was under the impression that most smart pointer
classes inlined well enough that the static analyzer was still
effective at finding null issues. Do you have a small example that
causes a null pointer warning with a raw pointer, but doesn't cause
a null pointer warning with a smart pointer?<br>
<br>
2. and 4. seem like reasonable ideas to me, assuming they are
implementable.<div><div><br>
<br>
<div>On 11/6/2015 3:31 PM, George Burgess IV
via cfe-dev wrote:<br>
</div>
</div></div><blockquote type="cite"><div><div>
<div dir="ltr">Hello friends!
<div><br>
</div>
<div>I've been evaluating the state of null analysis/etc. in
clang recently, and it looks like clang's story for static
nullness analysis has been getting quite a bit better over
time. With the help of others, I've identified a few areas
where we may be able to improve, but I'd really like opinions
on whether we think these changes would actually be a good
thing.</div>
<div><br>
</div>
<div>Specifically, I have four distinct changes in mind:</div>
<div>1. Turn some amount of nullability analysis on by default
(with -Wall) in clang. This would be conceptually <i>very</i> similar
to uninitialized value checking, and would be able to catch
simple cases like</div>
<div><br>
</div>
<div>Foo *p = nullptr;</div>
<div>if (p = getPtr())</div>
<div> p->oneThing();</div>
<div>else</div>
<div> p->anotherThing(); // warning: p is null.</div>
<div><br>
</div>
<div>...But no promises for any nontrivial cases (without
heavily annotated locals/function signatures ;) ), because
there's currently no planned way to silence the warning if
we're somehow wrong.</div>
<div><br>
</div>
<div>2. Speaking of nullness annotations, clang supports a lot
of them. Migrating old code to use them could be painful, so
having a <a href="https://docs.google.com/document/d/1vXuhRTQsbf4F9PbFtCoapuAhCU4RrD-IAiaUfTwp4uA/edit?usp=sharing" target="_blank">tool that annotates obvious things for us
may be nice to have</a> (<a href="https://docs.google.com/document/d/1vXuhRTQsbf4F9PbFtCoapuAhCU4RrD-IAiaUfTwp4uA/edit?usp=sharing" target="_blank">https://docs.google.com/document/d/1vXuhRTQsbf4F9PbFtCoapuAhCU4RrD-IAiaUfTwp4uA/edit?usp=sharing</a>).</div>
<div><br>
</div>
<div>3. Add a CXXRecordDecl-level attribute that instructs
nullness analysis to treat instances of the attributed type as
a pointer for the sake of nullness analysis. This would enable
nullness analysis of things like unique_ptr/shared_ptr/...
Doc is available <a href="https://docs.google.com/document/d/1Zyb8o210EqkAXxrnrv4XtRu4w_i0yXO04p4KTuTde4M/edit?usp=sharing" target="_blank">here</a> (<a href="https://docs.google.com/document/d/1Zyb8o210EqkAXxrnrv4XtRu4w_i0yXO04p4KTuTde4M/edit?usp=sharing" target="_blank">https://docs.google.com/document/d/1Zyb8o210EqkAXxrnrv4XtRu4w_i0yXO04p4KTuTde4M/edit?usp=sharing</a>).</div>
<div><br>
</div>
<div>4. Add clang_tidy checks for missing nullness annotations
on function signatures/global variable decls/member variable
decls/...</div>
<div><br>
</div>
<div>Like said, any feedback on how {useful,useless} we think
these things would be (and feedback on on the designs
themselves) is highly appreciated. :)</div>
<div><br>
</div>
<div>Thanks for your time!</div>
<div>George</div>
</div>
<br>
<fieldset></fieldset>
<br>
</div></div><pre>_______________________________________________
cfe-dev mailing list
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><span><font color="#888888">
</font></span></pre><span><font color="#888888">
</font></span></blockquote><span><font color="#888888">
<br>
<pre cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
</pre>
</font></span></div>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>