<div class="gmail_quote">On Thu, Mar 24, 2011 at 6:32 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
We implement __attribute__((nonnull)), but only test for it on CallExpr nodes. When calling a constructor, the AST only has a CXXConstructorExpr with no CallExpr, so the warning doesn't fire. This patch adds the same test that CallExpr uses to CXXConstructorExpr.<div>


<br></div><div>Please review!</div></blockquote><div><br></div><div>LGTM</div><div><br></div><div>I find a few things about the existing code strange, but your patch doesn't introduce any of them.</div><div><br></div>
<div>Why do we loop over attributes inside of CheckNonNull... as well as in the callers? I understand why the caller loop is necessary, I don't understand why we still need the inner loop, or why we don't sink the callers' loops into CheckNonNull, but maybe other code needs this.</div>
<div><br></div><div>Also, It seems like the location of the argument which is erroneously NULL would be a better location than the location of the call...</div></div>