Re: [lmi] PCH

[Greg Chicares]

On 2016-06-11 23:56, Vadim Zeitlin wrote:
> On Mon, 6 Jun 2016 20:37:59 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-06-05 23:33, Greg Chicares wrote:
> GC> > On 2016-06-05 22:23, Vadim Zeitlin wrote:
> GC> >> On Sun, 5 Jun 2016 22:04:32 +0000 Greg Chicares <address@hidden> wrote:
> GC> >> 
> GC> >> GC> (1) #include LMI_PCH_HEADER
> GC> >> GC> 
> GC> >> GC> What macro definition makes that work with gcc?
> GC> >> 
> GC> >>  Nothing really special, i.e. here is what I have in Makefile.am:
> GC> >> 
> GC> >>        AM_CXXFLAGS = -DLMI_PCH_HEADER="<pchfile.hpp>"
> GC> >> 
> GC> >> and, for the GUI code:
> GC> >> 
> GC> >>        libskeleton_la_CXXFLAGS = ... 
> -DLMI_PCH_HEADER="<pchfile_wx.hpp>"
> GC> > 
> GC> > I'll change the '.cpp' files,
> GC> 
> GC> Pushed.
> 
>  Thanks again for doing this, this is enough to resolve almost all
> differences between my branch sources and master all while still using the
> precompiled headers in the MSVC build. I just had to adjust a couple of
> files to include pchfile_wx.hpp instead of pchfile.hpp, please see
> https://github.com/vadz/lmi/pull/42

I personally don't care which PCH file each TU includes, because I'm not
using PCH. But this part of the patch doesn't just change PCH:

https://github.com/vadz/lmi/commit/a66c3dd7caffa1bfb3c363fbe18d8e860a9c168e.patch
...
diff --git a/tier_view.cpp b/tier_view.cpp
index f3fccd8..103681d 100644
--- a/tier_view.cpp
+++ b/tier_view.cpp
@@ -25,6 +25,7 @@

 #include "multidimgrid_any.hpp"
 #include "multidimgrid_tools.hpp"
+#include "multidimgrid_safe.tpp"
 #include "safely_dereference_as.hpp"
 #include "stratified_charges.hpp"
 #include "stratified_charges.xpp"

Why is that needed? Has it always been wrong?

BTW, in commit ee6c0230a33cbf23da6c56faac72254281804a09 I changed the
way 'workhorse.make' determined which objects depend on wx (for the
purpose of suppressing '-Wcast-qual'). It seemed that inclusion of
the wx-specific PCH header was a better way of determining that. Now,
however, we're including that PCH header in some files that actually
don't depend on wx, and those files shouldn't be exempted from that
extra warning--so I'll revert that modification.

>  I had a small problem with just one file, main_common.cpp, which is also
> part of libskeleton, and so would normally need to include pchfile_wx.hpp,
> but this didn't seem to be appropriate to do for this file as it's also
> used as part of other targets (e.g. product_files) which don't have
> anything to do with wxWidgets. After hesitating for some time, I just moved
> this file from libskeleton project to liblmi one and removed it from the
> other targets (such as product_files) and everything builds perfectly for
> me. I wonder if this shouldn't be also done in objects.make, this file just
> doesn't seem to belong to libskeleton, unless I'm missing something.

I disagree with this change. There are several 'main*.cpp' files, and
code that is common to all has been factored into 'main_common.cpp'.
This change would remove that common code from non-wx binaries: for
example, they would no longer call set_terminate().

What problem are you trying to solve here--something to do with PCH?

> GC> > but leave 'Makefile.am' to you.
> GC> 
> GC> I didn't touch that.
> 
>  Can I just commit my changes to this one now?

See above--I think that would be an error.