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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
