Make #else/#endif comments more consistent

From: Anton Voloshin <a(dot)voloshin(at)postgrespro(dot)ru>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Make #else/#endif comments more consistent
Date: 2022-08-29 09:38:45
Message-ID: 2b315693-9b08-08b3-8b1a-d6b931e44b50@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Make-else-endif-comments-more-consistent.patch text/x-patch 36.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-08-29 09:42:05 Re: Reducing the chunk header sizes on all memory context types
Previous Message Amit Kapila 2022-08-29 09:37:46 Re: Reducing the chunk header sizes on all memory context types