Re: make pg_attribute_noreturn() work for msvc?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: make pg_attribute_noreturn() work for msvc?
Date: 2019-11-12 22:22:05
Message-ID: 27977.1573597325@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-11-12 22:39:20 Re: checking my understanding of TupleDesc
Previous Message Andres Freund 2019-11-12 22:21:31 Re: JIT performance bug/regression & JIT EXPLAIN