https://chromium.googlesource.com/chromium/src/+/master/docs/writing_clang_plugins.md#Don_t-write-a-clang-plugin

there's nothing chromium specific about this. but adding it to clang itself might be good.

do you have a link to the win bot?

https://codereview.chromium.org/2323483005/#ps200001

https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/295882/steps/compile%20%28with%20patch%29/logs/stdio

https://codereview.chromium.org/2323483005/diff/200001/base/trace_event/trace_log.cc

FAILED: obj/base/base/trace_log.obj 
ninja -t msvc -e environment.x64 -- E:\b\c\cipd\goma/gomacc.exe "E:\b\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\VC\bin\amd64/cl.exe" /nologo /showIncludes /FC @obj/base/base/trace_log.obj.rsp /c ../../base/trace_event/trace_log.cc /Foobj/base/base/trace_log.obj /Fd"obj/base/base_cc.pdb"
e:\b\c\b\win\src\base\trace_event\trace_log.cc(612): error C2220: warning treated as error - no 'object' file generated
e:\b\c\b\win\src\base\trace_event\trace_log.cc(612): warning C4806: '&': unsafe operation: no value of type 'bool' promoted to type 'base::trace_event::TraceLog::Mode' can equal the given constant

Line 612:

if (!enabled_modes_ & FILTERING_MODE)

So I guess the warning is "binary and of bool and this constant will always be false". I wonder if it warns if FILTERING_MODE happens to be 1.

Sorry for the bot link, I meant to but forgot to add it. But you found the right one.

> So I guess the warning is "binary and of bool and this constant will always be false". I wonder if it warns if FILTERING_MODE happens to be 1.

Oh I see what you mean. Lemme try:

cat test.cc
---------------
#include <stdio.h>

enum Flag {
  ONE = 1,
  TWO = 2
};

int main(int argc, char**) {
 printf("%d\n", (!argc & ONE));
 printf("%d\n", (!argc & TWO));
 return 0;
}
---------------

both g++ and clang build it quietly with -Wall -Wextra.

Instead MSVC:
D:\>cl test.cc
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24213.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cc
test.cc(10): warning C4806: '&': unsafe operation: no value of type 'bool' promo
ted to type 'Flag' can equal the given constant
Microsoft (R) Incremental Linker Version 14.00.24213.1


Note how the warning is only on line 10 (argc & TWO).
So, yes you are right.
The warning here is not about "your operator precedence is weird", but more about kind of  static analysis.

> there's nothing chromium specific about this. but adding it to clang itself might be good.
Now, here's the challenge.
From a pure C++ viewpoint I could see some borderline legit cases of doing this, like
if (!something & mask), which evaluates to 1 only if |something| is false and mask has its msb set to 1 (I don't remember, are you supposed to rely on the fact that true == 1, or is it impl specific?)
Although I feel that this should be extremely rare.

> there's nothing chromium specific about this. but adding it to clang itself might be good.
And yes, you are right again. I guess this could be some optional -Wsuspicious_operator_precedence that we'd enable in chromium? Or maybe just part of -Wextra?

"could see borderline legit use" isn't the bar for warnings. It's "does the warning have a high true positive and a low false positive rate" and the best way to find out is to implement it and see what it finds (similar precedent: http://llvm.org/devmtg/2011-11/Weber_Wennborg_UsingClangInChromium.pdf slide 17)

I wrote a patch for this today. I'm running it on chrome atm to measure false positive rate.

thakis@thakis:~/src/llvm-build$ cat test.cc
enum E { a };
void f(int x) {
  if (!x & a)
    ;
}
thakis@thakis:~/src/llvm-build$ bin/clang -c -Wall test.c
test.c:3:7: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses]
  if (!x & a)
      ^  ~
test.c:3:7: note: add parentheses after the '!' to evaluate the bitwise operator first
  if (!x & a)
      ^
       (    )
test.c:3:7: note: add parentheses around left hand side expression to silence this warning
  if (!x & a)
      ^
      ( )
1 warning generated.

tweaked it slightly and sent it out: https://reviews.llvm.org/D26035

http://llvm.org/viewvc/llvm-project?view=revision&revision=285310

https://codereview.chromium.org/2523073002 brought that warning in

And even though it's only been in the tree for 7h, it's already finding bugs: https://codereview.chromium.org/2510793003/#msg17

Thanks for asking for this :-)

>Status: Fixed
wow! Thanks Nico!

>  it's already finding bugs: https://codereview.chromium.org/2510793003/#msg17
Wow^2: caught bug in real-time?

++thanks;