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 GCC-compatible ones (using __attribute__, as
before), as well as MSVC (using __declspec).  (When PostgreSQL
requires C11, the latter two variants can be dropped.)

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.  In one case,
we can make up for it by adding the pg_noreturn to a wrapper function
and adding a pg_unreachable(), in the other case, the latter was
already done before.

Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/pxr5b3z7jmkpenssra5zroxi7qzzp6eswuggokw64axmdixpnk@zbwxuq7gbbcw

Author: petere

contrib/dblink/dblink.c

@@ -160,8 +160,7 @@ xpstrdup(const char *in)
 	return pstrdup(in);
 }
 
-static void
-pg_attribute_noreturn()
+pg_noreturn static void
 dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 {
 	char	   *msg = pchomp(PQerrorMessage(conn));
@@ -170,8 +169,7 @@ dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 	elog(ERROR, "%s: %s", p2, msg);
 }
 
-static void
-pg_attribute_noreturn()
+pg_noreturn static void
 dblink_conn_not_avail(const char *conname)
 {
 	if (conname)

contrib/pgcrypto/px.h

@@ -181,7 +181,7 @@ int			px_find_hmac(const char *name, PX_HMAC **res);
 int			px_find_cipher(const char *name, PX_Cipher **res);
 int			px_find_combo(const char *name, PX_Combo **res);
 
-void		px_THROW_ERROR(int err) pg_attribute_noreturn();
+pg_noreturn void px_THROW_ERROR(int err);
 const char *px_strerror(int err);
 
 const char *px_resolve_alias(const PX_Alias *list, const char *name);

src/backend/access/transam/xlogrecovery.c

@@ -4778,8 +4778,7 @@ check_primary_slot_name(char **newval, void **extra, GucSource source)
  * that we have odd behaviors such as unexpected GUC ordering dependencies.
  */
 
-static void
-pg_attribute_noreturn()
+pg_noreturn static void
 error_multiple_recovery_targets(void)
 {
 	ereport(ERROR,

src/backend/backup/basebackup_incremental.c

@@ -139,9 +139,9 @@ static void manifest_process_wal_range(JsonManifestParseContext *context,
 									   TimeLineID tli,
 									   XLogRecPtr start_lsn,
 									   XLogRecPtr end_lsn);
-static void manifest_report_error(JsonManifestParseContext *context,
-								  const char *fmt,...)
-			pg_attribute_printf(2, 3) pg_attribute_noreturn();
+pg_noreturn static void manifest_report_error(JsonManifestParseContext *context,
+											  const char *fmt,...)
+			pg_attribute_printf(2, 3);
 static int	compare_block_numbers(const void *a, const void *b);
 
 /*

src/backend/postmaster/autovacuum.c

@@ -317,7 +317,7 @@ int			AutovacuumLauncherPid = 0;
 
 static Oid	do_start_worker(void);
 static void ProcessAutoVacLauncherInterrupts(void);
-static void AutoVacLauncherShutdown(void) pg_attribute_noreturn();
+pg_noreturn static void AutoVacLauncherShutdown(void);
 static void launcher_determine_sleep(bool canlaunch, bool recursing,
 									 struct timeval *nap);
 static void launch_worker(TimestampTz now);

src/backend/postmaster/launch_backend.c

@@ -171,7 +171,7 @@ static pid_t internal_forkexec(const char *child_kind, int child_slot,
 typedef struct
 {
 	const char *name;
-	void		(*main_fn) (const void *startup_data, size_t startup_data_len) pg_attribute_noreturn();
+	void		(*main_fn) (const void *startup_data, size_t startup_data_len);
 	bool		shmem_attach;
 } child_process_kind;
 

src/backend/postmaster/postmaster.c

@@ -425,7 +425,7 @@ static void LogChildExit(int lev, const char *procname,
 static void PostmasterStateMachine(void);
 static void UpdatePMState(PMState newState);
 
-static void ExitPostmaster(int status) pg_attribute_noreturn();
+pg_noreturn static void ExitPostmaster(int status);
 static int	ServerLoop(void);
 static int	BackendStartup(ClientSocket *client_sock);
 static void report_fork_failure_to_client(ClientSocket *client_sock, int errnum);

src/backend/replication/logical/tablesync.c

@@ -139,8 +139,7 @@ static StringInfo copybuf = NULL;
 /*
  * Exit routine for synchronization worker.
  */
-static void
-pg_attribute_noreturn()
+pg_noreturn static void
 finish_sync_worker(void)
 {
 	/*

src/backend/replication/walsender.c

@@ -237,7 +237,7 @@ typedef void (*WalSndSendDataCallback) (void);
 static void WalSndLoop(WalSndSendDataCallback send_data);
 static void InitWalSenderSlot(void);
 static void WalSndKill(int code, Datum arg);
-static void WalSndShutdown(void) pg_attribute_noreturn();
+pg_noreturn static void WalSndShutdown(void);
 static void XLogSendPhysical(void);
 static void XLogSendLogical(void);
 static void WalSndDone(WalSndSendDataCallback send_data);

src/backend/utils/adt/ri_triggers.c

@@ -235,10 +235,10 @@ static bool ri_PerformCheck(const RI_ConstraintInfo *riinfo,
 static void ri_ExtractValues(Relation rel, TupleTableSlot *slot,
 							 const RI_ConstraintInfo *riinfo, bool rel_is_pk,
 							 Datum *vals, char *nulls);
-static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
-							   Relation pk_rel, Relation fk_rel,
-							   TupleTableSlot *violatorslot, TupleDesc tupdesc,
-							   int queryno, bool is_restrict, bool partgone) pg_attribute_noreturn();
+pg_noreturn static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
+										   Relation pk_rel, Relation fk_rel,
+										   TupleTableSlot *violatorslot, TupleDesc tupdesc,
+										   int queryno, bool is_restrict, bool partgone);
 
 
 /*

src/backend/utils/fmgr/dfmgr.c

@@ -67,8 +67,8 @@ static DynamicFileList *file_tail = NULL;
 char	   *Dynamic_library_path;
 
 static void *internal_load_library(const char *libname);
-static void incompatible_module_error(const char *libname,
-									  const Pg_magic_struct *module_magic_data) pg_attribute_noreturn();
+pg_noreturn static void incompatible_module_error(const char *libname,
+												  const Pg_magic_struct *module_magic_data);
 static char *expand_dynamic_library_name(const char *name);
 static void check_restricted_library_name(const char *name);
 static char *substitute_libpath_macro(const char *name);

src/backend/utils/hash/dynahash.c

@@ -272,7 +272,7 @@ static HASHBUCKET get_hash_entry(HTAB *hashp, int freelist_idx);
 static void hdefault(HTAB *hashp);
 static int	choose_nelem_alloc(Size entrysize);
 static bool init_htab(HTAB *hashp, long nelem);
-static void hash_corrupted(HTAB *hashp) pg_attribute_noreturn();
+pg_noreturn static void hash_corrupted(HTAB *hashp);
 static uint32 hash_initial_lookup(HTAB *hashp, uint32 hashvalue,
 								  HASHBUCKET **bucketptr);
 static long next_pow2_long(long num);

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;

src/bin/pg_combinebackup/load_manifest.c

@@ -68,9 +68,9 @@ static void combinebackup_per_wal_range_cb(JsonManifestParseContext *context,
 										   TimeLineID tli,
 										   XLogRecPtr start_lsn,
 										   XLogRecPtr end_lsn);
-static void report_manifest_error(JsonManifestParseContext *context,
-								  const char *fmt,...)
-			pg_attribute_printf(2, 3) pg_attribute_noreturn();
+pg_noreturn static void report_manifest_error(JsonManifestParseContext *context,
+											  const char *fmt,...)
+			pg_attribute_printf(2, 3);
 
 /*
  * Load backup_manifest files from an array of backups and produces an array

src/bin/pg_dump/pg_backup_utils.h

@@ -29,7 +29,7 @@ extern const char *progname;
 
 extern void set_dump_section(const char *arg, int *dumpSections);
 extern void on_exit_nicely(on_exit_nicely_callback function, void *arg);
-extern void exit_nicely(int code) pg_attribute_noreturn();
+pg_noreturn extern void exit_nicely(int code);
 
 /* In pg_dump, we modify pg_fatal to call exit_nicely instead of exit */
 #undef pg_fatal

src/bin/pg_upgrade/pg_upgrade.h

@@ -481,7 +481,7 @@ int			get_user_info(char **user_name_p);
 void		check_ok(void);
 void		report_status(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3);
 void		pg_log(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3);
-void		pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2) pg_attribute_noreturn();
+pg_noreturn void pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2);
 void		end_progress_output(void);
 void		cleanup_output_dirs(void);
 void		prep_status(const char *fmt,...) pg_attribute_printf(1, 2);

src/bin/pg_verifybackup/pg_verifybackup.c

@@ -69,9 +69,9 @@ static void verifybackup_per_wal_range_cb(JsonManifestParseContext *context,
 										  TimeLineID tli,
 										  XLogRecPtr start_lsn,
 										  XLogRecPtr end_lsn);
-static void report_manifest_error(JsonManifestParseContext *context,
-								  const char *fmt,...)
-			pg_attribute_printf(2, 3) pg_attribute_noreturn();
+pg_noreturn static void report_manifest_error(JsonManifestParseContext *context,
+											  const char *fmt,...)
+			pg_attribute_printf(2, 3);
 
 static void verify_tar_backup(verifier_context *context, DIR *dir);
 static void verify_plain_backup_directory(verifier_context *context,

src/bin/pg_verifybackup/pg_verifybackup.h

@@ -98,8 +98,8 @@ typedef struct verifier_context
 extern void report_backup_error(verifier_context *context,
 								const char *pg_restrict fmt,...)
 			pg_attribute_printf(2, 3);
-extern void report_fatal_error(const char *pg_restrict fmt,...)
-			pg_attribute_printf(1, 2) pg_attribute_noreturn();
+pg_noreturn extern void report_fatal_error(const char *pg_restrict fmt,...)
+			pg_attribute_printf(1, 2);
 extern bool should_ignore_relpath(verifier_context *context,
 								  const char *relpath);
 

src/bin/pgbench/pgbench.h

@@ -140,9 +140,9 @@ struct PgBenchExprList
 
 extern int	expr_yyparse(PgBenchExpr **expr_parse_result_p, yyscan_t yyscanner);
 extern int	expr_yylex(union YYSTYPE *yylval_param, yyscan_t yyscanner);
-extern void expr_yyerror(PgBenchExpr **expr_parse_result_p, yyscan_t yyscanner, const char *message) pg_attribute_noreturn();
-extern void expr_yyerror_more(yyscan_t yyscanner, const char *message,
-							  const char *more) pg_attribute_noreturn();
+pg_noreturn extern void expr_yyerror(PgBenchExpr **expr_parse_result_p, yyscan_t yyscanner, const char *message);
+pg_noreturn extern void expr_yyerror_more(yyscan_t yyscanner, const char *message,
+										  const char *more);
 extern bool expr_lex_one_word(PsqlScanState state, PQExpBuffer word_buf,
 							  int *offset);
 extern yyscan_t expr_scanner_init(PsqlScanState state,
@@ -153,9 +153,9 @@ extern char *expr_scanner_get_substring(PsqlScanState state,
 										int start_offset,
 										bool chomp);
 
-extern void syntax_error(const char *source, int lineno, const char *line,
-						 const char *command, const char *msg,
-						 const char *more, int column) pg_attribute_noreturn();
+pg_noreturn extern void syntax_error(const char *source, int lineno, const char *line,
+									 const char *command, const char *msg,
+									 const char *more, int column);
 
 extern bool strtoint64(const char *str, bool errorOK, int64 *result);
 extern bool strtodouble(const char *str, bool errorOK, double *dv);

src/common/parse_manifest.c

@@ -114,8 +114,8 @@ static void json_manifest_finalize_wal_range(JsonManifestParseState *parse);
 static void verify_manifest_checksum(JsonManifestParseState *parse,
 									 const char *buffer, size_t size,
 									 pg_cryptohash_ctx *incr_ctx);
-static void json_manifest_parse_failure(JsonManifestParseContext *context,
-										char *msg);
+pg_noreturn static void json_manifest_parse_failure(JsonManifestParseContext *context,
+													char *msg);
 
 static int	hexdecode_char(char c);
 static bool hexdecode_string(uint8 *result, char *input, int nbytes);
@@ -889,6 +889,7 @@ static void
 json_manifest_parse_failure(JsonManifestParseContext *context, char *msg)
 {
 	context->error_cb(context, "could not parse backup manifest: %s", msg);
+	pg_unreachable();
 }
 
 /*

src/include/bootstrap/bootstrap.h

@@ -33,7 +33,7 @@ extern PGDLLIMPORT Form_pg_attribute attrtypes[MAXATTR];
 extern PGDLLIMPORT int numattr;
 
 
-extern void BootstrapModeMain(int argc, char *argv[], bool check_only) pg_attribute_noreturn();
+pg_noreturn extern void BootstrapModeMain(int argc, char *argv[], bool check_only);
 
 extern void closerel(char *relname);
 extern void boot_openrel(char *relname);
@@ -64,6 +64,6 @@ typedef void *yyscan_t;
 extern int	boot_yyparse(yyscan_t yyscanner);
 extern int	boot_yylex_init(yyscan_t *yyscannerp);
 extern int	boot_yylex(union YYSTYPE *yylval_param, yyscan_t yyscanner);
-extern void boot_yyerror(yyscan_t yyscanner, const char *message) pg_attribute_noreturn();
+pg_noreturn extern void boot_yyerror(yyscan_t yyscanner, const char *message);
 
 #endif							/* BOOTSTRAP_H */

src/include/c.h

@@ -145,6 +145,26 @@
 #define pg_nodiscard
 #endif
 
+/*
+ * 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>.
+ *
+ * In a declaration, function specifiers go before the function name.  The
+ * common style is to put them before the return type.  (The MSVC fallback has
+ * the same requirement.  The GCC fallback is more flexible.)
+ */
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
+#define pg_noreturn _Noreturn
+#elif defined(__GNUC__) || defined(__SUNPRO_C)
+#define pg_noreturn __attribute__((noreturn))
+#elif defined(_MSC_VER)
+#define pg_noreturn __declspec(noreturn)
+#else
+#define pg_noreturn
+#endif
+
 /*
  * This macro will disable address safety instrumentation for a function
  * when running with "-fsanitize=address". Think twice before using this!
@@ -213,30 +233,24 @@
 #define pg_attribute_printf(f,a)
 #endif
 
-/* GCC and Sunpro support aligned, packed and noreturn */
+/* GCC and Sunpro support aligned and packed */
 #if defined(__GNUC__) || defined(__SUNPRO_C)
 #define pg_attribute_aligned(a) __attribute__((aligned(a)))
-#define pg_attribute_noreturn() __attribute__((noreturn))
 #define pg_attribute_packed() __attribute__((packed))
-#define HAVE_PG_ATTRIBUTE_NORETURN 1
 #elif defined(_MSC_VER)
 /*
- * MSVC supports aligned.  noreturn is also possible but in MSVC it is
- * declared before the definition while pg_attribute_noreturn() macro
- * is currently used after the definition.
+ * MSVC supports aligned.
  *
  * Packing is also possible but only by wrapping the entire struct definition
  * which doesn't fit into our current macro declarations.
  */
 #define pg_attribute_aligned(a) __declspec(align(a))
-#define pg_attribute_noreturn()
 #else
 /*
  * NB: aligned and packed are not given default definitions because they
  * affect code functionality; they *must* be implemented by the compiler
  * if they are to be used.
  */
-#define pg_attribute_noreturn()
 #endif
 
 /*
@@ -858,8 +872,8 @@ typedef NameData *Name;
  * we should declare it as long as !FRONTEND.
  */
 #ifndef FRONTEND
-extern void ExceptionalCondition(const char *conditionName,
-								 const char *fileName, int lineNumber) pg_attribute_noreturn();
+pg_noreturn extern void ExceptionalCondition(const char *conditionName,
+											 const char *fileName, int lineNumber);
 #endif
 
 /*

src/include/commands/defrem.h

@@ -160,6 +160,6 @@ extern List *defGetQualifiedName(DefElem *def);
 extern TypeName *defGetTypeName(DefElem *def);
 extern int	defGetTypeLength(DefElem *def);
 extern List *defGetStringList(DefElem *def);
-extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate) pg_attribute_noreturn();
+pg_noreturn extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate);
 
 #endif							/* DEFREM_H */

src/include/common/parse_manifest.h

@@ -34,8 +34,7 @@ typedef void (*json_manifest_per_wal_range_callback) (JsonManifestParseContext *
 													  TimeLineID tli,
 													  XLogRecPtr start_lsn, XLogRecPtr end_lsn);
 typedef void (*json_manifest_error_callback) (JsonManifestParseContext *,
-											  const char *fmt,...) pg_attribute_printf(2, 3)
-			pg_attribute_noreturn();
+											  const char *fmt,...) pg_attribute_printf(2, 3);
 
 struct JsonManifestParseContext
 {

src/include/mb/pg_wchar.h

@@ -768,9 +768,9 @@ extern void check_encoding_conversion_args(int src_encoding,
 										   int expected_src_encoding,
 										   int expected_dest_encoding);
 
-extern void report_invalid_encoding(int encoding, const char *mbstr, int len) pg_attribute_noreturn();
-extern void report_untranslatable_char(int src_encoding, int dest_encoding,
-									   const char *mbstr, int len) pg_attribute_noreturn();
+pg_noreturn extern void report_invalid_encoding(int encoding, const char *mbstr, int len);
+pg_noreturn extern void report_untranslatable_char(int src_encoding, int dest_encoding,
+												   const char *mbstr, int len);
 
 extern int	local2local(const unsigned char *l, unsigned char *p, int len,
 						int src_encoding, int dest_encoding,

src/include/parser/parse_relation.h

@@ -110,9 +110,9 @@ extern bool isLockedRefname(ParseState *pstate, const char *refname);
 extern void addNSItemToQuery(ParseState *pstate, ParseNamespaceItem *nsitem,
 							 bool addToJoinList,
 							 bool addToRelNameSpace, bool addToVarNameSpace);
-extern void errorMissingRTE(ParseState *pstate, RangeVar *relation) pg_attribute_noreturn();
-extern void errorMissingColumn(ParseState *pstate,
-							   const char *relname, const char *colname, int location) pg_attribute_noreturn();
+pg_noreturn extern void errorMissingRTE(ParseState *pstate, RangeVar *relation);
+pg_noreturn extern void errorMissingColumn(ParseState *pstate,
+										   const char *relname, const char *colname, int location);
 extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
 					  VarReturningType returning_type,
 					  int location, bool include_dropped,

src/include/parser/scanner.h

@@ -145,6 +145,6 @@ extern void setup_scanner_errposition_callback(ScannerCallbackState *scbstate,
 											   core_yyscan_t yyscanner,
 											   int location);
 extern void cancel_scanner_errposition_callback(ScannerCallbackState *scbstate);
-extern void scanner_yyerror(const char *message, core_yyscan_t yyscanner) pg_attribute_noreturn();
+pg_noreturn extern void scanner_yyerror(const char *message, core_yyscan_t yyscanner);
 
 #endif							/* SCANNER_H */

src/include/postmaster/autovacuum.h

@@ -58,8 +58,8 @@ extern void autovac_init(void);
 /* called from postmaster when a worker could not be forked */
 extern void AutoVacWorkerFailed(void);
 
-extern void AutoVacLauncherMain(const void *startup_data, size_t startup_data_len) pg_attribute_noreturn();
-extern void AutoVacWorkerMain(const void *startup_data, size_t startup_data_len) pg_attribute_noreturn();
+pg_noreturn extern void AutoVacLauncherMain(const void *startup_data, size_t startup_data_len);
+pg_noreturn extern void AutoVacWorkerMain(const void *startup_data, size_t startup_data_len);
 
 extern bool AutoVacuumRequestWork(AutoVacuumWorkItemType type,
 								  Oid relationId, BlockNumber blkno);