(Moving this discussion to hackers. Previously in [0].) Background:
The ereport_domain() macro has a pg_unreachable() which is meant to be
hit when the elevel >= ERROR so that the compiler can determine that
elog/ereport ERROR never returns. This works fine in gcc and clang but
MSVC seems to be unable to figure this out. This causes us to have to
maintain hundreds of lines like "return NULL; /* keep compiler quiet
*/" to avoid warnings.
What's causing this?:
You might imagine that this happens because pg_unreachable() isn't
working for MSVC, but you'd be wrong. It's because of the
double-evaluation mitigation of elevel in the ereport_domain() macro.
We have:
const int elevel_ = (elevel);
and because later we do:
if (elevel_ >= ERROR) \
pg_unreachable(); \
the MSVC compiler can't figure out that the pg_unreachable() is always
hit because it sees elevel_ as variable rather than constant, even
when the macro's elevel parameter is a compile-time constant.
Fixing it:
Ideally, MSVC would have something like __builtin_const_p(). Looking
around, I found [1], which hacks together a macro which does the same
job. The problem is, the macro uses _Generic(), which is C11. Tom
suggested we adjust the meson build scripts to make MSVC always build
with C11, which seems doable due to 8fd9bb1d9 making our minimum
Visual Studio version 2019, and going by [2], vs2019 supports C11.
So, I think that means we can adjust the meson build scripts to pass
/std:c11 when building in MSVC. The attached patch does this and
defines a pg_builtin_constant() macro and adjusts ereport_domain() to
use it.
One thing I've not done yet is adjust all other usages of
__builtin_const_p() to use pg_builtin_const().
I did a quick check of the postgres.exe size with and without this
change. It does save about 13KB, but that seems to be entirely from
the /std:c11 flag. None is due to the compiler emitting less code
after elog(ERROR) / ereport(ERROR) :-(
postgres.exe size:
master: 10_143_232 bytes
patched: 10_129_920 bytes
Does anyone have any objections or comments to any of the above?
David
[0]
https://postgr.es/m/caaphdvrfdxjbrv6kcx_ghkysufubndyssjppcjqigourfje...@mail.gmail.com
[1]
https://stackoverflow.com/questions/49480442/detecting-integer-constant-expressions-in-macros
[2]
https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/
v3-0001-Detect-elog-ERROR-can-t-return-in-MSVC-when-using.patch
Description: Binary data
