[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 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:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) xbolva00 wrote: > xbolva00 wrote: > > aaron.ballman

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Ok, I will fix it as you suggested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) jfb wrote: > xbolva00 wrote: > > xbolva00

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 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:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) xbolva00 wrote: > xbolva00 wrote: > >

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 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:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 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:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) aaron.ballman wrote: > This comment

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-01 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:11031-11032 // Do not diagnose macros. - if (Loc.isMacroID()) + if (Loc.isMacroID() || XorLHS.get()->getBeginLoc().isMacroID() || +

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 218258. xbolva00 added a comment. Fixed review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:54-55 +const SourceLocation Loc, +const Expr *SubLHS = nullptr, +const Expr *SubRHS = nullptr); +

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1652771 , @xbolva00 wrote: > >> tbh, I would appreciate if you would leave the definition of > >> diagnoseXorMisusedAsPow() where it is and add a forward declare of the > >> function earlier in the file. > > I

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> tbh, I would appreciate if you would leave the definition of >> diagnoseXorMisusedAsPow() where it is and add a forward declare of the >> function earlier in the file. I uploaded the patch almost 2 weeks ago and nobody complained so I thought it is okay. Uploaded,

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 218115. xbolva00 added a comment. Fixed review comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1649353 , @xbolva00 wrote: > In D66397#1647455 , @aaron.ballman > wrote: > > > Adding @rsmith to see if we can make forward progress on this patch again. > > > On the other

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-28 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. In D66397#1647455 , @aaron.ballman wrote: > Adding @rsmith to see if we can make forward progress on this patch again. On the other side, I don't want to waste Richard's time since I dont want to extend (variables and macros

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding @rsmith to see if we can make forward progress on this patch again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-25 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Another bug found by -Wxor-used-as-pow :) https://github.com/christinaa/PcapPlusPlus/commit/4646a004f5168bcb78fe2dce78afa08d794c1450 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-23 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I am strongly in favour to just land this as is. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-23 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: regehr. xbolva00 added a comment. >> I now agree that it makes sense to warn when the operands are macros or >> variables. I could re-add macro support and then @jfb or @regehr would blame this diagnostic because of macro support =] variables could open a new box

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-23 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. In D66397#1642012 , @Quuxplusone wrote: > @thakis wrote: > > > What was the motivation for firing on more than bare literals? > > Well, fundamentally, there is no difference in code quality between any of > these snippets: > >

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. @thakis wrote: > What was the motivation for firing on more than bare literals? Well, fundamentally, there is no difference in code quality between any of these snippets: #define BIG 300 double bigpower1() { return 10 ^ BIG; } static constexpr int BIG =

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216724. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/warn-xor-as-pow.cpp Index: test/SemaCXX/warn-xor-as-pow.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216723. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/warn-xor-as-pow.cpp Index: test/SemaCXX/warn-xor-as-pow.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Removed macro support (again...). I agree with Nico, this warning is on by default so should be as exact as possible without many false positives. While we all know that the Chromium's false positive case could be rewritten for better code readability, on the other

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216720. xbolva00 added a comment. Addressed Bruno Ricci's tips. Removed macro support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> What was the motivation for firing on more than bare literals? First patch supported diagnosing macros. There is no exact motivation why, simply “I just did it” to present working prototype. Based on review, things were adjusted. Some reviewers wanted to keep full

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639818 , @rsmith wrote: > The `ALPHA_OFFSET` code seems to be unnecessarily "clever" to me. I think the > warning can be suppressed by reversing the operands: > > `ALPHA_OFFSET ^ 2` > > But if I were maintaining

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> other warning that behaves differently for ordinary and hex literals Such literals should indicate that the user knows what he/she is doing (bit tricks, ..). I agree that there should be a additional docs (as we mentioned it and kindly pinged @hans) and not force

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. In D66397#1640685 , @xbolva00 wrote: > >> I think adding parens and casts are fairly well-understood to suppress > >> warnings. > > It should work here as well. #define ALPHA_OFFSET (3). Did anobody from > Chromium try it? > > >>

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. > This should not warn. Please verify that patch was applied correctly and you > use newly built clang with this patch. (I use arc patch DX) You were right, I messed up on my side. Sorry about the noise ! I don't have much to add to the macro yes/no discussion

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> I think adding parens and casts are fairly well-understood to suppress >> warnings. It should work here as well. #define ALPHA_OFFSET (3). Did anobody from Chromium try it? >> They have varying levels of C++ proficiency I consider things like hex decimals or

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. > I don't understand how you draw that conclusion, but the reason behind why we > went with that as a way to silence the diagnostic is because using the prefix > acts as a signal that the developer wants to do bit manipulation more than > just a decimal literal does.

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I think everybody should be fine with this now :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Implemented @Quuxplusone's idea to improve silence note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216478. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/warn-xor-as-pow.cpp Index: test/SemaCXX/warn-xor-as-pow.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216458. xbolva00 edited the summary of this revision. xbolva00 added a comment. Detect digit separators. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: lib/Sema/SemaExpr.cpp

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D66397#1639840 , @xbolva00 wrote: > Swap trick is cool, we can suggest it always(vs. ‘xor’) Well, you should avoid suggesting that the user rewrite `2 ^ 10` as `10 ^ 2`. Anyway, I think some of the observers here might be

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Swap trick is cool, we can suggest it always(vs. ‘xor’) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. The `ALPHA_OFFSET` code seems to be unnecessarily "clever" to me. I think the warning can be suppressed by reversing the operands: `ALPHA_OFFSET ^ 2` But if I were maintaining that code I would get rid of the xor hack entirely and use #define CHANNEL_OFFSET(i) (i)

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Could the following text be acceptable? replace expression with '0xA ^ 1' or ‘10 xor 1’ to silence this warning (In C mode without xor macro “replace expression with '0xA ^ 1' to silence this warning”) I am not fan of “the ‘xor’ alternative token” CHANGES SINCE

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> C++14 binary literals Thanks, noted. >> Suggest “xor” We could. Aaron what do you think? But I still like “0x” more. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639749 , @thakis wrote: > >> I think I missed the workaround. I only saw the suggestion to write 0x2 > >> instead of 2 which doesn't seem more clear at all to me. Was there another > >> fix suggestion? > > > >

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. >> I think I missed the workaround. I only saw the suggestion to write 0x2 >> instead of 2 which doesn't seem more clear at all to me. Was there another >> fix suggestion? > > Nope, you didn't miss it -- that was the suggestion. Using a hex notation is > the way we let

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I also strongly -1 to disabling the warning for macros. It's just a disservice to everyone, and the fact there's precedent doesn't mean it's the right solution. That being said, does using C++14 binary literals also silence the diag? It should. CHANGES SINCE LAST

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639696 , @thakis wrote: > In D66397#1635792 , @aaron.ballman > wrote: > > > In D66397#1635745 , @xbolva00 > > wrote: > > > > > I

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. In D66397#1635792 , @aaron.ballman wrote: > In D66397#1635745 , @xbolva00 wrote: > > > I think it is too soon to jump and disable all macros. We still catch all > > motivating cases, it

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639590 , @xbolva00 wrote: > But it seems everything is autogenerated and nothing is “custom message”. Any > custom change is removed by next autogeneration. > > Possibly @hans could tell us if there is a way

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. But it seems everything is autogenerated and nothing is “custom message”. Any custom change is removed by next autogeneration. Possibly @hans could tell us if there is a way CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639376 , @xbolva00 wrote: > I fixed and enabled this checker to warn for macros too. > > @aaron.ballman is right. Yes, as he said... If the user has a time to add > -Wno-xor to build systems, he/she has also

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I fixed and enabled this checker to warn for macros too. @aaron.ballman is right. Yes, as he said... If the user has a time to add -Wno-xor to build systems, he/she has also time to improve code readability and use hexadecimal literals. But I am a bit worried

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 216399. xbolva00 added a comment. Fixed review nits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: lib/Sema/SemaExpr.cpp test/SemaCXX/warn-xor-as-pow.cpp Index:

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1639185 , @xbolva00 wrote: > I agree. And I think they should just use const int instead of macros after > all. > > Maybe we should just improve silence hint if rhs is macro? > #define ALPHA_OFFSET 0x3 ? I

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I agree. And I think they should just use const int instead of macros after all. Maybe we should just improve silence hint if rhs is macro? #define ALPHA_OFFSET 0x3 ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1635857 , @xbolva00 wrote: > >> suggested source-code fixit of #define ALPHA_OFFSET 0x3 > > > No, there is a note “replace with 0x2 ^ ALPHA_OFFSET” to silence it. Didnt > you suggest 0x2 / 0xA? :) > > While

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> File >> /home/bruno/software/llvm-project/clang/test/SemaCXX/warn-xor-as-pow.cpp >> Line 119: result of '2 ^ ALPHA_OFFSET' is 1; did you mean '1 << >> ALPHA_OFFSET' (8)? This should not warn. Please verify that patch was applied correctly and you use newly

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. Is the test up-to-date ? I tried to apply the patch locally and I get a bunch of failures : error: 'warning' diagnostics expected but not seen: File /home/bruno/software/llvm-project/clang/test/SemaCXX/warn-xor-as-pow.cpp Line 73: result of '2 ^ 64' is 66;

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Any objections to this patch except “macro” question? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> suggested source-code fixit of #define ALPHA_OFFSET 0x3 No, there is a note “replace with 0x2 ^ ALPHA_OFFSET” to silence it. While we can suggest as you wrote: #define ALPHA_OFFSET 0x3 I really dont think it is worth to do. CHANGES SINCE LAST ACTION

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D66397#1635796 , @xbolva00 wrote: > Now, Chromium disabled this warning :( until we “fix” it. If Chromium is willing to make build-system changes to add `-Wno-xor-as-pow`, then surely they should also be willing to make

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: hans. xbolva00 added a comment. >> I think it's too soon to disable any macros. It is hard to say (this was discussed a lot). @tkanis or @hans what do you think? Maybe you could do experiments for us. Comment the code which disables this warning in macros and try

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1635745 , @xbolva00 wrote: > I think it is too soon to jump and disable all macros. We still catch all > motivating cases, it found bugs in Chromium. I think it's too soon to disable any macros. The workaround

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. I think it is too soon to jump and disable all macros. We still catch all motivating cases, it found bugs in Chromium. Chromium's case makes sense but on the other hand, LHS does not make much sense to be a macro. #define TWO 2 and res = TWO ^ 32? And even for this

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D66397#1635715 , @xbolva00 wrote: > I agree what @tkanis suggested and be silent if RHS is macro as real world > code shows it. Opinions? > > @jfb @aaron.ballman I suspect we can come up with examples where the macro

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 215957. xbolva00 added a comment. Do not warn if RHS is macro. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/ https://reviews.llvm.org/D66397 Files: lib/Sema/SemaExpr.cpp test/SemaCXX/warn-xor-as-pow.cpp Index:

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: jfb. xbolva00 added a comment. I agree what @tkanis suggested and be silent if RHS is macro as real world code shows it. Opinions? @jfb @aaron.ballman Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66397/new/

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-19 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. From post commit discussion, Nico Weber: 1. Hi, 2. 3. this results in a false positive on webrtc, on this code: 4. 5. https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c=package:chromium=1002 6. 7. The code does this:

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-08-18 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 created this revision. xbolva00 added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. Handle specific case (2 ^ 64) - 1 Repository: rC Clang https://reviews.llvm.org/D66397 Files: lib/Sema/SemaExpr.cpp