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

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

steakhal added a comment.

The code looks good now.
Let's have one last round.

Do you have real-world reports?



================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:762-763
+def CXXArrayDeleteChecker : Checker<"ArrayDelete">,
+  HelpText<"Reports destructions of arrays of polymorphic objects that are"
+           "destructed as their base class.">,
+  Documentation<HasDocumentation>;
----------------
I thin these strings are concatenated like in C, thus it's gonna have "aredestructed" joined.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:174-177
+  OS << "Deleting an array of `" << TargetType.getAsString()
+     << "` objects as their base class `"
+     << SourceType.getAsString(C.getASTContext().getPrintingPolicy())
+     << "` is undefined";
----------------
Prefer single quotes over backticks.


================
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();
----------------
What is the problem with this?
I thought `getPointeeType()` works for ReferenceTypes.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:218-219
+
+  OS << "Casting from `" << SourceType.getAsString() << "` to `"
+     << TargetType.getAsString() << "` here";
+
----------------
We use single apostrophes for quoting names in CSA.


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

https://reviews.llvm.org/D158156