[PATCH] D68694: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun created this revision.
vladimir.plyashkun added a reviewer: JonasToth.
vladimir.plyashkun added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

Hi! Clang-Tidy has been integrated in CLion some time ago, but a lot of users started to complain on `hicpp-signed-bitwise` inspection as it produces a lot of noise, especially for positive integer literals.

There are already some issues (with discussions) in LLVM tracker with same problem:
https://bugs.llvm.org/show_bug.cgi?id=36961
https://bugs.llvm.org/show_bug.cgi?id=43606

In my opinion this check should be disabled in case of integer literals, since there are a lot of existing code (even in system libraries) where user can do nothing, e.g.:

  #include <fcntl.h>
  
  int main() {
      open("file", O_RDONLY | O_NOCTTY); // <-- warning here
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68694

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp


Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp
@@ -96,10 +96,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= UByte2; // Ok
 
-  int SignedInt1 = 1 << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
-  int SignedInt2 = 1u << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  int SignedInt1 = 1 << 12; // Ok
+  int SignedInt2 = 1u << 12; // Ok
 }
 
 void f1(unsigned char c) {}
@@ -157,8 +155,7 @@
   r = -1 >> -i;
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
 
-  r = ~0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a unary bitwise operator
+  r = ~0; // Ok
   r = ~0u; // Ok
   k = ~k;  // Ok
 
@@ -231,10 +228,8 @@
 enum EnumConstruction {
   one = 1,
   two = 2,
-  test1 = 1 << 12,
-  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test1 = 1 << 12, // Ok
   test2 = one << two,
   // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
-  test3 = 1u << 12,
-  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test3 = 1u << 12, // Ok
 };
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
@@ -19,7 +19,9 @@
 
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-      expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed-operand");
+      expr(ignoringImpCasts(hasType(isSignedInteger())),
+           unless(integerLiteral()))
+          .bind("signed-operand");
 
   // The standard [bitmask.types] allows some integral types to be implemented
   // as signed types. Exclude these types from diagnosing for bitwise or(|) and


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68694.224014.patch
Type: text/x-patch
Size: 2406 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191009/cbeec557/attachment.bin>