[libcxx] Silence -Wformat-nonliteral warnings in the Windows support code

[mstorsjo]

Summary

This brings the mingw build back to zero build warnings as it was
at some earlier time.

[Mordante]

LGTM!

[ldionne]

Would it make sense to turn these warnings into errors when building the library? That way we can never regress.

[mstorsjo]

In D119134#3308361, @ldionne wrote:

Would it make sense to turn these warnings into errors when building the library? That way we can never regress.

For building in CI configurations, I think it would make sense to tighten down as many configurations as possible with -Werror, to avoid regressing something that has been clean at some point. For non-CI builds, I don't think we should be building with -Werror by default though.

(The clang-cl builds are incredibly noisy these days, though, due to [[no_unique_address]] not being recognized in MSVC mode - I haven't had time to bring that issue up, and I'm not sure if we want to wrap the attribute in a macro or not.)

[Mordante]

libcxx/src/support/win32/locale_win32.cpp

100

I just noticed D119295 so maybe use the macros added there before landing. (Assuming that patch will be accepted.)

[mstorsjo]

libcxx/src/support/win32/locale_win32.cpp

100

Yes, maybe - I was hoping to backport this to 14.x to silence the build there too, so then it'd depend on whether that one (which is a bit bigger) is deemed OK to backport.

[philnik]

libcxx/src/support/win32/locale_win32.cpp

100

I can also hold back on D119295 until this PR is shipped. Just notify me on discord when you have.

[mstorsjo]

Enable LIBCXX_ENABLE_WERROR in the configurations that are fixed by this.

[• Quuxplusone]

FWIW this seems fine to me, but I'll let someone else be the final decider. :)

[ldionne]

LGTM, also looks fine to cherry-pick onto 14.x

[ldionne]

libcxx/utils/ci/run-buildbot

604 ↗

(On Diff #407456)

Would you be OK with moving it to the CMake cache instead?

[mstorsjo]

libcxx/utils/ci/run-buildbot

604 ↗

(On Diff #407456)

I think I’d omit this from this particular patch, and once D119430 lands, I’d make a different patch for enabling LIBCXX_ENABLE_WERROR - which would be opt-out for the few configs that don’t pass in that mode. But let’s move that to a later discussion.

libcxx/src/support/win32/locale_win32.cpp

int snprintf_l(char *ret, size_t n, locale_t loc, const char *format, ...)
     va_start( ap, format );
 #if defined(_LIBCPP_MSVCRT)
     // FIXME: Remove usage of internal CRT function and globals.
     int result = __stdio_common_vsprintf(
         _CRT_INTERNAL_LOCAL_PRINTF_OPTIONS | _CRT_INTERNAL_PRINTF_STANDARD_SNPRINTF_BEHAVIOR,
         ret, n, format, loc, ap);
 #else
     __libcpp_locale_guard __current(loc);
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     int result = vsnprintf( ret, n, format, ap );
+#pragma clang diagnostic pop
 #endif
     va_end(ap);
     return result;
 }
 
 int asprintf_l( char **ret, locale_t loc, const char *format, ... )
 {
     va_list ap;

libcxx/src/support/win32/support.cpp

 // a pointer to a malloc'd string in *sptr.
 // If return >= 0, use free to delete *sptr.
 int __libcpp_vasprintf( char **sptr, const char *__restrict format, va_list ap )
 {
     *sptr = NULL;
     // Query the count required.
     va_list ap_copy;
     va_copy(ap_copy, ap);
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     int count = vsnprintf( NULL, 0, format, ap_copy );
+#pragma clang diagnostic pop
     va_end(ap_copy);
     if (count < 0)
         return count;
     size_t buffer_size = static_cast<size_t>(count) + 1;
     char* p = static_cast<char*>(malloc(buffer_size));
     if ( ! p )
         return -1;
     // If we haven't used exactly what was required, something is wrong.
     // Maybe bug in vsnprintf. Report the error and return.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     if (vsnprintf(p, buffer_size, format, ap) != count) {
+#pragma clang diagnostic pop
         free(p);
         return -1;
     }
     // All good. This is returning memory to the caller not freeing it.
     *sptr = p;
     return count;
 }