Refactoring patches

[Pavel Sanda]

On Tue, Oct 13, 2020 at 11:40:51AM -0400, Richard Kimberly Heck wrote:
> On 10/13/20 3:09 AM, Yuriy Skalko wrote:
> >> Yes. We'll deal with the rest later. 
> >> Pavel
> > Done at 2a594d3.
> >
> > The .cpp changing part (for future applying) is in attached patch.
> 
> Suggestion: Let's see if we can get people on different platforms and
> with different compilers to try the patch. As has already been said, we
> do occasionally run into cases where includes are needed sometimes but
> not others.

If we are going after redundant includes we should first decide what's
the goal and why we are doing it.

Are we trying to make it

- "semantically correct", by which I mean removing includes unrelated to
   the content of the processed file (i.e. forgotten header which is no
   more used becase the code using it is gone). 
   Having "unnecessary" include whose content is used (but already included
   recursively via another include) is actually proper.

- just trying to speedup things, so any possible include gone is good.

If it's not clear what I mean think about

a) .h example:
    BiblioInfo.h::#include "Citation.h"
    We really use stuff from Citation.h, but the include is technically not necessary
    becase we get it from different include.

b) .cpp example:
Undo.cpp includes Undo.h. Again this include would be brought by other includes as well.

Do we want to keep those or kick them out?


I didn't make any measurements by my expectation is that deleting those multiply-imported
includes does not make big difference for compilation time becase our ifdefs will be caught
by preprocessor which is damn fast (compared to real c++ compilation).

The real "forgotten" includes which would trigger compiler analysis could make difference
but I am afraid you would need to manually check each include of this type.


If we prefer kicking out any unnecessary header there is already
development/tools/header_check.sh at our disposal; reading bug 6305 and
thread https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg155492.html
might be of help as well ;)

I don't have strong opinions about the direction, but I think we should first agree
what we are trying to achieve here.


And then there is somewhat separate issue about generic system headers which
sometimes land in the code because someone brave enough tried to run it
on solaris or haiku or god knows what. Here I would be more conservative,
and would not just blindly delete those because I think 
i) there will be few of them so one could possibly check git history how it appeared
ii) the speedup gain is negligible compared to problems we could cause elsewhere

Pavel