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

[Roman Lebedev via cfe-commits]

And i have again forgot to add the appropriate -commits list from the start.
Is there any reason at all why phab does not/should not do that automatically?

---------- Forwarded message ----------
From: Roman Lebedev via Phabricator <reviews at reviews.llvm.org>
Date: Tue, Oct 31, 2017 at 8:44 PM
Subject: [PATCH] D39462: [Sema] Implement
-Wmaybe-tautological-constant-compare for when the tautologicalness is
data model dependent
To: lebedev.ri at gmail.com, richard at metafoo.co.uk, rjmccall at gmail.com,
dblaikie at gmail.com
Cc: mclow.lists at gmail.com, aaron.ballman at gmail.com,
bcain at codeaurora.org, smeenai at fb.com, jkorous at apple.com,
shenhan at google.com


lebedev.ri created this revision.
lebedev.ri added a project: clang.

As brought up in https://reviews.llvm.org/D39149, and post-review
mails for some other commits,
the current `-Wtautological-constant-compare` is dumb, much like
unreachable code diagnostic.

The common complaint is that it diagnoses the comparisons between an `int` and
`long` when compiling for a 32-bit target as tautological, but not when
compiling for 64-bit targets. The underlying problem is obvious: data model.
In most cases, 64-bit target is `LP64` (`int` is 32-bit, `long` and pointer are
64-bit), and the 32-bit target is `ILP32` (`int`, `long`, and pointer
are 32-bit).

I.e. the common pattern is: (pseudocode)

  #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()) {}
  }

The fix is simple: if the types of the values being compared are different, but
are of the same size, then we issue `-Wmaybe-tautological-constant-compare`
instead of `-Wtautological-constant-compare`.
That new diagnostic is *not* enabled by default/-Wall/-Wextra.

Caveat: i did not actually verify that ^ pseudocode actually triggers
the new diag.
Looking at AST, it should.


Repository:
  rL LLVM

https://reviews.llvm.org/D39462

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/maybe-tautological-constant-compare.c
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39462.121010.patch
Type: text/x-patch
Size: 18196 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171031/ac1812cc/attachment-0001.bin>