[lmi] Include what you use

[Vadim Zeitlin]

 Hello,

 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, see this output for miscellany.cpp file:

---------------------------------- >8 --------------------------------------
miscellany.hpp should add these lines:

miscellany.hpp should remove these lines:
- #include <algorithm>  // lines 29-29
- #include <iterator>  // lines 35-35
- #include <utility>  // lines 39-39

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
#include <iomanip>            // for operator<<, setfill, setw
#include <ios>                // for ios, operator|, ios_base, ostringstream
#include <limits>             // for numeric_limits
#include <sstream>            // for basic_ostream, basic_ostream::operator<<
#include <string>             // for string
#include <vector>             // for vector
---

miscellany.cpp should add these lines:
struct tm;

miscellany.cpp should remove these lines:
- #include <cstddef>  // lines 33-33
- #include <fstream>  // lines 36-36
- #include <istream>  // lines 37-37

The full include-list for miscellany.cpp:
#include "pchfile.hpp"
#include "miscellany.hpp"
#include "alert.hpp"       // for alarum, LMI_FLUSH
#include "assert_lmi.hpp"  // for LMI_ASSERT
#include "bourn_cast.hpp"  // for bourn_cast
#include "ssize_lmi.hpp"   // for ssize
#include <algorithm>       // for equal, count, max
#include <cmath>           // for ceil, floor
#include <cstdio>          // for snprintf, size_t
#include <ctime>           // for gmtime, strftime, time, time_t
#include <iterator>        // for istreambuf_iterator, operator!=, operator==
struct tm;
---
---------------------------------- >8 --------------------------------------

 The tool recommends removing 6 headers, but <fstream> is definitely needed
by miscellany.cpp, which uses std::ifstream, so it has false positives.
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
(Improve each_equal() efficiency; augment its unit test, 2017-01-22), 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.

 I didn't try running iwyu on all the files yet, nor reviewing all of its
suggestions, but it seems clear that:

1. They will need to be manually reviewed and sometimes ignored.
2. Some of them will be useful and should result in better build times.

 Do you think it's worth continuing experimenting with this? It would take
some time, but, hopefully, not very long if we want to just run it manually
once and apply as many of its suggestions as possible and reasonable. My
initial hope was to make it part of the CI builds, so that we could be
notified whenever a header becomes unnecessary in the future too, which
would IMO b even more useful, but could take much longer.

 This is definitely not a high priority task in any case, so the question
is hardly urgent, but I'd just like to know if I should keep this in my
TODO and maybe propose a patch removing the unnecessary includes, or if you
think it's not worth applying such patches at all and I should just remove
this from my TODO.

 Please let me know if you have any opinion about it and thanks in advance,
VZ

pgpX1C2Y5bZBN.pgp
Description: PGP signature