aaron.ballman added inline comments.
================
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ if (p) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
----------------
SilverGeri wrote:
> aaron.ballman wrote:
> > hokein wrote:
> > > Does it work the case like:
> > >
> > > ```
> > > int *p = nullptr;
> > > if (p == nullptr) {
> > > p = new int[3];
> > > delete[] p;
> > > }
> > > ```
> > >
> > > ?
> > Similarly, it should not mishandle a case like:
> >
> > void f(int *p) {
> > if (p) {
> > delete p;
> > } else {
> > // Do something else
> > }
> > }
> it warns only if the compund statement contains only one statement (which is
> the delete). We want to warn because it is unnecessary to check the pointer
> validity if you want to just call `delete`. In other cases, we can't be sure
> about the actual behaviour.
In my example, the compound statement does contain only one statement, the
delete, but diagnosing is likely a false positive due to the else clause. In
that case, the pointer validity controls more than just the delete because it
also controls whether to execute the else clause. Removing the if clause
shouldn't be a mechanical change in the presence of an else clause, so the
fixit is definitely inappropriate. I think that diagnosing is still pretty
reasonable, however.
Another test case, which I think may already be handled appropriately but
should still be explicitly tested:
```
if (p) {
// This comment should not disable the check or the fixit.
// Nor should this comment.
delete p;
}
```
I think this check should still be able to remove the if clause, but we should
make sure that the comments don't disable the check, and that the fixit doesn't
destroy the comments.
https://reviews.llvm.org/D21298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
