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

Tristan Partin <tristan@neon.tech>

From: "Tristan Partin" <tristan@neon.tech>
To: "Peter Eisentraut" <peter@eisentraut.org>
Cc: "pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2024-02-12T17:15:07Z
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)



Commits

  1. Remove unused #include's from backend .c files