pg_attribute_noreturn(), MSVC, C11

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: pg_attribute_noreturn(), MSVC, C11
Date: 2024-12-13 19:10:13
Message-ID: pxr5b3z7jmkpenssra5zroxi7qzzp6eswuggokw64axmdixpnk@zbwxuq7gbbcw
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I just encountered another
warning C4715: 'XYZ: not all control paths return a value

with msvc in CI in a case where it should be trivial for the compiler to
recognize that the function return isn't reachable.

Which made me check if these days msvc has something like gcc's
__attribute__((noreturn)).

And it turns out that yes! The _Noreturn attribute has been added to C11 and
msvc supports C11:
https://learn.microsoft.com/en-us/cpp/c-language/noreturn?view=msvc-170

Besides the _Noreturn keyword the standard also added a stdnoreturn.h which
provides the 'noreturn' macro.

I first thought we could just implement pg_attribute_noreturn() using
_Noreturn if available. However, our current pg_attribute_noreturn() is in the
wrong place for that to work :(. _Noreturn is to be placed with the return
type, whereas function attributes with the __attribute__(()) syntax are after
the parameter list.

So we probably can't transparently switch between these.

C11 has been out a while, so I'm somewhat inclined to adopt _Noreturn/noreturn
in a conditional way. Older compilers would still work, just not understand
noreturn.

One wrinkle: _Noreturn/noreturn have been deprecated in C23, because that
adopted C++11's attribute syntax (i.e. [[noreturn]]). But that's at least in
the same place as _Noreturn/return.

We can't remove [[noreturn]] with preprocessor magic, so it's not really
viable to use that for, uhm, quite a while.

If we were to use _Noreturn, I think it could just be something like:

I think it should suffice to do something like

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
#define pg_noreturn _Noreturn
#else
#define pg_noreturn
#endif

(or alternatively include stdreturn if __STDC_VERSION__ indicates support and
define a bare 'noreturn' if not)

For msvc that mean we'd need to add /std:c11 (or /std:c17) to the compiler
flags, as it otherwise it results in a weird mix of c89 an c99). But that
might be a good idea anyway. With one minor change [1] the tests pass with
msvc when using /std:c17.

Before realizing that we'd have to change our existing annotations and that
there's no way to use both old and new syntax, depending on compiler support,
I was thinking this would be an obvious thing to do. I'm still leaning on it
being worth it, but not as clearly as before.

For an example of _Noreturn being used: https://godbolt.org/z/j15d35dao

Greetings,

Andres Freund

[1] The msvc implementation of VA_ARGS_NARGS only works with the traditional
preprocessor, but C11 support enables the new one. But we can detect that
with something like
(!defined(_MSVC_TRADITIONAL) || _MSVC_TRADITIONAL)

See https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170
and https://learn.microsoft.com/en-us/cpp/build/reference/std-specify-language-standard-version?view=msvc-170

Without adapting the definition of VA_ARGS_NARGS compilation fails as it
results in the macro resulting in VA_ARGS_NARGS_(prefix, 63, 62, ...) in
the using code...

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2024-12-13 19:54:11 Re: pg_attribute_noreturn(), MSVC, C11
Previous Message Jeff Davis 2024-12-13 17:07:54 Re: fixing tsearch locale support