Re: [lmi] PCH

[Greg Chicares]

On 2016-06-14 00:19, Greg Chicares wrote:
> On 2016-06-13 16:21, Vadim Zeitlin wrote:
>> On Mon, 13 Jun 2016 15:38:02 +0000 Greg Chicares <address@hidden> wrote:
[...]
>> GC> >  I had a small problem with just one file, main_common.cpp, which is 
>> also
>> GC> > part of libskeleton, and so would normally need to include 
>> pchfile_wx.hpp,
>> GC> > but this didn't seem to be appropriate to do for this file as it's also
>> GC> > used as part of other targets (e.g. product_files) which don't have
>> GC> > anything to do with wxWidgets. After hesitating for some time, I just 
>> moved
>> GC> > this file from libskeleton project to liblmi one and removed it from 
>> the
>> GC> > other targets (such as product_files) and everything builds perfectly 
>> for
>> GC> > me. I wonder if this shouldn't be also done in objects.make, this file 
>> just
>> GC> > doesn't seem to belong to libskeleton, unless I'm missing something.
>> GC> 
>> GC> I disagree with this change. There are several 'main*.cpp' files, and
>> GC> code that is common to all has been factored into 'main_common.cpp'.
>> GC> This change would remove that common code from non-wx binaries: for
>> GC> example, they would no longer call set_terminate().
>> 
>>  Sorry, I don't understand this at all, I must have again badly explained
>> what I did. All the targets that link with main_common.o currently still
>> continue to link with it, by definition non-wx targets don't link with
>> (wx-dependent) libskeleton anyhow, so they're not affected at all.
> 
> Okay, I read "removed it from the other targets (such as product_files)"
> as suggesting that product_files$(EXEEXT) would no longer link that object.
> 
>>  Again, all I suggest doing is this:
>> ---------------------------------- >8 --------------------------------------
>> diff --git a/objects.make b/objects.make
>> index c39baff..7972ae7 100644
>> --- a/objects.make
>> +++ b/objects.make
>> @@ -296,6 +296,7 @@ lmi_common_objects := \
>>    ihs_mortal.o \
>>    ihs_server7702.o \
>>    lmi.o \
>> +  main_common.o \
>>    md5.o \
>>    mec_input.o \
>>    mec_server.o \
>> @@ -327,7 +328,6 @@ skeleton_objects := \
>>    illustration_document.o \
>>    illustration_view.o \
>>    input_sequence_entry.o \
>> -  main_common.o \
>>    mec_document.o \
>>    mec_view.o \
>>    msw_workarounds.o \
>> ---------------------------------- >8 --------------------------------------
>> 
>>  What exactly would it break?
> 
> Actually, I had thought of proposing that as an alternative less violent
> than what I thought you were suggesting. Let me give that some thought.

Here's a problem:

lmi_cli_monolithic$(EXEEXT): $(cli_objects) $(lmi_common_objects)
lmi_cgi$(EXEEXT): $(cgi_objects) $(lmi_common_objects)

Both $(cgi_objects) and $(cli_objects) include 'main_common.o', so adding
that object file to $(lmi_common_objects) would cause it to be included
twice. Perhaps the linker would remove duplicates in this case, but maybe
there's another case involving libraries where it wouldn't; we just don't
know. And moving an object file from one list to another to suit the needs
of one compiler results in a haphazard design that, years later, can be
understood only in light of compromises made for pragmatic purposes that
may be forgotten. Worst of all, we recently spent hours debugging a real
problem with boost::filesystem that was caused by including the same
object files in two different places, and then of course there's that
similar $(duplicated_objects) issue that we haven't resolved.

The right way to approach this is to ask why the lists of objects are
written this way, what the real problem is, and how to solve it.

Fundamentally, we have two different blobs of insurance calculations,
"lmi" and "antediluvian", each of which can be combined with any of
three interface blobs: "cgi", "cli", and "wx" (except that we have
never combined "antediluvian" with "wx"). Each interface blob has its
own main TU: main_cgi, main_cli, and main_wx. Code that is common
among all three is factored into main_common, which is included in the
object-lists cgi_objects, cli_objects, and, originally, lmi_wx_objects
(there being no antediluvian_wx binary), which was renamed here:
  http://lists.nongnu.org/archive/html/lmi/2014-02/msg00007.html
so that main_common wound up in skeleton_objects. Now we have
  cgi_objects
    main_cgi.o
    main_common.o
  cli_objects
    main_cli.o
    main_common.o
but for wx, main_wx and main_common are in different lists:
  skeleton_objects
    main_common.o
  lmi_wx_objects
    main_wx.o
and now we're talking about moving main_common to lmi_common_objects,
which is harder to understand and causes duplication.

The real problem is that for msvc...

>> VZ> All files compiled together as part of the same target must use the
>> VZ> same precompiled header and as these files are part of libskeleton,
>> VZ> together with other files that include pchfile_wx.hpp because they
>> VZ> use wxWidgets, they need to also include this header and not just
>> VZ> pchfile.hpp as well, even if they don't use any GUI features.

Factoring out common main code into 'main_common.cpp' isn't the issue;
compiling it as one separate '.o' file is, because msvc needs to compile
it with different PCH headers in different contexts. Moving it from one
object list to another isn't the right solution. I see two possibilities:

(1) Redo the PCH changes so that the PCH header to be included is given
as a macro, as I believe you were doing formerly, and set that macro
appropriately for each makefile target. That might work if you maintain
separate directories for each target's object files. Then you'd have
  wx/main_common.o, compiled with a wx-specific PCH header; and
  cli/main_common.o, compiled with a generic PCH header;
and, I suppose, a cgi directory with a duplicate compiled the same way
as for cli. This doesn't affect gcc: no makefile change is needed
except to add a PCH macro for msvc's use only.

(2) Instead of compiling 'main_common.cpp' separately, #include it
in each of the main source files. This should have the same effect as
separate compilation, and doesn't materially affect build speed or
total binary size. Isn't this the most natural way to address the
msvc issue?