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

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)