From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@gmail.com>
cc: Pg Hackers <pgsql-hackers@postgresql.org>,
        Michael Paquier <michael@paquier.xyz>,
        Piotr Stefaniak <postgres@piotr-stefaniak.me>
Subject: Re: Emacs vs pg_indent's weird indentation for function declarations
In-reply-to: <17265.1557955842@sss.pgh.pa.us>
References: 
 <CAEepm=0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug@mail.gmail.com>
 <7659.1548653310@sss.pgh.pa.us> <20190128070842.GF15401@paquier.xyz>
 <CAEepm=0UtAPN2RChs7=pOqHm2uvbU6Px7vfyvt8_-eiP7=ozUg@mail.gmail.com>
 <CA+hUKGKCvoUsU_HpW+hg8WNDUFjQEgQvVi9AwYdsqgJfV8TzxQ@mail.gmail.com>
 <14259.1557895390@sss.pgh.pa.us> <9170.1557955021@sss.pgh.pa.us>
 <CA+hUKGKb9n8xgWW3uEiA3Txvb9Er-FRX-wAAiiWdAjD7-nN44Q@mail.gmail.com>
 <17265.1557955842@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 15 May 2019 17:30:42 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <16885.1558104483.1@sss.pgh.pa.us>
Date: Fri, 17 May 2019 10:48:03 -0400
Message-ID: <16886.1558104483@sss.pgh.pa.us>
Precedence: bulk

I wrote:
>>> A small problem with the "rejiggering" is that it now makes the wrong
>>> choice for K&R-style function definitions, causing them to be weirdly
>>> indented.  For our purposes, that's a non-problem so I'm not excited
>>> about trying to make it smart enough to recognize those.  We do have
>>> a couple of amazingly old and crufty K&R-style functions in src/port/,
>>> though, so probably we'd wish to fix those.

To be concrete about that: the existing behavior when trying to decide
whether "foo(" starts a function declaration or a function definition
is to scan to the matching right paren[1] and then, if the immediately
next character is ';' or ',', it's a declaration; else it's a definition.
This fails on decls with attributes, such as

extern void pg_re_throw(void) __attribute__((noreturn));

My patch changes the rule to be "scan until we see a ';' ',' or '{'
that's not within parens; then it's a definition if that character
is '{', otherwise a declaration".  So it gets the right answer
for the above, but the wrong answer for

int
isinf(x)
double		x;
{

It doesn't really seem practical to me to make the lookahead function
smart enough to tell the difference between attributes and K&R-style
parameter declarations.  What I'm thinking of doing to have an
upstreamable patch is to invent a new switch, perhaps '-kr'/'-nkr',
to indicate whether the user is more worried about K&R function
declarations than she is about function attributes.  The default
for this switch could be debated perhaps, but I'd just stick an
explicit -nkr into pgindent's invocation, so we wouldn't care too
much.  (This would also ensure that pgindent would fail if you
try to use a too-old copy of pg_bsd_indent...)

			regards, tom lane

[1] Um, well, actually the existing behavior is to scan to the
*first* right paren.  But upgrading it to count parens correctly
seems like an unobjectionable improvement in any case.