On Thu, 2019-02-14 at 10:38 -0500, Jason Merrill wrote: > On 2/6/19 9:23 PM, David Malcolm wrote: > > PR c++/88680 reports excess warnings from -Wtype-limits after the > > C++ > > FE's use of location wrappers was extended in r267272 for cases > > such as: > > > > const unsigned n = 8; > > static_assert (n >= 0 && n % 2 == 0, ""); > > > > t.C:3:18: warning: comparison of unsigned expression >= 0 is always > > true > > [-Wtype-limits] > > 3 | static_assert (n >= 0 && n % 2 == 0, ""); > > | ~~^~~~ > > > > The root cause is that the location wrapper around "n" breaks the > > suppression of the warning for the "if OP0 is a constant that is >= > > 0" > > case. > > > > This patch fixes it by calling fold_for_warn on OP0, extracting the > > constant. > > Is there a reason not to do this for OP1 as well?
There's an asymmetry in the warning; it's looking for a comparison of a LHS expression against an RHS constant 0, spelled as "0". If we fold_for_warn on the RHS, then that folding introduces a warning for expressions that aren't spelled as "0" but can be folded to 0, e.g., with: enum { FOO, BAR }; pr88680.C:34:9: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] 34 | if (n >= FOO) | ~~^~~~~~ pr88680.C:36:9: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits] 36 | if (n < FOO) | ~~^~~~~ which we didn't warn for before. We need to fold the LHS so that we can look through wrapper nodes, and around wrappers around enum values, to suppress the warning if the folded version of the LHS is a constant that fits in the data type (for the example given in the original message above). Dave