On 24 March 2011 18:40, Chandler Carruth <[email protected]> wrote:
> On Thu, Mar 24, 2011 at 6:32 PM, Nick Lewycky <[email protected]> wrote: > >> 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. >> >> Please review! >> > > LGTM > > I find a few things about the existing code strange, but your patch doesn't > introduce any of them. > > Why do we loop over attributes inside of CheckNonNull... as well as in > the callers? > Because this is legal: void test(void *, void *, void *) __attribute__((nonnull(1, 2))) __attribute__((nonnull(3))); The inner loop finds the "1, 2" inside, the outer loop finds the nonnull attribute(s) amongst all the attributes. > 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. > > Also, It seems like the location of the argument which is erroneously NULL > would be a better location than the location of the call... > We highlight the expression's source range. It looks like this: foo.cc:10:3: warning: null passed to a callee which requires a non-null argument [-Wnonnull] Test(NULL); ^ ~~~~ foo.cc:11:8: warning: null passed to a callee which requires a non-null argument [-Wnonnull] Test t(NULL); ^ ~~~~ foo.cc:12:3: warning: null passed to a callee which requires a non-null argument [-Wnonnull] test2(0); ^ ~ Nick
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
