[libc++] Ignore bogus tautologic comparison warnings

[smeenai]

Summary

clang 6 has a new diagnostic -Wtautological-constant-compare, which
fires for the code in question when int and long have the same size (for
example, when compiling for a 32-bit target, or for Windows 64-bit). The
warning is somewhat bogus here, since the code is only tautological on
certain platforms, so just silence it.

[smeenai]

Remove stray comment

[zturner]

It would be better if we could just fix the code. If another compiler comes along and implements this same warning, we're going to have to fix it again. The only difference between any of this function is which cstdlib function is called. Why not just wrap all of this and put a cast inside the wrapped function, like:

template<> inline int as_integer(const string &func, const wstring &s, size_t *idx, int base) {
  return as_integer_helper<long,int>(func, s, idx, base, wcstol);
}

and then inside of this function put the range check and use a cast to silence the warning.

[mclow.lists]

In D41368#958865, @zturner wrote:

It would be better if we could just fix the code.

I disagree (See discussion of D39149). Even if it was "just this one place" (which it isn't), this affects users of clang, not just libc++.
This is a (small, but significant) barrier to people trying to write correct, portable generic code, and clang is just wrong to warn here.

As an aside, I definitely object to "just fix the code". The code is correct as written.
I think what you mean is "it would be better if we could just tell the compiler to shut up" (which is what you've proposed - but incompletely)

[smeenai]

@mclow.lists are you okay with this approach? I'm also fine using a cast to silence the warning, as @zturner suggested, but we should suppress the warning in some way, otherwise libc++ 6 is gonna have compile warnings with clang 6 out of the box, which isn't great.

A third alternative, which is the least invasive, though not complete in some sense: we just add -Wno-tautological-constant-compare to the compile flags for libc++ (in CMake), to suppress the warning during libc++'s compilation. There's still an instance of the warning in a header, but all other clients of the header should treat it as a system header (in which case warnings will be suppressed anyway). It's not targeted at all and could suppress legitimate instances of the warning though.

[smeenai]

Also FWIW I agree with you that the warning is completely bogus here, but it looks like the fix to that warning isn't gonna make it into clang 6, at least, so we have to adjust accordingly.

[bcain]

In D41368#959579, @smeenai wrote:

@mclow.lists are you okay with this approach? I'm also fine using a cast to silence the warning, as @zturner suggested, but we should suppress the warning in some way, otherwise libc++ 6 is gonna have compile warnings with clang 6 out of the box, which isn't great.

A third alternative, which is the least invasive, though not complete in some sense: we just add -Wno-tautological-constant-compare to the compile flags for libc++ (in CMake), to suppress the warning during libc++'s compilation. There's still an instance of the warning in a header, but all other clients of the header should treat it as a system header (in which case warnings will be suppressed anyway). It's not targeted at all and could suppress legitimate instances of the warning though.

I agree, if we're willing to disable the warning in libc++ builds we should be willing to disable it via pragma.

[smeenai]

The warning was removed from -Wall.

include/istream

#ifndef _LIBCPP_NO_EXCEPTIONS
     catch (...)
     {
         this->__set_badbit_and_consider_rethrow();
     }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     return *this;
 }
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-constant-compare"
+#endif
+
 template <class _CharT, class _Traits>
 basic_istream<_CharT, _Traits>&
 basic_istream<_CharT, _Traits>::operator>>(int& __n)
 {
 #ifndef _LIBCPP_NO_EXCEPTIONS
     try
     {
 #endif  // _LIBCPP_NO_EXCEPTIONS
#ifndef _LIBCPP_NO_EXCEPTIONS
     catch (...)
     {
         this->__set_badbit_and_consider_rethrow();
     }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     return *this;
 }
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic pop
+#endif // __clang__
+
 template<class _CharT, class _Traits>
 basic_istream<_CharT, _Traits>&
 operator>>(basic_istream<_CharT, _Traits>& __is, _CharT* __s)
 {
 #ifndef _LIBCPP_NO_EXCEPTIONS
     try
     {
 #endif  // _LIBCPP_NO_EXCEPTIONS

src/string.cpp

as_integer_helper(const string& func, const S& str, size_t* idx, int base, F f)
     return r;
 }
 
 template<typename V, typename S>
 inline
 V
 as_integer(const string& func, const S& s, size_t* idx, int base);
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-constant-compare"
+#endif
+
 // string
 template<>
 inline
 int
 as_integer(const string& func, const string& s, size_t* idx, int base )
 {
     // Use long as no Standard string to integer exists.
     long r = as_integer_helper<long>( func, s, idx, base, strtol );
     if (r < numeric_limits<int>::min() || numeric_limits<int>::max() < r)
         throw_from_string_out_of_range(func);
     return static_cast<int>(r);
 }
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic pop
+#endif
+
 template<>
 inline
 long
 as_integer(const string& func, const string& s, size_t* idx, int base )
 {
     return as_integer_helper<long>( func, s, idx, base, strtol );
 }
 

 template<>
 inline
 unsigned long long
 as_integer( const string& func, const string& s, size_t* idx, int base )
 {
     return as_integer_helper<unsigned long long>( func, s, idx, base, strtoull );
 }
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wtautological-constant-compare"
+#endif
+
 // wstring
 template<>
 inline
 int
 as_integer( const string& func, const wstring& s, size_t* idx, int base )
 {
     // Use long as no Stantard string to integer exists.
     long r = as_integer_helper<long>( func, s, idx, base, wcstol );
     if (r < numeric_limits<int>::min() || numeric_limits<int>::max() < r)
         throw_from_string_out_of_range(func);
     return static_cast<int>(r);
 }
 
+#if __clang_major__ >= 6
+#pragma clang diagnostic pop
+#endif
+
 template<>
 inline
 long
 as_integer( const string& func, const wstring& s, size_t* idx, int base )
 {
     return as_integer_helper<long>( func, s, idx, base, wcstol );
 }