subject:"\[PATCH\] D68694\: \[clang\-tidy\] hicpp\-signed\-bitwise\: Do not show \"use of a signed integer operand with a binary bitwise operator\" for positive integer operands"

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

2019-10-30 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've committed on your behalf in 
4de6b1586807285e20a5db6596519c2336a64568 
.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-23 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun marked an inline comment as done.
vladimir.plyashkun added a comment.

In D68694#1711035 , @aaron.ballman 
wrote:

> LGTM aside from a small nit.


Thanks, fixed it!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-23 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun updated this revision to Diff 226101.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp

Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand with a binary bitwise operator
+  
+  unsigned URes2 = URes << 1; //Ok
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  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, //Ok
+};
Index: clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
+++ clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
@@ -7,3 +7,11 @@
 undefined or implementation defined behaviour.
 
 The according rule is defined in the `High Integrity C++ Standard, Section 5.6.1 `_.
+
+Options
+---
+
+.. option:: IgnorePositiveIntegerLiterals
+
+   If this option is set to `true`, the check will not warn on bitwise operations with positive integer literals, e.g. `~0`, `2 << 1`, etc. 
+   Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,10 @@
   ` check.
 
   Rewrites function signatures to use a trailing return type.
+  
+- The :doc:`hicpp-signed-bitwise
+  ` now supports ``IgnorePositiveIntegerLiterals``
+  option.
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp
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
@@ -17,9 +17,24 @@
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+

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

2019-10-16 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a small nit.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:174
+- The :doc:`hicpp-signed-bitwise
+  ` now supports 
`IgnorePositiveIntegerLiterals`
+  option.

`IgnorePositiveIntegerLiterals` should use two backticks instead of one.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-15 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth accepted this revision.
JonasToth added a comment.
This revision is now accepted and ready to land.

LGTM.
But i was inactive for a long time, if @aaron.ballman accepts as well you can 
commit instantly. Otherwise please let the other people that commented some 
time to react. :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-15 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun added a comment.

In D68694#1706204 , @JonasToth wrote:

> > Do you know who is responsible for it? Because i haven't worked with 
> > documentation before and don't know what i need to do to update it.
>
> The files reside in `clang-tools-extra/docs/ReleaseNotes.rst` and 
> `clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst`.
>
> You can check other files for reference, e.g. `readability-magic-numbers.rst`.
>  In the Release Notes you can add one entry at the end of the clang-tidy 
> section.


Thanks! I've fixed documentation.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-15 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun updated this revision to Diff 224976.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
  clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp

Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand with a binary bitwise operator
+  
+  unsigned URes2 = URes << 1; //Ok
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  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, //Ok
+};
Index: clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
+++ clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst
@@ -7,3 +7,11 @@
 undefined or implementation defined behaviour.
 
 The according rule is defined in the `High Integrity C++ Standard, Section 5.6.1 `_.
+
+Options
+---
+
+.. option:: IgnorePositiveIntegerLiterals
+
+   If this option is set to `true`, the check will not warn on bitwise operations with positive integer literals, e.g. `~0`, `2 << 1`, etc. 
+   Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,10 @@
   ` check.
 
   Rewrites function signatures to use a trailing return type.
+  
+- The :doc:`hicpp-signed-bitwise
+  ` now supports `IgnorePositiveIntegerLiterals`
+  option.
 
 Improvements to include-fixer
 -
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp
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
@@ -17,9 +17,24 @@
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+

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

2019-10-11 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

> Do you know who is responsible for it? Because i haven't worked with 
> documentation before and don't know what i need to do to update it.

The files reside in `clang-tools-extra/docs/ReleaseNotes.rst` and 
`clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst`.

You can check other files for reference, e.g. `readability-magic-numbers.rst`.
In the Release Notes you can add one entry at the end of the clang-tidy section.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun added a comment.

In D68694#1705668 , @JonasToth wrote:

> The new switch needs documentation as well, and maybe even a note in the 
> release notes (as it is publicly discussed as issue?).


Do you know who is responsible for it? Because i haven't worked with 
documentation before and don't know what i need to do to update it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun updated this revision to Diff 224597.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694

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


