aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def:1 +//=- UnsafeBufferUsageGadgets.def - List of ways to use a buffer --*- C++ -*-=// +// ---------------- Is this file named properly if it is also going to handle `SAFE_GADGET`? ================ Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def:9 + +#ifndef GADGET +#define GADGET(name) ---------------- You have some comments below about what gadgets are and what makes one safe or unsafe; should those comments be hoisted into this .def file? ================ Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:17-21 +static auto hasPointerType() { + return anyOf(hasType(pointerType()), + hasType(autoType(hasDeducedType( + hasUnqualifiedDesugaredType(pointerType()))))); } ---------------- Something to think about: ObjC pointers are pointer-like but not actual pointers; should those be covered as well (via `isAnyPointerType()`)? ================ Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:37 +#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def" +#undef GADGETS + }; ---------------- NoQ wrote: > Whoops typo! We don't even need the `#undef` because the .def file already does that for us. Same observation applies below. ================ Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:63 + + Gadget(Kind K) : K(K) {} + ---------------- Should this be an explicit constructor? (Same question applies below to derived classes as well.) ================ Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:113 + : UnsafeGadget(Kind::Increment), + Op(Result.Nodes.getNodeAs<UnaryOperator>("op")) {} + ---------------- Should we make a `static constexpr const char *` for these strings so we can give it a name and ensure it's used consistently? 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