[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth closed this revision. JonasToth added a comment. Commit in https://reviews.llvm.org/rCTE329789 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth updated this revision to Diff 141970. JonasToth added a comment. - remove shifting exception for standarized bitmask types Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 Files: clang-tidy/hicpp/SignedBitwiseCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp test/clang-tidy/hicpp-signed-bitwise.cpp Index: test/clang-tidy/hicpp-signed-bitwise.cpp === --- test/clang-tidy/hicpp-signed-bitwise.cpp +++ test/clang-tidy/hicpp-signed-bitwise.cpp @@ -41,9 +41,12 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator UResult = SValue & -1; // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult&= 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator UResult = UValue & 1u; // Ok UResult = UValue & UValue; // Ok + UResult&= 2u; // Ok unsigned char UByte1 = 0u; unsigned char UByte2 = 16u; @@ -68,18 +71,30 @@ UByte1 = UByte1 | UByte2; // Ok UByte1 = UByte1 | SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= UByte2; // Ok UByte1 = UByte1 ^ UByte2; // Ok UByte1 = UByte1 ^ SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= UByte2; // Ok UByte1 = UByte1 >> UByte2; // Ok UByte1 = UByte1 >> SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= UByte2; // Ok UByte1 = UByte1 << UByte2; // Ok UByte1 = UByte1 << SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1<<= SByte2; + // 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 @@ -195,10 +210,16 @@ int s3; s3 = IntOne | IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3|= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne & IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3&= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne ^ IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3^= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 | s2; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 & s2; // Signed Index: test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp === --- test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp +++ test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp @@ -8,51 +8,57 @@ std::locale::category C = std::locale::category::ctype; SResult = std::locale::category::none | std::locale::category::collate; + SResult|= std::locale::category::collate; SResult = std::locale::category::ctype & std::locale::category::monetary; + SResult&= std::locale::category::monetary; SResult = std::locale::category::numeric ^ std::locale::category::time; + SResult^= std::locale::category::time; SResult = std::locale::category::messages | std::locale::category::all; SResult = std::locale::category::all & C; + SResult&= std::locale::category::all; SResult = std::locale::category::all | C; + SResult|= std::locale::category::all; SResult = std::locale::category::all ^ C; + SResult^= std::locale::category::all; // std::ctype_base::mask std::ctype_base::mask M = std::ctype_base::mask::punct; SResult = std::ctype_base::mask::space | std::ctype_base::mask::print; SResult = std::ctype_base::mask::cntrl & std::ctype_base::mask::upper; SResult = std::ctype_base::mask::lower ^
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
aaron.ballman added inline comments.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > How about: "shifting a value of bitmask type"
> > > Not sure about that.
> > >
> > > The general bitmasks are covered by the second `diag`.
> > >
> > > This one should only trigger if a shift with the standardized bitmask
> > > types occurs. This exception is necessary because those are allowed for
> > > the other bitwise operations(&, |, ^), but i decided that shifting them
> > > makes no sense.
> > I think "standardized bitmask type" is not very clear because it's hard to
> > know what is and isn't a standardized bitmask type (it's not a term of art
> > I'm used to hearing, anyway). I agree that shifting by them makes no sense,
> > but I also have a hard time imagining anyone is using these as shift values
> > in the first place, so perhaps the diagnostic can be removed entirely
> > unless we can find some code in the wild that does something like this?
> I totally agree that its bad.
>
> Removing this exception is ok with me.
Okay, we'll go that route -- we can always bring it back if it turns out to
have value later. Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth added inline comments.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > How about: "shifting a value of bitmask type"
> > Not sure about that.
> >
> > The general bitmasks are covered by the second `diag`.
> >
> > This one should only trigger if a shift with the standardized bitmask types
> > occurs. This exception is necessary because those are allowed for the other
> > bitwise operations(&, |, ^), but i decided that shifting them makes no
> > sense.
> I think "standardized bitmask type" is not very clear because it's hard to
> know what is and isn't a standardized bitmask type (it's not a term of art
> I'm used to hearing, anyway). I agree that shifting by them makes no sense,
> but I also have a hard time imagining anyone is using these as shift values
> in the first place, so perhaps the diagnostic can be removed entirely unless
> we can find some code in the wild that does something like this?
I totally agree that its bad.
Removing this exception is ok with me.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
aaron.ballman added inline comments.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
JonasToth wrote:
> aaron.ballman wrote:
> > How about: "shifting a value of bitmask type"
> Not sure about that.
>
> The general bitmasks are covered by the second `diag`.
>
> This one should only trigger if a shift with the standardized bitmask types
> occurs. This exception is necessary because those are allowed for the other
> bitwise operations(&, |, ^), but i decided that shifting them makes no sense.
I think "standardized bitmask type" is not very clear because it's hard to know
what is and isn't a standardized bitmask type (it's not a term of art I'm used
to hearing, anyway). I agree that shifting by them makes no sense, but I also
have a hard time imagining anyone is using these as shift values in the first
place, so perhaps the diagnostic can be removed entirely unless we can find
some code in the wild that does something like this?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth added inline comments.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
aaron.ballman wrote:
> How about: "shifting a value of bitmask type"
Not sure about that.
The general bitmasks are covered by the second `diag`.
This one should only trigger if a shift with the standardized bitmask types
occurs. This exception is necessary because those are allowed for the other
bitwise operations(&, |, ^), but i decided that shifting them makes no sense.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LG aside from a diagnostic wording nit.
Comment at: clang-tidy/hicpp/SignedBitwiseCheck.cpp:92
+ if (N.getNodeAs("std_type"))
+diag(Location, "shifting a value of the standardized bitmask types");
+ else
How about: "shifting a value of bitmask type"
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth updated this revision to Diff 141547. JonasToth added a comment. - fix std bitmask type shifting issue Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 Files: clang-tidy/hicpp/SignedBitwiseCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp test/clang-tidy/hicpp-signed-bitwise.cpp Index: test/clang-tidy/hicpp-signed-bitwise.cpp === --- test/clang-tidy/hicpp-signed-bitwise.cpp +++ test/clang-tidy/hicpp-signed-bitwise.cpp @@ -41,9 +41,12 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator UResult = SValue & -1; // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult&= 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator UResult = UValue & 1u; // Ok UResult = UValue & UValue; // Ok + UResult&= 2u; // Ok unsigned char UByte1 = 0u; unsigned char UByte2 = 16u; @@ -68,18 +71,30 @@ UByte1 = UByte1 | UByte2; // Ok UByte1 = UByte1 | SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= UByte2; // Ok UByte1 = UByte1 ^ UByte2; // Ok UByte1 = UByte1 ^ SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= UByte2; // Ok UByte1 = UByte1 >> UByte2; // Ok UByte1 = UByte1 >> SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= UByte2; // Ok UByte1 = UByte1 << UByte2; // Ok UByte1 = UByte1 << SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1<<= SByte2; + // 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 @@ -195,10 +210,16 @@ int s3; s3 = IntOne | IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3|= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne & IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3&= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne ^ IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3^= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 | s2; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 & s2; // Signed Index: test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp === --- test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp +++ test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp @@ -8,51 +8,57 @@ std::locale::category C = std::locale::category::ctype; SResult = std::locale::category::none | std::locale::category::collate; + SResult|= std::locale::category::collate; SResult = std::locale::category::ctype & std::locale::category::monetary; + SResult&= std::locale::category::monetary; SResult = std::locale::category::numeric ^ std::locale::category::time; + SResult^= std::locale::category::time; SResult = std::locale::category::messages | std::locale::category::all; SResult = std::locale::category::all & C; + SResult&= std::locale::category::all; SResult = std::locale::category::all | C; + SResult|= std::locale::category::all; SResult = std::locale::category::all ^ C; + SResult^= std::locale::category::all; // std::ctype_base::mask std::ctype_base::mask M = std::ctype_base::mask::punct; SResult = std::ctype_base::mask::space | std::ctype_base::mask::print; SResult = std::ctype_base::mask::cntrl & std::ctype_base::mask::upper; SResult = std::ctype_base::mask::lower ^
[PATCH] D45414: [clang-tidy] add missing assignment operations in hicpp-signed-bitwise
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, hokein, alexfh. Herald added subscribers: cfe-commits, xazax.hun, klimek. This patch resolves the bug https://bugs.llvm.org/show_bug.cgi?id=36963. - implement missing assignment operators for `hicpp-signed-bitwise` - add tests - mention fix in release notes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45414 Files: clang-tidy/hicpp/SignedBitwiseCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp test/clang-tidy/hicpp-signed-bitwise.cpp Index: test/clang-tidy/hicpp-signed-bitwise.cpp === --- test/clang-tidy/hicpp-signed-bitwise.cpp +++ test/clang-tidy/hicpp-signed-bitwise.cpp @@ -41,9 +41,12 @@ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator UResult = SValue & -1; // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult&= 1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator UResult = UValue & 1u; // Ok UResult = UValue & UValue; // Ok + UResult&= 2u; // Ok unsigned char UByte1 = 0u; unsigned char UByte2 = 16u; @@ -68,18 +71,30 @@ UByte1 = UByte1 | UByte2; // Ok UByte1 = UByte1 | SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1|= UByte2; // Ok UByte1 = UByte1 ^ UByte2; // Ok UByte1 = UByte1 ^ SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1^= UByte2; // Ok UByte1 = UByte1 >> UByte2; // Ok UByte1 = UByte1 >> SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= SByte2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator + UByte1>>= UByte2; // Ok UByte1 = UByte1 << UByte2; // Ok UByte1 = UByte1 << SByte2; // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator + UByte1<<= SByte2; + // 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 @@ -195,10 +210,16 @@ int s3; s3 = IntOne | IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3|= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne & IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3&= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = IntOne ^ IntTwo; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator + s3^= IntTwo; // Signed + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 | s2; // Signed // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator s3 = s1 & s2; // Signed Index: test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp === --- test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp +++ test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp @@ -8,51 +8,57 @@ std::locale::category C = std::locale::category::ctype; SResult = std::locale::category::none | std::locale::category::collate; + SResult|= std::locale::category::collate; SResult = std::locale::category::ctype & std::locale::category::monetary; + SResult&= std::locale::category::monetary; SResult = std::locale::category::numeric ^ std::locale::category::time; + SResult^= std::locale::category::time; SResult = std::locale::category::messages | std::locale::category::all; SResult = std::locale::category::all & C; + SResult&= std::locale::category::all; SResult = std::locale::category::all | C; + SResult|= std::locale::category::all; SResult = std::locale::category::all ^ C; + SResult^= std::locale::category::all; // std::ctype_base::mask std::ctype_base::mask M =
