Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
Yeah, I tried to fix even that case but is not so simple so not worth any extra
time/complexivity.
> Dňa 8. 10. 2019 o 19:09 užívateľ Arthur O'Dwyer
> napísal:
>
>
> On Mon, Oct 7, 2019 at 6:58 PM Dávid Bolvanský
> wrote:
> >> FWIW I found the "always evaluates to 'true'" bit important to
> >> understand the warning.
> >
> > Yeah. I moved this check somewhere else, so we can print precise message:
> > r373973 should emit "bitwise negation of a boolean expression always
> > evaluates to 'true'; did you mean logical negation?" where possible.
> > In the suspicious case like int i = ~b there is a general message
> > "bitwise negation of a boolean expression; did you mean logical
> > negation?".
> >
> > I like it now. What do you think? fine for you?
>
> I see. Yes, all the cases I tried produce appropriate diagnostics. I like it!
>
>> Hm, there is no "bitwise negation of a boolean expression always
>> evaluates to 'true'; did you mean logical negation?" for chromium
>> case [ https://bugs.chromium.org/p/chromium/issues/detail?id=1011810 ]. I
>> will try to fix it.
>
> The important part there seems to be that the result of `~b` (which must be
> either -1 or -2) is used as the operand to `!=` or `==`.
> My opinion is that it is not worth the extra complication just to improve the
> error message for this case.
> It would be interesting to do some kind of general-purpose dataflow before
> emitting diagnostics...
> I notice that Clang's optimizer is smart enough to optimize
> bool foo(bool a, bool b) {
> return a == ~b;
> }
> bool bar(int x) {
> return x + 1 < -INT_MAX;
> }
> into `return 0`. If it could propagate that information up and produce a
> diagnostic, users might appreciate that. But the challenge as always is that
> we can never tell if the user might sometimes be doing that sort of thing on
> purpose (in inlined code, in macros, in generated code, etc).
>
> my $.02,
> –Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
On Mon, Oct 7, 2019 at 6:58 PM Dávid Bolvanský
wrote:
>> FWIW I found the "always evaluates to 'true'" bit important to
>> understand the warning.
>
> Yeah. I moved this check somewhere else, so we can print precise message:
> r373973 should emit "bitwise negation of a boolean expression always
> evaluates to 'true'; did you mean logical negation?" where possible.
> In the suspicious case like int i = ~b there is a general message
> "bitwise negation of a boolean expression; did you mean logical
> negation?".
>
> I like it now. What do you think? fine for you?
I see. Yes, all the cases I tried produce appropriate diagnostics. I like
it!
Hm, there is no "bitwise negation of a boolean expression always
> evaluates to 'true'; did you mean logical negation?" for chromium
> case [ https://bugs.chromium.org/p/chromium/issues/detail?id=1011810 ].
> I will try to fix it.
The important part there seems to be that the result of `~b` (which must be
either -1 or -2) is used as the operand to `!=` or `==`.
My opinion is that it is not worth the extra complication just to improve
the error message for this case.
It would be interesting to do some kind of *general*-purpose dataflow
before emitting diagnostics...
I notice that Clang's optimizer is smart enough to optimize
bool foo(bool a, bool b) {
return a == ~b;
}
bool bar(int x) {
return x + 1 < -INT_MAX;
}
into `return 0`. If it could propagate that information up and produce a
diagnostic, users might appreciate that. But the challenge as always is
that we can never tell if the user might sometimes be doing that sort of
thing on purpose (in inlined code, in macros, in generated code, etc).
my $.02,
–Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
Hm, there is no "bitwise negation of a boolean expression always evaluates to 'true'; did you mean logical negation?" for chromium case. I will try to fix it. ut 8. 10. 2019 o 0:03 Dávid Bolvanský napísal(a): > > "FWIW I found the "always evaluates to 'true'" bit important to > understand the warning." > > Yeah. I moved this check somewhere else, so we can print precise message: > r373973 should emit "bitwise negation of a boolean expression always > evaluates to 'true'; did you mean logical negation?" where possible. > In the suspicious case like int i = ~b there is a general message > "bitwise negation of a boolean expression; did you mean logical > negation?". > > I like it now. What do you think? fine for you? > > po 7. 10. 2019 o 17:29 Dávid Bolvanský napísal(a): > > > > Typo was fixed some days ago :) > > > > Odoslané z iPhonu > > > > Dňa 7. 10. 2019 o 17:22 užívateľ Arthur O'Dwyer > > napísal: > > > > > > On Mon, Oct 7, 2019 at 10:59 AM Dávid Bolvanský via cfe-commits > > wrote: > >> > >> Okey, I will see what I can do (I know I need to move checking code > >> somewhere else). > >> > >> > Dňa 7. 10. 2019 o 16:54 užívateľ Nico Weber > >> > napísal: > >> > FWIW I found the "always evaluates to 'true'" bit important to > >> > understand the warning. > > > > > > +1, I think "always evaluates to true" is useful, especially for people who > > don't immediately intuit the difference between "bitwise negation" and > > "logical negation." (Although the fixit will help clear up the difference.) > > > > Also, Dávid, you misspelled "logical" as "logicial" in the patch I saw. So > > you might need to push a fix for that typo, unless you already caught it. > > My suggested message follows— > > > > - "bitwise negation of a boolean expression; did you mean a logicial > > negation?">, > > + "bitwise negation of a boolean expression is always true; did you mean > > logical negation?">, > > > > my $.02, > > –Arthur ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
"FWIW I found the "always evaluates to 'true'" bit important to understand the warning." Yeah. I moved this check somewhere else, so we can print precise message: r373973 should emit "bitwise negation of a boolean expression always evaluates to 'true'; did you mean logical negation?" where possible. In the suspicious case like int i = ~b there is a general message "bitwise negation of a boolean expression; did you mean logical negation?". I like it now. What do you think? fine for you? po 7. 10. 2019 o 17:29 Dávid Bolvanský napísal(a): > > Typo was fixed some days ago :) > > Odoslané z iPhonu > > Dňa 7. 10. 2019 o 17:22 užívateľ Arthur O'Dwyer > napísal: > > > On Mon, Oct 7, 2019 at 10:59 AM Dávid Bolvanský via cfe-commits > wrote: >> >> Okey, I will see what I can do (I know I need to move checking code >> somewhere else). >> >> > Dňa 7. 10. 2019 o 16:54 užívateľ Nico Weber napísal: >> > FWIW I found the "always evaluates to 'true'" bit important to understand >> > the warning. > > > +1, I think "always evaluates to true" is useful, especially for people who > don't immediately intuit the difference between "bitwise negation" and > "logical negation." (Although the fixit will help clear up the difference.) > > Also, Dávid, you misspelled "logical" as "logicial" in the patch I saw. So > you might need to push a fix for that typo, unless you already caught it. > My suggested message follows— > > - "bitwise negation of a boolean expression; did you mean a logicial > negation?">, > + "bitwise negation of a boolean expression is always true; did you mean > logical negation?">, > > my $.02, > –Arthur ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
Typo was fixed some days ago :) Odoslané z iPhonu > Dňa 7. 10. 2019 o 17:22 užívateľ Arthur O'Dwyer > napísal: > > >> On Mon, Oct 7, 2019 at 10:59 AM Dávid Bolvanský via cfe-commits >> wrote: > >> Okey, I will see what I can do (I know I need to move checking code >> somewhere else). >> >> > Dňa 7. 10. 2019 o 16:54 užívateľ Nico Weber napísal: >> > FWIW I found the "always evaluates to 'true'" bit important to understand >> > the warning. > > +1, I think "always evaluates to true" is useful, especially for people who > don't immediately intuit the difference between "bitwise negation" and > "logical negation." (Although the fixit will help clear up the difference.) > > Also, Dávid, you misspelled "logical" as "logicial" in the patch I saw. So > you might need to push a fix for that typo, unless you already caught it. > My suggested message follows— > > - "bitwise negation of a boolean expression; did you mean a logicial > negation?">, > + "bitwise negation of a boolean expression is always true; did you mean > logical negation?">, > > my $.02, > –Arthur ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
Okey, I will see what I can do (I know I need to move checking code somewhere else). > Dňa 7. 10. 2019 o 16:54 užívateľ Nico Weber napísal: > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r373743 - [NFCI] Improve the -Wbool-operation's warning message
FWIW I found the "always evaluates to 'true'" bit important to understand
the warning.
We did hit this (at least once) in Chromium after all [1] (looks like a
real bug – nothing for you to do about that, and thanks for the warning),
and I don't think I would've understood what the warning wanted from me if
I hadn't seen the old warning text in the commit.
[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=1011810
On Fri, Oct 4, 2019 at 8:53 AM David Bolvansky via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: xbolva00
> Date: Fri Oct 4 05:55:13 2019
> New Revision: 373743
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373743=rev
> Log:
> [NFCI] Improve the -Wbool-operation's warning message
>
> Based on the request from the post commit review. Also added one new test.
>
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/test/Sema/warn-bitwise-negation-bool.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=373743=373742=373743=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct 4
> 05:55:13 2019
> @@ -6638,7 +6638,7 @@ def note_member_declared_here : Note<
> def note_member_first_declared_here : Note<
>"member %0 first declared here">;
> def warn_bitwise_negation_bool : Warning<
> - "bitwise negation of a boolean expression always evaluates to 'true'">,
> + "bitwise negation of a boolean expression; did you mean a logicial
> negation?">,
>InGroup>;
> def err_decrement_bool : Error<"cannot decrement expression of type
> bool">;
> def warn_increment_bool : Warning<
>
> Modified: cfe/trunk/test/Sema/warn-bitwise-negation-bool.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-bitwise-negation-bool.c?rev=373743=373742=373743=diff
>
> ==
> --- cfe/trunk/test/Sema/warn-bitwise-negation-bool.c (original)
> +++ cfe/trunk/test/Sema/warn-bitwise-negation-bool.c Fri Oct 4 05:55:13
> 2019
> @@ -12,9 +12,11 @@ typedef _Bool boolean;
> #endif
>
> void test(boolean b, int i) {
> - b = ~b; // expected-warning {{bitwise negation of a boolean expression
> always evaluates to 'true'}}
> + b = ~b; // expected-warning {{bitwise negation of a boolean expression;
> did you mean a logicial negation?}}
>// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
> - b = ~(b); // expected-warning {{bitwise negation of a boolean
> expression always evaluates to 'true'}}
> + b = ~(b); // expected-warning {{bitwise negation of a boolean
> expression; did you mean a logicial negation?}}
>// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
>b = ~i;
> + i = ~b; // expected-warning {{bitwise negation of a boolean expression;
> did you mean a logicial negation?}}
> + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"!"
> }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
