Re: pg_attribute_noreturn(), MSVC, C11 - Mailing list pgsql-hackers

Hi,

On 2025-03-10 13:27:07 +0100, Peter Eisentraut wrote:
> From: Peter Eisentraut <peter@eisentraut.org>
> Date: Thu, 13 Feb 2025 16:01:06 +0100
> Subject: [PATCH v3 1/4] pg_noreturn to replace pg_attribute_noreturn()
> 
> We want to support a "noreturn" decoration on more compilers besides
> just GCC-compatible ones, but for that we need to move the decoration
> in front of the function declaration instead of either behind it or
> wherever, which is the current style afforded by GCC-style attributes.
> Also rename the macro to "pg_noreturn" to be similar to the C11
> standard "noreturn".
> 
> pg_noreturn is now supported on all compilers that support C11 (using
> _Noreturn), as well as MSVC (using __declspec).  (When PostgreSQL
> requires C11, the latter variant can be dropped.)  (We don't need the
> previously used variant for GCC-compatible compilers using
> __attribute__, because all reasonable candidates already support C11,
> so that variant would be dead code in practice.)
> 
> Now, all supported compilers effectively support pg_noreturn, so the
> extra code for !HAVE_PG_ATTRIBUTE_NORETURN can be dropped.
> 
> This also fixes a possible problem if third-party code includes
> stdnoreturn.h, because then the current definition of
> 
>     #define pg_attribute_noreturn() __attribute__((noreturn))
> 
> would cause an error.
> 
> Note that the C standard does not support a noreturn attribute on
> function pointer types.  So we have to drop these here.  There are
> only two instances at this time, so it's not a big loss.

That's still somewhat sad, but it's probably not worth having a separate
attribute just for those cases...

With the minor comment suggestion below, I think this looks good.


> diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
> index ec8eddad863..d32c0d318fb 100644
> --- a/src/backend/utils/mmgr/slab.c
> +++ b/src/backend/utils/mmgr/slab.c
> @@ -601,8 +601,8 @@ SlabAllocFromNewBlock(MemoryContext context, Size size, int flags)
>   *        want to avoid that.
>   */
>  pg_noinline
> +pg_noreturn
>  static void
> -pg_attribute_noreturn()
>  SlabAllocInvalidSize(MemoryContext context, Size size)
>  {
>      SlabContext *slab = (SlabContext *) context;

Hm, is it good to put the pg_noreturn after the pg_noinline?


> +/*
> + * pg_noreturn corresponds to the C11 noreturn/_Noreturn function specifier.
> + * We can't use the standard name "noreturn" because some third-party code
> + * uses __attribute__((noreturn)) in headers, which would get confused if
> + * "noreturn" is defined to "_Noreturn", as is done by <stdnoreturn.h>.
> + */
> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
> +#define pg_noreturn _Noreturn
> +#elif defined(_MSC_VER)
> +#define pg_noreturn __declspec(noreturn)
> +#else
> +#define pg_noreturn
> +#endif

I think it'd be good to add a comment explaining the placement of pg_noreturn
in a declaration, it's getting pickier...


> Subject: [PATCH v3 2/4] Add another pg_noreturn

WFM


> Subject: [PATCH v3 3/4] Swap order of extern/static and pg_nodiscard
> 
> Clang in C23 mode requires all attributes to be before extern or
> static.  So just swap these.  This also keeps the order consistent
> with the previously introduced pg_noreturn.

WFM.


> From 0d9f2f63e2166592bd703320cf18dfb61a47ab28 Mon Sep 17 00:00:00 2001
> From: Peter Eisentraut <peter@eisentraut.org>
> Date: Sat, 28 Dec 2024 11:00:24 +0100
> Subject: [PATCH v3 4/4] Support pg_nodiscard on non-GNU compilers that support
>  C23
> 
> Support pg_nodiscard on compilers that support C23 attribute syntax.
> Previously, only GCC-compatible compilers were supported.
> 
> Also, define pg_noreturn similarly using C23 attribute syntax if the
> compiler supports C23.  This doesn't have much of an effect right now,
> since all compilers that support C23 surely also support the existing
> C11-compatible code.  But it keeps pg_nodiscard and pg_noreturn
> consistent.

This is a bit unexciting, but I guess worth it.

Greetings,

Andres Freund