Re: Harmonizing pg_bsd_indent parameter names

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Harmonizing pg_bsd_indent parameter names
Date: 2024-06-12 21:59:14
Message-ID: CAH2-WzmqVSQE-P_S39dzCAv-Gg3gYj4gLXKLP3K0cBGN3OSQUQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 12, 2024 at 5:32 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> I would be surprised if this 2-line patch created anything approaching a
> significant amount of work down the road. FWIW commit 10d34fe already
> changed one line in indent.c.

I missed that.

> > I'd like to push this patch now. It's generally easier to be strict
> > about these inconsistencies. My clang-tidy workflow doesn't
> > automatically filter out various special cases requiring subjective
> > judgement, so leaving stuff like this unfixed creates more work down
> > the road.
>
> Are these the only remaining inconsistencies?

No, but they're just about the only remaining inconsistencies that seem fixable.

The vast majority of the remaining inconsistencies (reported by
run-clang-tidy.py, using only its
readability-inconsistent-declaration-parameter-name and
readability-named-parameter checks) fit into one of two categories
(similar categories):

1. Functions such as GUC_yy_scan_string. These are functions where the
only way to fix the complained-about inconsistency is to change the
way that upstream Gnu flex generates its scanner C code.

There doesn't seem to be a built-in option that influences this behavior.

2. Postgres C code that uses the C preprocessor in the style of C++ templates.

In practice category 2 just means users of simplehash.h. There are a
couple of those, and I get at least one warning for each of them.

There is also one oddball case, not quite in either category. This
involves zic.c's declaration of
link(), when it should actually just be using the #include <unistd.h>
declaration. That's another weird upstream code thing -- this isn't
exactly fully under our control. I've avoided doing anything about
that, but perhaps I should have proposed a patch for that, too (it's
fairly similar to pg_bsd_indent). What do you think of that idea?

Personally I don't care all that much about the machine-generated code
(I'd fix it if there was a straightforward way to do so, but there
doesn't seem to be, so
meh). I use clangd's clang-tidy integration. It won't complain about
these cases anyway (it doesn't recognize that .l files and .y files
contain some C code).

--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2024-06-12 22:14:09 Re: [PATCH] pg_permissions
Previous Message Jelte Fennema-Nio 2024-06-12 21:50:15 Re: RFC: adding pytest as a supported test framework