[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Given that the compiler already has `-Wsign-conversion`, I'm not certain what value is added by this check. Can you explain a bit about why this is the correct approach for diagnosing the issue? When you ignore the false positives from the test, the only cases not pointed out by `-Wsign-compare` are the macro cases (which is reasonable behavior to not diagnose on -- the fix for one macro may make other macros invalid).



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp:36-37
+  if (x - 2 > 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+    // CHECK-FIXES: if (x - 2u > 0) {
+    return;
----------------
I consider this to be a false positive diagnostic -- the `2` is converted from a signed value into an unsigned value and there cannot possibly be a conversion error on that implicit cast.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp:100
+  if (y.size() - 1 > 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: signed value subtracted from
+    // CHECK-FIXES: if (y.size() - 1u > 0) {
----------------
Similarly, I consider this to be a false positive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71607