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

Reply via email to