[Dune] redundant includes in headers

[Oleh Krehel]

Dear Jö,

I agree that it's a headache to fix this problem, but it will only
grow larger and larger in time. Hopefully it will not become unmanageable.
As for <fmatrix.hh>, you've misunderstood me:
It includes:

> #include <cmath>
> #include <cstddef>
...
then
> #include <dune/common/fvector.hh>
which itself includes
> #include <cmath>
> #include <cstddef>.
This means that at least the two includes can be safely removed from fmatrix.hh.

regards,
Oleh



2011/8/19 Jö Fahlke <jorrit at jorrit.de>:
> Am Fri, 19. Aug 2011, 15:43:47 +0200 schrieb Oleh Krehel:
>> I've noticed this group of includes in fvector.hh:
>>
>> #include<cmath>
>> #include<cstddef>
>> #include<cstdlib>
>> #include<complex>
>> #include<cstring>
>>
>> Are they really necessary? Because when I've commented them away,
>> everything still works.
>
> What does "everything" mean in your case?  The right test would be "make
> headercheck".
>
> Even "make headercheck" will not catch all cases where includes are missing,
> depending on the compiler.  Some of the standard headers (say header A) may
> include (part of) other standard headers (say header B), so that you can use
> features from header B without including it, as long as you included header A.
> Unfortuneately, you cannot portably rely on that, since other compilers or
> standard libraries may handle header interdependencies differently.  The only
> exception I know is that you can omit "#include <iosfwd>" if you have done an
> "#include <ios>", because the standard actually mandates that the latter
> includes the former.  But I don't think we make very much use of those header
> in Dune.
>
> If you can show that fvector.hh does not actually directly use any of the
> features from a particular include (according to the C++-Standard), then we
> may consider removing that include.
>
>> But in fiedmatrix.hh it's definitely redundant:
>>
>> #include <cmath>
>> #include <cstddef>
>> #include <iostream>
>>
>> #include <dune/common/misc.hh>
>> #include <dune/common/exceptions.hh>
>> #include <dune/common/fvector.hh> // includes again <cmath> and <cstddef>
>>
>> I realize that it's not a priority and redundancies don't break anything,
>> but perhaps removing them could save a few milliseconds of build time:)
>
> Well, in this case it would be correct and portable to omit "#include
> <dune/common/fvector.hh>".  But it would be a maintanence headache: If
> somebody in the future changes fvector.hh such that it no longer needs
> <cstddef>, he can no longer simply remove that include.  First he must check
> whether any other source file includes fvector.hh (directly *or* indirectly),
> and then must adapt any that make the implicit assumption that fvector.h
> includes <cstddef>.
>
> While this is a headache in itself, it could be done for the core of Dune, by
> someone who doesn't have anything better to do with his time.  The even bigger
> issue arises when you consider external libraries or applications on top of
> Dune: whoever changes fvector.hh cannot possibly have access to all the code
> that uses it, so he cannot adapt that code.  And deprecation is impossible in
> this case, so there would be no smooth transition for the users.
>
>> I've read that the static analyzer features of eclipse or clang can
>> automatically
>> find the redundant/unused includes.
>
> Since the header a feature is defined/declared in is not nessessarily the
> header you should include to get that feature, these analyzers will need
> information they cannot possibly obtain from the source files alone.  At least
> if you want their result to be maintainable, not merely correct C++-code for
> the machine they were executed on.  It possible that they have this
> information built in for the standard library, but we would still need to feed
> them that information for Dune in a machine-readable format.
>
> Bye,
> Jö.
>
> --
> There's no evolution in the US.  It's not allowed.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQIVAwUBTk53B+mx9CKgc0OWAQictA//dKPN+t47Mll0ASmwfQsFFike0LFyWqdN
> DSj0eJEEDISOOgJj1OB266yNNZ5M6g/vonHMqKY8dfTlu6lXdeaEaoXYEI4ZcUZL
> zkgUjc//6AjiHhKK9t5lM6itdCqledLgNtMuHL4aCA9QeKABilTwWib6RYOMeAAT
> Un9exWOCG6353ub+XRfvOAWngmX5J1NzcIuxD6+5DFuXxRv0y+G3psexDiE5iWFY
> rPh6LppYGTHi35DAoAhuP3mpEnxIwRLjLJ2kB6+ul3Fe4VJ5xjQmhYLG1jLH5yoU
> 98s3IaW1gQ+vWiinyr+4JOa6uF7q5iLU0m4DdYk6m9jkRg5MtYDCVgcCBy+nDcFt
> ld70+KDkZ6FKMjROS2t3hnLwybCItKGtTBulrqY5c0ehFNK4ZBSO/4E+Qj8jEc+x
> aAT6u2GDCJaIvja6nH10DfxR8FKZByYvqOzk35Zc1J7wlFhji7uAuAqnKuoiKcQl
> naCme4rv6x4xNuClVGhcjmg24kaojP0Tk4hFy2xcSTw4sZrxlIUavev8SYxRuqYZ
> XJcXMzatejzmEj3caVQtHNZccWntlkyFPxdY1EmTjkLwqhBDhXruz3PhCgsdwWEQ
> 0RAC6LlEzZ9O5cHMaEBzvmS0mieH8XuPyHepsJj8XB3FgNPUz/oLe9+WQErnBvK0
> Nf2NH++xw+k=
> =BT+Q
> -----END PGP SIGNATURE-----
>
>