Re: pgindent && weirdness

On Tue, Feb 18, 2020 at 4:35 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Hmm ... this suggests to me that if you remove these alleged special
> cases for offsetof and sizeof, the new code handles them correctly
> anyway.  Do you think it's worth giving that a try?  Not because
> removing the special cases would have any value, but rather to see if
> anything interesting pops up.

Good thought, since keywords also have last_token == ident so it's
redundant to check the keyword.  But while testing that I realised
that either way we get this wrong:

-                       return (int) *s1 - (int) *s2;
+                       return (int) * s1 - (int) *s2;

So I think the right formulation is one that allows offsetof and
sizeof to receive not-a-cast treatment, but not any other known
keyword:

diff --git a/indent.c b/indent.c
index 9faf57a..ed6dce2 100644
--- a/indent.c
+++ b/indent.c
@@ -570,8 +570,15 @@ check_type:
                ps.in_or_st = false;    /* turn off flag for structure decl or
                                         * initialization */
            }
-           /* parenthesized type following sizeof or offsetof is not a cast */
-           if (ps.keyword == 1 || ps.keyword == 2)
+           /*
+                * parenthesized type following sizeof or offsetof is not a
+                * cast; we also assume the same about similar macros,
+                * so if there is any other non-keyword identifier immediately
+                * preceding a type name in parens we won't consider it to be
+                * a cast
+                */
+           if (ps.last_token == ident &&
+                       (ps.keyword == 0 || ps.keyword == 1 || ps.keyword == 2))
                ps.not_cast_mask |= 1 << ps.p_l_follow;
            break;

Another problem is that there is one thing in our tree that looks like
a non-cast under the new rule, but it actually expands to a type name,
so now we get that wrong!  (I mean, unpatched indent doesn't really
understand it either, it thinks it's a cast, but at least it knows the
following * is not a binary operator):

-       STACK_OF(X509_NAME) *root_cert_list = NULL;
+       STACK_OF(X509_NAME) * root_cert_list = NULL;

That's a macro from an OpenSSL header.  Not sure what to do about that.