Re: backend *.c #include cleanup (IWYU)

From: "Tristan Partin" <tristan(at)neon(dot)tech>
To: "Peter Eisentraut" <peter(at)eisentraut(dot)org>
Cc: "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: backend *.c #include cleanup (IWYU)
Date: 2024-02-12 17:15:07
Message-ID: CZ39Q43QATH3.3OC51OS52QWLW@neon.tech
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat Feb 10, 2024 at 1:40 AM CST, Peter Eisentraut wrote:
> I played with include-what-you-use (IWYU), "a tool for use with clang to
> analyze #includes in C and C++ source files".[0] I came across this via
> clangd (the language server), because clangd (via the editor) kept
> suggesting a bunch of #includes to remove. And I suppose it was right.
>
> So as a test, I ran IWYU over the backend *.c files and removed all the
> #includes it suggested. (Note that IWYU also suggests to *add* a bunch
> of #includes, in fact that is its main purpose; I didn't do this here.)
> In some cases, a more specific #include replaces another less specific
> one. (To keep the patch from exploding in size, I ignored for now all
> the suggestions to replace catalog/pg_somecatalog.h with
> catalog/pg_somecatalog_d.h.) This ended up with the attached patch,
> which has
>
> 432 files changed, 233 insertions(+), 1023 deletions(-)
>
> I tested this with various compilation options (assert, WAL_DEBUG,
> LOCK_DEBUG, different geqo variants, etc.) to make sure a header wasn't
> just used for some conditional code section. Also, again, this patch
> touches just *.c files, so nothing declared from header files changes in
> hidden ways.
>
> Also, as a small example, in src/backend/access/transam/rmgr.c you'll
> see some IWYU pragma annotations to handle a special case there.
>
> The purpose of this patch is twofold: One, it's of course a nice
> cleanup. Two, this is a test how well IWYU might work for us. If we
> find either by human interpretation that a change doesn't make sense, or
> something breaks on some platform, then that would be useful feedback
> (perhaps to addressed by more pragma annotations or more test coverage).
>
> (Interestingly, IWYU has been mentioned in src/tools/pginclude/README
> since 2012. Has anyone else played with it? Was it not mature enough
> back then?)
>
> [0]: https://include-what-you-use.org/

I like this idea. This was something I tried to do but never finished in
my last project. I have also been noticing the same issues from clangd.
Having more automation around include patterns would be great! I think
it would be good if there a Meson run_target()/Make .PHONY target that
people could use to test too. Then, eventually the cfbot could pick this
up.

--
Tristan Partin
Neon (https://neon.tech)

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Corey Huinker 2024-02-12 17:24:46 Re: Document efficient self-joins / UPDATE LIMIT techniques.
Previous Message Mats Kindahl 2024-02-12 17:09:06 Re: glibc qsort() vulnerability