Index: 
clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand 
with a binary bitwise operator
+  
+  unsigned URes2 = URes << 1; //Ok
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  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, //Ok
+};
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp
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
@@ -17,9 +17,24 @@
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+   : expr(ignoringImpCasts(hasType(isSignedInteger()
+  .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


Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand with a binary bitwise operator
+  
+  unsigned URes2 = URes << 1; //Ok
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun marked 2 inline comments as done.
vladimir.plyashkun added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {}
+

lebedev.ri wrote:
> vladimir.plyashkun wrote:
> > lebedev.ri wrote:
> > > i'm not sure this should look for a global option with such name?
> > I think that this method is common and used in so many inspections. 
> > For example this [[ 
> > https://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html
> >   | check ]] also have option called `IgnoreMacros` which is retrieved in 
> > same way (by calling getLocalOrGlobal method)
> I'm very specifically discriminating against 
> `"IgnorePositiveIntegerLiterals"` here.
> I know `getLocalOrGlobal()` is widely used, because in those places the same 
> flag
> is used in multiple modules with same meaning.
> Is that the case here?
Ok, i agree. Fixed.



Comment at: 
clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp:19
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator

JonasToth wrote:
> Could you please add `URes << 1` as well? I believe that was problematic in 
> the stack-overflow-question, wasn't it?
Yes, sure.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {}
+

vladimir.plyashkun wrote:
> lebedev.ri wrote:
> > i'm not sure this should look for a global option with such name?
> I think that this method is common and used in so many inspections. 
> For example this [[ 
> https://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html
>   | check ]] also have option called `IgnoreMacros` which is retrieved in 
> same way (by calling getLocalOrGlobal method)
I'm very specifically discriminating against `"IgnorePositiveIntegerLiterals"` 
here.
I know `getLocalOrGlobal()` is widely used, because in those places the same 
flag
is used in multiple modules with same meaning.
Is that the case here?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun marked an inline comment as done.
vladimir.plyashkun added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {}
+

lebedev.ri wrote:
> i'm not sure this should look for a global option with such name?
I think that this method is common and used in so many inspections. 
For example this [[ 
https://clang.llvm.org/extra/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.html
  | check ]] also have option called `IgnoreMacros` which is retrieved in same 
way (by calling getLocalOrGlobal method)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

The new switch needs documentation as well, and maybe even a note in the 
release notes (as it is publicly discussed as issue?).
Otherwise I am fine with the changes!




Comment at: 
clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp:19
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator

Could you please add `URes << 1` as well? I believe that was problematic in the 
stack-overflow-question, wasn't it?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added inline comments.



Comment at: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp:23-24
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {}
+

i'm not sure this should look for a global option with such name?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun added a comment.

@aaron.ballman
@JonasToth
Thanks, i agree too! I've prepared new revision with additional option to 
preserve old inspection behaviour.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-10-11 Thread Vladimir Plyashkun via Phabricator via cfe-commits

vladimir.plyashkun updated this revision to Diff 224557.
vladimir.plyashkun added a comment.

Provide additional option to preserve current inspection behaviour


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694

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


Index: 
clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: 
hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand 
with a binary bitwise operator
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand 
with a binary bitwise operator
+  IResult = ~0; //Ok
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  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, //Ok
+};
Index: clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
===
--- clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
+++ clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h
@@ -22,10 +22,13 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp
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
@@ -17,9 +17,24 @@
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IgnorePositiveIntegerLiterals(
+  Options.getLocalOrGlobal("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-  
expr(ignoringImpCasts(hasType(isSignedInteger(.bind("signed-operand");
+  (IgnorePositiveIntegerLiterals
+   ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+  unless(integerLiteral()))
+   : expr(ignoringImpCasts(hasType(isSignedInteger()
+  .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


Index: clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise-integer-literals.cpp
@@ -0,0 +1,31 @@
+// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t -- \
+// RUN:   -config="{CheckOptions: [{key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: 1 }]}" \
+// RUN: -- -std=c++11
+
+void examples() {
+  unsigned UValue = 40u;
+  unsigned URes;
+  
+  URes = UValue & 1u; //Ok
+  URes = UValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int IResult;
+  IResult = 10 & 2; //Ok
+  IResult = 3 << -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  
+  int Int = 30;
+  IResult = Int << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of

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

2019-10-10 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

In D68694#1701772 , @aaron.ballman 
wrote:

> > 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.:
>
> I think that this check should behave how the coding standard dictates it 
> behaves. By my reading of HIC++, the check is behaving by design. I think 
> there are a few ways forward though (not mutually exclusive):
>
> 0) See if the HIC++ folks are interested in clarifying their standard for 
> integer literals and if they are, modify the check
>
> 1. Add an off-by-default option that suppresses the diagnostic with positive 
> integer literals so that users can explicitly decide to deviate locally
> 2. Document that this happens and is expected behavior; users can disable the 
> check if they don't wish to conform to that standard.


Hey :)
I would personally prefer option 1. My biggest wish is to have time to come 
back to development in a few weeks (but not guaranteed :/) then I would iterate 
the check a bit to make it more userfriendly as well!

Best, Jonas


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

18 matches

Site Navigation

Mail list logo

Footer information