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