[PATCH] D39149: [libc++] Prevent tautological comparisons

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D39149#910825, @mclow.lists wrote:

> I dislike this change fairly strongly.


Agreed. Would you be happier with just disabling the diagnostics around the problematic parts?

In https://reviews.llvm.org/D39149#910845, @lebedev.ri wrote:

> 1. Disable that warning in `libc++` cmakelists. Be careful though, i think it might disable the entire 'tautological comparison' diagnostic family.


Note that one of the tautological comparisons is in a header, so it affects all libc++ clients and not just libc++ itself.

> 2. Use preprocessor pragmas to disable the diagnostic for the selected code, https://reviews.llvm.org/rL315882. Brittle, ugly, effective, has the least impact. <- Best?

I was considering doing this instead. It also seems ugly in its own way, but possibly less ugly than this patch.

> 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?

Yeah, exactly. If the warning could only fire when the comparison was *always* going to be tautological, that would avoid any issues like this, but I'm not sure how best to go about that.


https://reviews.llvm.org/D39149