Re: [lmi] precompiled headers in LMI

[Greg Chicares]

On 2008-03-23 17:56Z, Vadim Zeitlin wrote:
> 
>  While working on MSVC compilation I noticed that LMI sources mention the
> symbol LMI_COMPILER_USES_PCH and there already exists a pchfile.hpp file.

When last I worked on this, gcc didn't yet support pch. That was
a long time ago. It would be really good to step into the twenty-
first century and use pch now, because that should make us all
more efficient. Some groundwork like LMI_COMPILER_USES_PCH has
already been laid, though it was never used with any compiler
except borland.

> However this file is only included for Borland C++ compiler as it's guarded
> by "#ifdef __BORLANDC__" and not "#ifdef LMI_COMPILER_USES_PCH" (or nothing

That's a design defect.

> at all) as I'd expect and also not included from all files while both
> Borland and MSVC normally need all files to include the PCH header if it's

That's a defect in executing the design intention.

> used. To be precise, at least with MSVC you can use different PCH options
> for different files in the project but this would make the MSVC project
> files difficult to generate automatically.
> 
>  Anyhow, at this point I'd just like to know if you think this is something
> worth spending time on?

Yes, definitely.

> Of course, PCH support is not limited to MSVC and
> Borland compilers, recent (>= 3.4) g++ versions support them as well, and
> while cygwin has some bugs related to them I don't think they affect LMI
> and, anyhow, these bugs will be fixed sooner or later and PCH support would
> be already usable under Unix systems.

If we run into a problem with a particular msw port of gcc, then
we can just inhibit pch for that compiler, and it'll behave the
same way it does today.

>  And, while you certainly are already aware of it, I'd just like to remind
> that the benefit of PCH is that judicious inclusion of the mostly common
> used headers in pchfile.hpp (or, probably, lmi_wx_pch.hpp and other files
> for each target) can dramatically reduce the compilation time, e.g. factor
> of 10 is not uncommon.

Yes, I've seen such speedups myself. In my experience (with tools
that are by now long obsolete), pch files could grow quite large,
and could actually make builds slower if too many headers were
precompiled. But I imagine that wx has chosen a set of headers
judiciously and we can realize a benefit easily just by using its
default pch selections in <wx/wxprec.h> for example.

>  So what do you think of modifying LMI sources to use them?

I welcome patches and think I should be liberal in applying them.
I suppose the changes will mostly be conditionally compiled, so
that they'd have no effect on production builds unless (after
appropriate testing) we enable pch by default.

Please do make sure LMI_IGNORE_PCH is honored, so that pch can be
turned off even for a compiler that supports it, because target
'%.physical_closure' in 'workhorse.make' depends on that.

Would you mind showing me what sort of changes you'd want to make
in approximately one typical lmi file, first, just so I can see
how intrusive it might be? I figure you'd follow wx practice, so
I looked at 'src/common/wincmn.cpp'; let me ask some questions in
'//' comments below.

#include "wx/wxprec.h"

#ifdef __BORLANDC__
    #pragma hdrstop
#endif

// Nothing unexpected so far.

// Now, certainly we need to include some headers here in case
// WX_PRECOMP is not defined; but do we have to guard them with
// '#ifndef WX_PRECOMP'? Shouldn't the preprocessor figure out
// that they were already included?

#ifndef WX_PRECOMP
    #include "wx/string.h"
    #include "wx/log.h"
    #include "wx/intl.h"
    #pragma hdrstop
    #pragma hdrstop
// Does that pragma need to be written twice?
// Why is it written even once here, given that WX_PRECOMP is not
// defined? Should the condition have been '#ifdef WX_PRECOMP'?
#endif

#ifndef WX_PRECOMP
    #include "wx/string.h"
    #include "wx/log.h"
    #include "wx/intl.h"
    #include "wx/frame.h"
// Now I really wonder whether the preceding block's conditional
// is right: the first three headers have been named twice, and
// they are also present in "wx/wxprec.h".

Would the following alternative work as well?

#if defined WX_PRECOMP
#   include "wx/wxprec.h"
#   if !defined __GCC__
    // EDG, borland, and ms all seem to use this pragma:
#       pragma hdrstop
#   endif // !defined __GCC__
#endif // defined WX_PRECOMP

#include "wx/string.h"   // [included in pch]
...
#include "wx/platinfo.h" // [not included in pch]