lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] Special build types, e.g. gcov


From: Vadim Zeitlin
Subject: Re: [lmi] Special build types, e.g. gcov
Date: Thu, 30 Oct 2014 14:28:44 +0100

On Thu, 30 Oct 2014 00:43:28 +0000 Greg Chicares <address@hidden> wrote:

GC> You can always get a std::string from an any_member<>,

 Oops, it looks like the existing is way too complicated for its own good.
To be honest, I have no idea why was it written like this and just assumed
this was the canonical way of getting values from Input. Using str() is
clearly much simpler and better.

GC> So do these lines, which may possibly DTRT for the effective-date test:
GC> 
GC>     #include "value_cast.hpp"
GC> 
GC>     std::string const effective_date_jdn_str = cell["EffectiveDate"].str();
GC>     std::string const first_of_month_jdn_str = 
value_cast<std::string>(first_of_month.julian_day_number());
GC>     LMI_ASSERT_EQUAL(effective_date_jdn_str, first_of_month_jdn_str);
GC> 
GC> but you can certainly write it in a less ugly manner.

 I'm not sure if it's really that much better, but here is my version:

        calendar_date effective_date;
        std::istringstream is(cell["EffectiveDate"].str());
        LMI_ASSERT(is >> effective_date);
        LMI_ASSERT_EQUAL(effective_date, first_of_month);

(full patch attached).


GC> LMI_SO is dll[ex|im]port for msw compilers that need it, but perhaps
GC> more importantly it's the ELF visibility attribute. I don't know
GC> whether the latter ever really caught on,

 This is an easy one: yes, it did. It's universally supported and used by
all non-trivial C++ shared libraries because it allows to export many fewer
functions which helps the run-time linker enormously resulting in
dramatically better load times.

GC> But here we expose first one class, then another...
GC> 
GC> The farmer takes a wife
GC> The wife takes the child
GC> ...
GC> The rat takes the cheese
GC> ...and pretty soon everything's "encapsulated" in a single ocean.

 I don't think I can agree with this. The encapsulated parts are those
which are not exported, i.e. those without LMI_SO, and there are still
quite a few of those. IMHO it's not a problem to export those parts which
are already used outside of liblmi.dll, it's just a recognition of the fact
that they are, indeed, used. Ideally all the classes and functions which
could be used outside of this DLL would be marked with LMI_SO, while none
of the rest would be.


GC> Or you could read the rest of the message, but the last person who
GC> did that went quite mad.

 I read it and I don't feel madder than before, is this a good or bad sign?

GC> >    I'm completely lost as to how is this possible,
GC> >    so I'm going to try rebuilding without optimizations to be able to 
debug
GC> >    the code as something seems to be going seriously wrong inside
GC> >    yare_input ctor.
GC> 
GC> Well, it looks like this LMI_MSC code should do its intended job:
GC> 
GC> yare_input::yare_input(Input const& z)
GC> {
GC> // TODO ?? This temporarily works around a deeper defect. See:
GC> //   http://lists.nongnu.org/archive/html/lmi/2009-02/msg00074.html
GC> #if defined LMI_MSC
GC>     const_cast<Input&>(z).RealizeAllSequenceInput();
GC> #endif // defined LMI_MSC

 I can confirm that this works, i.e. the code like this

        Input cell = default_cell();
        cell.RealizeAllSequenceInput();
        yare_input const cell_values(cell);

        std::vector<double> const&
            general_account_rate = cell_values.GeneralAccountRate;
        LMI_ASSERT(!general_account_rate.empty());

passes the assert.

<off-topic linguistic remark>
GC> ...but when you need the heavy artillery, do this:
GC> 
GC>   wxBusyInfo info
GC>   (
GC>     wxBusyInfoFlags()
GC>       .Parent(parent)
GC>       .Icon(wxArtProvider::GetIcon(wxART_CTHULHU))
GC>       .Title("<b>Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn</b>")

 At last some clear language. BTW, I am glad to say that I think I've
finally understood what does yare_input mean, after looking up the word
"yare" in a dictionary. Until now I kept thinking of it as "yet another
regularly expressible input" which seemed almost but not quite right.

</off-topic linguistic remark>

GC> to summon the utmost abomination:
GC> 
GC>   Input::magically_rectify()

 This works as well, applied in this form:

        Input cell = Input::magically_rectify(default_cell());

(magical or not, it still doesn't modify its const input parameter as I
originally thought it would do).


 To summarize, with the attached patches the tests build in both normal
("ship") and "so_test" builds and still pass in the former. They do _not_
pass in the latter, however, with the following mysterious message box
given on startup:

        ---------------------------
        Error
        ---------------------------
        boost::filesystem::path: invalid name "C:" in path: 
"C:/opt/lmi/data/new-16.png"
        ---------------------------
        OK   
        ---------------------------

(the file in question does exist BTW). This is not specific to the tests,
the main LMI executable itself, built with USE_SO_ATTRIBUTES=1, gives
exactly the same error and refuses to start, so I guess it's worth looking
into this and I'll do it later. But the patches here should still be
applied IMHO, as they at least make the test build -- and also because they
make the test code simpler and more clear.

 Thanks,
VZ

Attachment: 0001-Fix-typo-in-general_account_date-variable-name.patch
Description: Text document

Attachment: 0002-Operate-directly-on-strings-in-default-input-test-ca.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]