[PATCH] Switch CLogger to use cppformat

[Philip Taylor]

sys_vswprintf has many problems:

• The Windows implementation (wprintf.cpp) is horrid.
• Android doesn't support %hs/%ls (#2996) and requires ugly hacks.
• It possibly has other portability issues, so you need to be careful to test on every platform if you want to do anything unusual with it.
• It has a fixed-size output buffer.
• It's only used by CLogger.
• It uses wchar_t, which is a stupid type and everyone should use UTF-8 instead.

Proposal: Change CLogger to use ​cppformat instead of sys_vswprintf. The fmt::sprintf function has basically the same format string and function call syntax as sprintf, so it's relatively easy to drop in. (Boost Format has very different (and ugly) syntax.) It's more portable than libc-provided sprintf, and interacts better with C++ (e.g. it understands std::string arguments).

(In the long term we might want to change other things to use cppformat too (debug_printf, CConsole, CTextRenderer, maybe other miscellaneous users of various sprintf functions), but CLogger is enough for now.)

Possible downsides:

• Code bloat. There's a ton of templated code in every fmt::sprintf call, whereas sprintf calls merely push all the arguments onto the stack. On a Linux release build, I see the stripped executable growing from 6,269KB to 6,437KB (~3%) after applying these patches, though that isn't too bad.
• There's not really any compile-time type checking. But GCC only supports __attribute__((format ...)) on char* strings, not wchar_t*, so it wasn't previously used on CLogger anyway and there was no other checking, so this isn't a regression. There is at least enough compile-time checking to prevent you passing wstring or wchar_t*, or random objects that can't be converted to a recognised type.
• There is some run-time type checking, and it throws exceptions if it detects a mistake, instead of just printing garbage and only possibly crashing. Most log messages are in error paths, so they won't be well tested and are relatively likely to have type errors.

Does anyone have any other opinions on whether this is a good change?

[Philip Taylor]

(Seems to build okay on Windows too, after adding a #include "precompiled.h" and rejigging some #pragma warnings to disable C4512.)

[philip]

In 16176:

Import cppformat v0.11.0

Refs #3011.

[philip]

In 16182:

CLogger: Use cppformat instead of sys_vswprintf.

sys_vswprintf relies on platform-specific printf implementations, which vary widely between platforms (in handling of truncation, return values, use of %s/%S/%hs/%ls for mixing char and wchar_t strings, etc) and are therefore a pain.

Use cppformat's fmt::sprintf instead, which has very similar syntax to sprintf but is more C++ish and is portable.

Also, wchar_t is stupid, so use char* strings (which are expected to be UTF-8) in CLogger. This creates a bit of a pain with changing all callers to convert to char* strings, but that's their fault for not using UTF-8 already.

Refs #3011.

[Philip Taylor]

Fixed with r16176, r16177, r16178, r16179, r16180, r16181, r16182, r16183, r16184, r16185, r16186, r16187, r16188, r16189, r16190, r16191, r16192, r16193.

Still to do:

• Communicate with cppformat upstream to report the changes I made to our copy, so hopefully they can apply similar changes and limit divergence.
• Think about updating to the latest release ("0.12.0", which is sorted like a much older version than "v0.11.0" in the tag list, so I accidentally picked up v0.11.0 instead).

[Philip Taylor]

Also r16208.

[leper]

We should create a new ticket to upgrade cppformat, and also to upstream the changes.