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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
