[PATCH] D39462: [Sema] Implement -Wmaybe-tautological-constant-compare for when the tautologicalness is data model dependent

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 12 03:59:13 PST 2017

lebedev.ri added a comment.

In https://reviews.llvm.org/D39462#922826, @rjmccall wrote:

> I don't speak for the entire project, but I'm not sure I'm interested in the diagnostic you're actually offering to contribute here.  It may produce a warning on your specific test case, but I think it's really much too rigid and will lead to massive false positives.  I sketched the basics of a design that I think I could accept; if you don't want to implement it, that's your right, but that doesn't make me more likely to accept what you're willing to implement.

Just to reiterate that we are talking about the same thing here:

- https://reviews.llvm.org/D38101 is already merged. `-Wtautological-constant-compare` is here.
- There are cases when it warns for some target platform, but not the other, as complained in https://reviews.llvm.org/D39149, and post-review mails for https://reviews.llvm.org/D38101
- So far it seems all the cases reduce to

  #include <limits>
  #include <cstdint>
  int main() {
    using T1 = long;
    using T2 = int;
    T1 r;
    if (r < std::numeric_limits<T2>::min()) {}
    if (r > std::numeric_limits<T2>::max()) {}

- *This* differential (https://reviews.llvm.org/D39462) would find such cases, and issue them under different diagnostic, thus reducing the "false-positive" (it is an open question whether they are actual false-positives or not) rate of `-Wtautological-constant-compare`.

Are you suggesting me to drop this, and implement some other huge new diagnostic that may catch such cases before `-Wtautological-constant-compare`, thus preventing `-Wtautological-constant-compare` from triggering on that completely?



More information about the cfe-commits mailing list