[PATCH] D144064: [-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Looks great overall!



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:156
+  // 2. the operand of a cast-to-(Integer or Boolean) operation; or
+  // 3. the operand of a pointer subtraction operation; or
+  // 4. the operand of a pointer comparison operation; or
----------------
malavikasamak wrote:
> malavikasamak wrote:
> > I don't understand why we are special handling only pointer subtraction here. Won't pointer addition be also considered UPC? If so, can we just add it to this patch.
> Never mind. It looks like pointer addition is not even legal and subtraction is a special case. 
I think this is about operations between two pointers, like `p - q`. You can subtract a pointer from a pointer (it gives you the distance between points in memory) but you can't add a pointer to a pointer (it makes no sense).

But yeah, could be clarified.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:175-178
+		   allOf(hasLHS(hasPointerType()),
+			 hasRHS(hasPointerType())),
+		   eachOf(hasLHS(InnerMatcher),
+			  hasRHS(InnerMatcher)));
----------------
Tabs vs. spaces.


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp:88
+
+// CHECK-NOT: fix-it:"{{.*}}":{
 [[clang::unsafe_buffer_usage]]
----------------
I think the original CHECK-NOT was better because it allows you to write more tests below it. It's good to have individual test cases self-contained and not affect the rest of the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144064