lebedev.ri added a comment. In https://reviews.llvm.org/D39149#910891, @bcain wrote:
> In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote: > > > That is my diagnostic, so i guess this is the time to reply :) > > > ... > > > 3. In `-Wtautological-constant-compare`, ignore any comparisons that > > compare with `std::numeric_limits`. Not a fan of this solution. <- Worst? > > ... > > Gee, #3 is the worst? That one seems the most appealing to me. I think I > could understand it if you had reservations about ignoring comparisons > against {CHAR,SHRT,INT,LONG}_{MIN,MAX} but std::numeric_limits is > well-qualified and everything. > > Can you help me understand why you don't prefer it? That my initial bug was not caught by gcc either, even though `-Wtype-limits` was enabled. Because for whatever reason gcc basically does not issue diagnostics for templated functions. Even though the tautologically-compared variable was *not* dependent on the template parameters in any way. See for yourself, i think this is *really* bad: https://godbolt.org/g/zkq3UD So all-size-fits-all things like that are just asking to be done wrong. I fail to see how `ignore any comparisons that compare with std::numeric_limits` would not be the same pitfall, unfortunately. > Also, is there any way that the warning could somehow be smart enough to know > what sizeof(int) and sizeof(long) are? I'm not sure i follow. It sure does know that, else it would not possible to diagnose anything. Or were you talking about In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote: > That is my diagnostic, so i guess this is the time to reply :) > As i see it, there are several options: > ... > > 5. The essential problem, i *think* is much like the problem with unreachable > code diagnostic, CppCon 2017: Titus Winters “Hands-On With Abseil” > <https://youtu.be/xu7q8dGvuwk?t=16m5s>. The check does not know/care whether > the comparison is tautological only with the current Fundamental type sizes, > or always. Perhaps this is the proper heading? ? > As an aside, how widely has this new clang behavior been tested? Is it > possible that this new warning behavior might get rolled back if a > big/important enough codebase triggers it? Are you talking about false-positives* or about real true-positive warnings? So far, to the best of my knowledge, there were no reports of false-positives*. If a false-positive is reported, and it is not obvious how to fix it, i suppose it might be reverted. True-positives are obviously not the justification for the revert, but a validation of the validity of the diagnostic. The cases like this one are troublesome. **If** it is possible to reduce a case that obviously should not warn, **then** the diagnostic should be adjusted not to warn. - The fact that under *different* circumstances the comparison is not tautological is not a false-positive. https://reviews.llvm.org/D39149 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
