Re: make pg_attribute_noreturn() work for msvc? - Mailing list pgsql-hackers

From Tom Lane
Subject Re: make pg_attribute_noreturn() work for msvc?
Date
Msg-id 27977.1573597325@sss.pgh.pa.us
Whole thread Raw
In response to Re: make pg_attribute_noreturn() work for msvc?  (Andres Freund <andres@anarazel.de>)
Responses Re: make pg_attribute_noreturn() work for msvc?  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2019-11-12 15:58:07 -0500, Tom Lane wrote:
>> I guess my big question about that is whether pgindent will make a
>> hash of it.

> If one writes 'pg_noreturn void', rather than 'void pg_noreturn', then
> there's only one place where pgindent changes something in a somewhat
> weird way. For tablesync.c, it indents the pg_noreturn for
> finish_sync_worker(). But only due to being on a separate newline, which
> seems unnecessary…

I think that it might be like that because some previous version of
pgindent changed it to that.  That's probably why we never adopted
this style generally in the first place.

> With 'void pg_noreturn', a few prototypes in headers get indented more
> than pretty, e.g. in pg_upgrade.h it turns

> void pg_noreturn pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2);
> into
> void        pg_noreturn pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2);

> I'm a bit confused as to why pg_upgrade.h doesn't use 'extern' for
> function declarations? Not that it's really related, except for the
> 'extern' otherwise hiding the effect of pgindent not liking 'void
> pg_noreturn'…

There are various headers where people have tended to not use "extern".
I always disliked that, thinking it was not per project style, but
never bothered to force the issue.  If we went around and inserted
extern in these places, it wouldn't bother me any.

> I don't see a reason not to go for 'pg_noreturn void'?

That seems kind of ugly from here.  Not sure why, but at least to
my mind that's a surprising ordering.

>> One idea is to merge it with the "void" result type that such
>> a function would presumably have, along the lines of
>> #define pg_noreturn    void __declspec(noreturn)

> Yea, that'd be an alternative. But since not necessary, I'd not go
> there?

I kind of liked that idea, too bad you don't.  One argument for it
is that then there'd be exactly one right way to do it, not two.
Also, if we find out that there's some compiler that's pickier
about where to place the annotation, we'd have a central place
to handle it.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: JIT performance bug/regression & JIT EXPLAIN
Next
From: Tom Lane
Date:
Subject: Re: checking my understanding of TupleDesc