Make #else/#endif comments more consistent
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46504psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 02:08 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t46504_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t46504_1 && git checkout t46504_1Patchset v1 (message #1) is on t46504_1
Hello,
while reading the postgres code, occasionally I see a little bit of
inconsistency in the comments after #else (and corresponding #endif).
In some places #else/endif's comment expresses condition for else block
to be active:
#ifdef HAVE_UUID_OSSP
...
#else /* !HAVE_UUID_OSSP */
...
#endif /* HAVE_UUID_OSSP */
and in others -- just the opposite:
#ifdef SHA2_UNROLL_TRANSFORM
...
#else /* SHA2_UNROLL_TRANSFORM */
...
#endif /* SHA2_UNROLL_TRANSFORM */
Also, #endif comment after #else might expresses condition for else
block to be active:
#ifdef USE_ICU
...
#else /* !USE_ICU */
...
#endif /* !USE_ICU */
or it might be just the opposite, like in HAVE_UUID_OSSP and
SHA2_UNROLL_TRANSFORM examples above.
I propose making them more consistent. Would the following guidelines be
acceptable?
1. #else/#elif/#endif's comment, if present, should reflect the
condition of the #else/#elif block as opposed to always being a copy
of #if/ifdef/ifndef condition.
e.g. prefer this:
#if LLVM_VERSION_MAJOR > 11
...
#else /* LLVM_VERSION_MAJOR <= 11 */
...
#endif /* LLVM_VERSION_MAJOR <= 11 */
over this:
#if LLVM_VERSION_MAJOR > 11
...
#else /* LLVM_VERSION_MAJOR > 11 */
...
#endif /* LLVM_VERSION_MAJOR > 11 */
2. In #else/#elif/#endif comments, prefer A to defined(A).
E.g. prefer this:
#endif /* DMETAPHONE_MAIN */
over
#endif /* defined DMETAPHONE_MAIN */
And this:
#else /* !_MSC_VER */
over
#else /* !defined(_MSC_VER) */
3. Textual hand-crafted condition comments are perfectly fine.
Like this:
#else /* no ppoll(), so use select() */
4. #else/#endif condition comment, if present, should reflect the
*effective* condition, i.e. condition taking into account previous
#if/#elif-s.
E.g. do this:
#if defined(HAVE_INT128)
...
#elif defined(HAS_64_BIT_INTRINSICS)
...
#else /* !HAVE_INT128 && !HAS_64_BIT_INTRINSICS */
...
#endif /* !HAVE_INT128 && !HAS_64_BIT_INTRINSICS */
5. Comment of the form "!A && !B", if deemed complicated enough, may
also be expressed as "neither A nor B" for easier reading.
Example:
#if (defined(HAVE_LANGINFO_H) && defined(CODESET)) || defined(WIN32)
...
#else /* neither (HAVE_LANGINFO_H && CODESET)
nor WIN32 */
...
#endif /* neither (HAVE_LANGINFO_H && CODESET)
nor WIN32 */
6. Use "!" as opposed to "not" to be consistent. E.g. do this:
#ifdef LOCK_DEBUG
...
#else /* !LOCK_DEBUG */
...
#endif /* !LOCK_DEBUG */
as opposed to:
#ifdef LOCK_DEBUG
...
#else /* not LOCK_DEBUG */
...
#endif /* not LOCK_DEBUG */
The draft of proposed changes is attached as
0001-Make-else-endif-comments-more-consistent.patch
In the patch I've also cleaned up some minor things, like removing
occasional "//" comments within "/* */" ones.
Any thoughts?
--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru
On 29.08.22 11:38, Anton Voloshin wrote:
I propose making them more consistent. Would the following guidelines be
acceptable?
I usually try to follow the guidelines in
<https://www.gnu.org/prep/standards/html_node/Comments.html>, which
pretty much match what you are proposing.
And this:
#else /* !_MSC_VER */
over
#else /* !defined(_MSC_VER) */
Note that for _MSC_VER in particular there is some trickiness: We
generally use it to tell apart different MSVC compiler versions. But it
is not present with MinGW. So !_MSC_VER and !defined(_MSC_VER) have
different meanings. So in this particular case, more precision in the
comments might be better.
On 29/08/2022 14:50, Peter Eisentraut wrote:
I usually try to follow the guidelines in
<https://www.gnu.org/prep/standards/html_node/Comments.html>, which
pretty much match what you are proposing.
Thank you for the link, it's a useful one and the wording is better than
mine.
Note that for _MSC_VER in particular there is some trickiness: We
generally use it to tell apart different MSVC compiler versions.
That's certainly true in branches <= 15, but in master, to my surprise,
I don't see any numerical comparisons of _MSC_VER since the recent
6203583b7.
I'm not sure explicit !defined(_MSC_VER) is all that more clear
than !_MSC_VER in the commentary. After all, we would probably
never (?) see an actual
#if (!_MSC_VER)
in a real code.
So I have mixed feelings on forcing define() on _MSC_VER, but if you
insist, I don't mind much either way.
What about other changes? Are there any obviously wrong or missed ones?
--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru