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.