[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> When I create the new RFC patch, I'll try to get details on that and include >> it in the description. Great! Thanks. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. In D63423#2732209 , @xbolva00 wrote: >>> Perhaps that should warn even if the RHS is in hex form > > It would be kinda strange, since in one clang release we ask users to silence > warning with hex form and newer release would

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> Perhaps that should warn even if the RHS is in hex form It would be kinda strange, since in one clang release we ask users to silence warning with hex form and newer release would warn anyway. Not a fan of this decision. >> , or is an enumerator constant, or This

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. In D63423#2732199 , @xbolva00 wrote: > No, we want to preserve warning for 2 ^ MACRO or 10 ^ MACRO. Perhaps that should warn even if the RHS is in hex form, or is an enumerator constant, or is not even constant at all. libpng:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. No, we want to preserve warning for 2 ^ MACRO or 10 ^ MACRO. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. In D63423#2732186 , @xbolva00 wrote: > I remember that was interest to support macros too :) tbh I cant remember now > such real world case where “macro ^ cst” was an issue but.. it was a long > time ago ;) > > If you are want,

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I remember that was interest to support macros too :) tbh I cant remember now such real world case where “macro ^ cst” was an issue but.. it was a long time ago ;) If you are want, you can send patch to to avoid warning in this case, we can discuss it. Repository:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. In D63423#2732158 , @Quuxplusone wrote: > In D63423#2732152 , @hvdijk wrote: > >> It's bad enough that this warns for >> >> #define A 2 >> int f() { return A ^ 1; } >> >> where as far as the

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D63423#2732152 , @hvdijk wrote: > It's bad enough that this warns for > > #define A 2 > int f() { return A ^ 1; } > > where as far as the users of A are concerned... I see how this warning is arguably overzealous in the

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2021-05-02 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment. It's bad enough that this warns for #define A 2 #define B 0 int f() { return A ^ B; } where as far as the users of A are concerned, A is just some arbitrary value, it's just random chance it happens to be two and there is no chance of it being misinterpreted as

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-18 Thread Dávid Bolvanský via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL369217: [Diagnostics] Diagnose misused xor as pow (authored by xbolva00, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-16 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 215693. xbolva00 added a comment. Better comparison for "xor". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-16 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 215592. xbolva00 added a comment. Fixed nits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM aside from a few nits, but please give a chance for other reviewers to weigh in on their comments. Comment at: lib/Sema/SemaExpr.cpp:11025-11031 +

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-16 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Any futher comments or is it OK now @jfb ? @aaron.ballman ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; aaron.ballman wrote: > xbolva00 wrote: > >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 214534. xbolva00 added a comment. Addressed review notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; xbolva00 wrote: > jfb wrote: > > Does this match the naming that GCC ended up

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 214533. xbolva00 added a comment. Added UDL tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 214525. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Hello, I fixed your notes + added test cases. Only remaining question is what to do in "2 ^ 64" (65, 66, .. etc) case. I can emit "result of 2 ^ 64" is "XYZ", but this is rather poor diagnostic. What should we suggest for "2 ^ 64"? (@jfb The "2 ^ 64 - 1" case to be

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 214524. xbolva00 added a comment. svn added forgotten test file.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-08-10 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 214523. xbolva00 added a comment. Addressed review notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; aaron.ballman wrote: > xbolva00 wrote: > > xbolva00 wrote: > > > jfb wrote: >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > xbolva00 wrote: > > jfb wrote: > > > xbolva00

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > jfb wrote:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; jfb wrote: > xbolva00 wrote:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > Quuxplusone wrote: > > xbolva00 wrote: > > > jfb wrote: > >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; Quuxplusone wrote: > xbolva00

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > jfb wrote: > > Doesn't this match any expression

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10983 + +S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr); + } else if (LeftSideValue == 10) { Quuxplusone wrote: > Do you

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked 3 inline comments as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; jfb wrote: > Does this match the naming

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10950 + + // Do not diagnose binary literals. + if (ExprStr.find("0b") != llvm::StringRef::npos) I would rather see this whole section as a three-liner: // Do not diagnose binary, octal,

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; Does this match the naming that GCC ended up with? Comment at:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Any comments? If no, please approve.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205692. xbolva00 added a comment. Improved code, added const. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. David Malcon - GCC: “I think we'd want to *not* warn if either of the operands are from a macro expansion.“ But I see no reason to follow this and why we should restrict this even more.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1550768 , @xbolva00 wrote: > >> Perhaps the author can run the check over a large corpus of code to see > >> whether fps come up in practice? (The presence of fps will suggest to not > >> warn in macros, but the

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> Perhaps the author can run the check over a large corpus of code to see >> whether fps come up in practice? (The presence of fps will suggest to not >> warn in macros, but the absence of fps won't tell us too much.) Sorry, I have no time nor spare computers to

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205646. xbolva00 added a comment. Added dots in the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1550713 , @jfb wrote: > In D63423#1550705 , @aaron.ballman > wrote: > > > In D63423#1550697 , @jfb wrote: > > > > > What I meant

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D63423#1550728 , @jfb wrote: > In D63423#1550725 , @lebedev.ri > wrote: > > > I've always been frustrated at how clang just gives up as soon as it sees a > > macro. > > At best

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In D63423#1550725 , @lebedev.ri wrote: > I've always been frustrated at how clang just gives up as soon as it sees a > macro. > At best that should be controlled with some command-line flag. I cannot promise nothing, but

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In D63423#1550725 , @lebedev.ri wrote: > I've always been frustrated at how clang just gives up as soon as it sees a > macro. > At best that should be controlled with some command-line flag. This patch is not the place to change

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Okey, it should be fine now. If you are agree with the current state, please approve it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I've always been frustrated at how clang just gives up as soon as it sees a macro. At best that should be controlled with some command-line flag. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205641. xbolva00 added a comment. Reverted macro support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. I am also in the "be a bit conservative at first and see how things shake out" camp, but it's y'all doing the hard work here, not me :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In D63423#1550705 , @aaron.ballman wrote: > In D63423#1550697 , @jfb wrote: > > > What I meant with macros was that I don't think we should warn on: > > > > #define LEGIT(a, b) ({ work work

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In D63423#1550697 , @jfb wrote: > What I meant with macros was that I don't think we should warn on: > > #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) > > LEGIT(10, 5); > > > If the constants are inline

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1550697 , @jfb wrote: > What I meant with macros was that I don't think we should warn on: > > #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) > > LEGIT(10, 5); > > > If the constants are

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. What I meant with macros was that I don't think we should warn on: #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) LEGIT(10, 5); If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote `CONSTANT ^ CONSTANT`

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205636. xbolva00 added a comment. Added support to warn in macros. Renamed to -Wxor-used-as-pow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1550586 , @xbolva00 wrote: > ... But I go thru codesearch and this is just one case I think. > > I think we should warn in macros... In codesearch case ,they are many more > true positives, then false negatives. >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. ... But I go thru codesearch and this is just one case I think. I think we should warn in macros... In codesearch case ,they are many more true positives, then false negatives. #define MAX_T_U32 ((2^32)-1) Ouch. CHANGES SINCE LAST ACTION

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1550563 , @xbolva00 wrote: > We will be noisy in this case > > #define IRQ_CHINT4_LEVEL(11 ^ 7) > #define IRQ_CHINT3_LEVEL(10 ^ 7) > #define IRQ_CHINT2_LEVEL(9 ^ 7) > #define

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. We will be noisy in this case #define IRQ_CHINT4_LEVEL(11 ^ 7) #define IRQ_CHINT3_LEVEL(10 ^ 7) #define IRQ_CHINT2_LEVEL(9 ^ 7) #define IRQ_CHINT1_LEVEL(8 ^ 7) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D63423#1547244 , @regehr wrote: > In D63423#1547216 , @xbolva00 wrote: > > > The only remaining question is, as your said, whether or not to diagnose > > xors in macros. @regerh

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3306 +def warn_xor_used_as_pow_base_two : Warning< + "result of '%0' is %1, maybe you mean '%2' (%3)?">, + InGroup; We usually use "did you mean" in these kinds of

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13 +res = a ^ b; +res = 2 ^ 0; +res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}} jfb

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205202. xbolva00 added a comment. Removed unused diagnostic message. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In D63423#1547244 , @regehr wrote: > In D63423#1547216 , @xbolva00 wrote: > > > The only remaining question is, as your said, whether or not to diagnose > > xors in macros. @regerh @jfb ?

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205198. xbolva00 added a comment. Addressed review notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. In D63423#1547216 , @xbolva00 wrote: > The only remaining question is, as your said, whether or not to diagnose xors > in macros. @regerh @jfb ? IMO it's better to not warn about xors in macros-- I like the current tradeoffs.

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10931 + // Do not diagnose hexadecimal literals + if (ExprStr.find("0x") != llvm::StringRef::npos) +return; Quuxplusone wrote: > Can you use

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Thanks, I will address your notes soon! The only remaining question is, as your said, whether or not to diagnose xors in macros. @regerh @jfb ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10931 + // Do not diagnose hexadecimal literals + if (ExprStr.find("0x") != llvm::StringRef::npos) +return; Can you use `starts_with` (or the LLVM equivalent) in both of these cases?

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205187. xbolva00 added a comment. Handle 10 ^ -C case.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment. Just wanted to say that I think I agree with the design choices here! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/SemaCXX/warn-xor-as-pow.cpp:85 + res = XOR(10, 10); + res = 10^-7; +} What should we suggest here ? :) Nevative shift ? think we should diagnose it. I am not sure

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205140. xbolva00 added a comment. Removed useless else block with return. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205136. xbolva00 added a comment. Updated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked 2 inline comments as done. xbolva00 added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10924 + + if (LeftSideValue == 2 || LeftSideValue == 10) { +llvm::APInt XorValue = LeftSideValue ^ RightSideValue; Quuxplusone wrote: > Do you have

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. Seems low-value at first glance, but I guess I can't argue with those Twitter and codesearch results. Do you have codesearch results for `^2`? https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=%5E+2=Search Seems like a lot of false positives (and a lot of code

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/Sema/warn-xor-as-pow.c:45 + // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}} + res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/Sema/warn-xor-as-pow.c:45 + // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}} + res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205126. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/warn-xor-as-pow.c

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. Why have `.c` and `.cpp` tests? Comment at: test/Sema/warn-xor-as-pow.c:43 + res = TWO_ULL ^ 16; + res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; maybe you mean '1<<32', but shift count >= width of type}} + // expected-note@-1 {{replace

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205121. xbolva00 added a comment. Fixed notes by @jfb. Thank you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. What does the suggested for for `2 ^ 32` look like? I hope it's not `1 << 32` :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. I think you want to avoid warning inside macros as well, say: #define AWESOME(x, y) ({ blah blah blah; x ^ y; blah }) AWESOME(2, 10); // probably not a bug Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13 +res = a ^ b; +res = 2 ^ 0; +res

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. As a reference: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205089. xbolva00 added a comment. Silence note added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Should we also emit a note how to silence it, eg. swap xor operands or add parentheses around 2/10 ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205058. xbolva00 added a comment. More tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorAsPow : DiagGroup<"xor-as-pow">; GCC folks will do this diagnostic soon too, so I

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 created this revision. xbolva00 added reviewers: jfb, rsmith. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Motivation: https://twitter.com/jfbastien/status/1139298419988549632 https://twitter.com/mikemx7f/status/1139335901790625793