I'm in favor of splitting the warning into subgroups, then deciding which ones should be in -Wall. We've done this with other warnings such as the conversion warnings and tautological compare warnings, with only a subset of them in -Wall.

Heads up in case it affects you refactoring work:
While looking through this code, I found a bug I previously made. I fixed it with a small change, but that lies in the middle of your refactoring during FieldDecl handling. The change is here:

In D71503#1784697, @lebedev.ri wrote:

Thank you for working on this!
This seems to be missing tests.

In D69292#1747062, @mclow.lists wrote:

Sorry I'm late to the party; I've been traveling for 3+ weeks.
I would like to be reassured that the following code will not warn:

`
 long foo = ...; // some calculation
 if (foo < std::numeric_limits<int>::min() || foo > std::numeric_limits<int>::max()) .....

This is important for systems where sizeof(int) == sizeof(long)

Add -Wall tests to check that certain warning groups are active with it and a test to check all warning groups that are turned on by -Wall.

The recommited patch is hitting an assert. See message and reduced test case below:

In D69292#1718415, @thakis wrote:

Abstractly this lgtm. Do you have any data on true / false positive rates?

In D66046#1717229, @thakis wrote:

Mr Trieu, what do you think about adding some or all of the Wtautological-compare warnings to Wall

In D66046#1696785, @xbolva00 wrote:

Could this patch solve https://bugs.llvm.org/show_bug.cgi?id=43573?

Add more test cases and split new warnings into a separate warning group, but still under -Wparentheses

I have reverted this in r370006.

In D66044#1626008, @NoQ wrote:

Looks great, thanks! I'd appreciate direct CFG tests for the part that changes the CFG (cf. test/Analysis/cfg.cpp), but i don't insist. Let's make sure @jfb is happy and commit :)

Better handling of arrays in self compares.

Update comments to explain why bitwise-xor is treated as a logical operator.

Check array accesses.

Create new function isIntOrEnumConstantZero to use instead of EvaluateAsInt

In D66045#1624389, @jfb wrote:

Does this work for unions as well?

LGTM

In rL290920#660685, @xbolva00 wrote:

Hello,

do you know why this warning is not on by default? :/