[PATCH] D137348: [-Wunsafe-buffer-usage] Introduce an abstraction for fixable code patterns.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:63
+
+  Gadget(Kind K) : K(K) {}
+
----------------
NoQ wrote:
> aaron.ballman wrote:
> > Should this be an explicit constructor? (Same question applies below to derived classes as well.)
> What's the value of making it `explicit` given that it's an abstract class that can't be constructed directly anyway?
None, I had missed that this was an abstract class and forgot to delete the comment here on the base class. It does apply to the (non-abstract) derived classes though.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:113
+      : UnsafeGadget(Kind::Increment),
+        Op(Result.Nodes.getNodeAs<UnaryOperator>("op")) {}
+
----------------
NoQ wrote:
> aaron.ballman wrote:
> > Should we make a `static constexpr const char *` for these strings so we can give it a name and ensure it's used consistently?
> I think it makes perfect sense to have different bind-names in different classes. They don't all correspond to the same role the bound expression plays.
Sorry, I was unclear. I meant a private data member of `IncrementGadget` so that the constructor and the `matcher()` functions use a named constant rather than a string literal.


Repository:
  rC Clang

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

https://reviews.llvm.org/D137348