Re: [lmi] Include what you use

[Greg Chicares]

On 10/26/21 10:29 PM, Vadim Zeitlin wrote:
> 
>  While trying to set up ccache support for the CI builds in an attempt to
> speed them up, I've stumbled upon another item in my TODO list tagged with
> "optimize-build": the one about running https://include-what-you-use.org/
> tool (also known by its "iwyu" abbreviation) on lmi and removing the
> unnecessary headers.
> 
>  It turns out that this tool (and even its very latest version!) is now
> packaged by Debian, so trying it is pretty simple and so I've just done it
> and my first impression is that it somewhat works, even if it's not 100%
> correct. For example [...]

Fascinating.

> The full include-list for miscellany.hpp:
> #include "config.hpp"
> #include "so_attributes.hpp"  // for LMI_SO
> #include <cctype>             // for tolower, toupper
> #include <climits>            // for UCHAR_MAX
> #include <cstdio>             // for EOF

Those comments enable us to understand why it wants to include each
of those headers. That's valuable. In some cases it duplicates what
the source already says:

iwyu: #include <climits>            // for UCHAR_MAX
lmi:  #include <climits>                      // UCHAR_MAX
iwyu: #include <cstdio>             // for EOF
lmi:  #include <cstdio>                       // EOF

but it's very nice to have an automatically-generated touchstone
to verify that.

> miscellany.cpp should add these lines:
> struct tm;

I wonder why it would complain here:
    std::tm const*const t1 = std::gmtime(&t0);

> However removing the (big, and hence expensive to compile) listed headers
> from miscellany.hpp does work, and it shows that it can find real problems
> because <iterator> became unnecessary since the changes of d7153f9db [...] but
> it went unnoticed for 4.5 years and probably would remain unnoticed for
> much longer if I didn't run the tool on it.

This is definitely valuable.

>  Do you think it's worth continuing experimenting with this?

Yes.

> maybe propose a patch removing the unnecessary includes
Yes, I think we should consider doing this for all files at once.
One large omnibus patch would be easier to handle than separate
patches for hundreds of files.