On Tue, Feb 18, 2020 at 4:35 AM Alvaro Herrera <alvhe...@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.