[PATCH] D158156: [analyzer] Add C++ array delete checker

[Balázs Benics via Phabricator via cfe-commits]

steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.

It's good enough already, given you apply my last suggestion.
References would be nice to support for notes, but not a blocker.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:199-201
+  // FIXME: This way of getting base types does not support reference types.
+  QualType SourceType = CastE->getSubExpr()->getType()->getPointeeType();
+  QualType TargetType = CastE->getType()->getPointeeType();
----------------
Discookie wrote:
> steakhal wrote:
> > What is the problem with this?
> > I thought `getPointeeType()` works for ReferenceTypes.
> Apparently not, because references aren't ReferenceTypes but qualified Types. I could add support for it in a future commit, but I'd think casting and deleting array-references wrongly is even less common than deleting array-pointers.
Nop, this is for tracking conversions. That might be common.
Such as in our codebase, we usually prefer references. However, in clang we have pointers all over the place, thus internally one might cast references, but delete or pass them back to a legacy API by taking the address of the reference.


================
Comment at: clang/test/Analysis/ArrayDelete.cpp:102-109
+    Base *b = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    Base &b2 = *b; // no-warning
+
+    // FIXME: Displaying casts of reference types is not supported.
+    Derived &d2 = static_cast<Derived&>(b2); // no-warning
+
+    Derived *d = &d2; // no-warning
----------------



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158156/new/

https://reviews.llvm.org/D158156