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

From Andres Freund
Subject Re: pg_attribute_noreturn(), MSVC, C11
Date
Msg-id w7uerqxgt7v2qpv2wiciepvoi2geg25r2gtgfy27lfyinqug75@lwrcmkortli5
Whole thread Raw
In response to Re: pg_attribute_noreturn(), MSVC, C11  (Peter Eisentraut <peter@eisentraut.org>)
List pgsql-hackers
Hi,

On 2024-12-14 18:18:35 +0100, Peter Eisentraut wrote:
> On 13.12.24 20:54, Andres Freund wrote:
> > Another wrinkle: While __attribute__((noreturn)) works for function pointers
> > (or function pointer typedefs) _Noreturn doesn't. Gah.  We only use it that
> > way in two places, but still :(
> 
> Yeah, I wrote an experimental patch for noreturn support some years ago, and
> that was also my result back then.

It's quite annoying...


> (I assume you have a current patch, otherwise I can dig out that one.)

Yea, I do. Not pretty, but ...

I guess I'll try to pretty it up a bit and post it then.


> I had also written down that there were some problems with Perl and Tcl
> headers, FWIW.  Did you have any problems with those?

Not so far.


> I think we can take a small loss here and move with the standard. Unless you
> can think of a way to define pg_noreturn_but_for_function_pointers in a
> systematic way.

The small loss unfortunately isn't that small, because clang treats
__attribute__((noreturn)) to be part of the function signature, but not
_Noreturn. Which means you can't just put __attribute__((noreturn)) to the
function pointer's signature, because it'll complain about incompatible
function pointers:

../../../../../home/andres/src/postgresql/src/backend/backup/basebackup_incremental.c:179:20: error: incompatible
functionpointer types assigning to 'json_manifest_error_callback' (aka 'void (*)(struct JsonManifestParseContext *,
constchar *, ...) __attribute__((noreturn))') from 'void (JsonManifestParseContext *, const char *, ...)' (aka 'void
(structJsonManifestParseContext *, const char *, ...)') [-Wincompatible-function-pointer-types]
 
  179 |         context->error_cb = manifest_report_error;

A workaround would be to have pg_nodiscard to just specify both
__attribute__((noreturn)) and _Nodiscard, and
pg_noreturn_but_for_function_pointers just specify __attribute__((noreturn)).
But at that point it's not obvious why we'd use _Nodiscard at all.

I wonder if we should switch to pg_nodiscard in the position that _Nodiscard
would be used and implement it as __attribute__((noreturn)) on gnu like
compilers and __declspec(noreturn) on msvc.


Kinda curious that we have pg_nodiscard, but didn't use __declspec(nodiscard).

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: pg_attribute_noreturn(), MSVC, C11
Next
From: "Andrey M. Borodin"
Date:
Subject: Re: bt_index_parent_check and concurrently build indexes