Re: [lmi] Switch to using C++11 uniform initialization in the ctor initializer lists?

[Greg Chicares]

On 2018-08-24 13:20, Vadim Zeitlin wrote:
> On Fri, 24 Aug 2018 01:15:42 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2018-08-24 00:23, Vadim Zeitlin wrote:
> GC> [...]
> 
> GC> If you mean starting with a clean copy of HEAD and repeating all the
> GC> work, then it would probably make more sense for me to be the one to do
> GC> that.
> 
>  On one hand, I think this is going to be pretty wasteful -- i.e. what
> would have been the point of me doing it if you're going to redo it anyhow?

The point is to see where we did anything differently, indicating either a
misunderstanding on my part, a genuine difference of opinion, or--most
significantly--a case where one or the other of us may have made a mistake,
which might otherwise become a latent error that will surprise us at the
most inconvenient moment.

> OTOH it could well be simpler for you to do it like this rather than
> checking my changes.

Yes, that, too.

> And, especially if you can resist the temptation to
> also change the alignment, you should normally arrive at the same end
> result, so we could just compare your and my versions to find any possible
> troublesome points.

Oh. I read the fifth word as "can't". But those differences are trivial,
and I can easily ignore them now and commit them later.

>  But, thinking more about it, I really do think that it's enough to check
> that my changes left any classes with ctors taking std::initializer_list
> unaffected. And there are relatively few members of such classes (mostly
> vectors and strings and a couple of deques) in lmi code and I did check
> each of them individually already. So maybe you could just redo this: it
> would take much less time than redoing all the changes (trust me, I did
> both and so can compare).

But I'm good at tedious, repetitive tasks; and it's worth a couple hours
of effort if I find even one actual mistake.

> GC> BTW, I can change any ctor-initializer pretty easily with something
> GC> like
> GC>   s/(\(.*\))/{\1}
> 
>  This is not optimal as there are ctor initializers involving expressions
> with parentheses in them. I used two "s" commands, one for the leading "("
> and one for the last ")" on a line and this worked for almost all cases
> except the couple which had several initializers on one line. I also
> recommend doing both "nmap" and "vmap" mappings to this command to allow
> applying it quickly.

I used
  :s/(\(.*\))$/{\1}
which is even less optimal, but not too bad. So many ctor-initializers
match that pattern that it's not hard to do the others by hand.

> GC> but do you have a good technique for finding all ctr-initializers?
> 
>  I only looked for "^ *: *[A-Za-z0-9_]+_( *\(|$)", i.e. just the first
> one, and then examined all the subsequent initializers manually. I.e. I did
> a "git grep" for the regex above (which is why it uses extended, and not
> Vim, syntax)

Copying and pasting that command didn't work for me:

$git grep "^ *: *[A-Za-z0-9_]+_( *\(|$)"
fatal: command line, '^ *: *[A-Za-z0-9_]+_( *\(|$)': Unmatched ( or \(

but perhaps you did this with some vim add-in. I just used
  /^ *: *\<.*_\>
which I thought would be similar enough, but I missed a few files:

$grep --files-with-matches '^ *: *\<.*_\> *\(\|$\)' *.?pp |wc -l          
89

You changed 112 files, but my command found only 89.

> and then just kept hitting Ctrl-N (my mapping for ":cn<CR>"),
> "<n>V" where "n" is the number of lines with initializers (you don't need
> to count them manually if you have "set relativenumber" option on) and
> pressing the key bound to my mapping with "s" commands. If there was only a
> single initializer, I pressed the same key directly, which is why I bound
> it both normal and visual modes.

That's a good idea. I saw many ctor-initializer blocks and decided just
to 'shift-V' and then repeat a ":'<'>s" command with '@@' because the
total number of changes didn't seem to justify my figuring out anything
much fancier than that.

I have three types of questions:

(1) Vectors, excluding those constructed as
  v(number_of_elements, default_value)
The commit message suggests that you endeavored to avoid also
  v(std::vector<same_type> const&)
(because initializing them through an initializer-list would invoke
the copy ctor twice), but are you sure about the following changes
marked with '<---' in the right margin, which seem to copy vector
arguments? (That would be just a pessimization, not an error.)

'dbvalue.cpp':

database_entity::database_entity
    (int                        key
    ,std::vector<int> const&    dims
    ,std::vector<double> const& data
    ,std::string const&         gloss
    )
    :key_          {key}
    ,axis_lengths_ {dims} <---
    ,data_values_  {data} <---
    ,gloss_        {gloss}


'input_sequence.cpp':

InputSequence::InputSequence(std::vector<std::string> const& v)
    :years_to_maturity_ {lmi::ssize(v)}
    ,seriatim_keywords_ {v} <---

InputSequence::InputSequence(std::vector<double> const& v)
    :years_to_maturity_ {lmi::ssize(v)}
    ,seriatim_numbers_  {v} <---

'stratified_charges.cpp':

stratified_entity::stratified_entity
    (std::vector<double> const& limits
    ,std::vector<double> const& values
    ,std::string const&         gloss
    )
    :limits_{limits} <---
    ,values_{values} <---
    ,gloss_ {gloss}

(2) Ctor invocations. Even if these do work, should they use '{}',
or would '()' be better? E.g., the first example below doesn't
initialize some sort of struct with six values; instead, it invokes
a ctor with six parameters. I'm thinking of initializer-lists as
extending C array initialization, and in C these would be function
calls.

'input_sequence_entry.cpp':

    :wxButton{parent, id, "...", wxDefaultPosition, wxDefaultSize, 
wxBU_EXACTFIT}

'loads_test.cpp':

    LoadsTest(load_details const& details)
        :details_ {details}
        ,database_{details.length_} <---
        ,loads_   {}

'mortality_rates_test.cpp':

MortalityRates::MortalityRates()
    :Length_               {0}
    ,AllowAdb_             {false}
...
    ,SubstandardTable_     {mce_table_none}
    ,round_coi_rate_       {0, r_not_at_all} <---

'multidimgrid_any.cpp':

    :wxGrid{parent, id, pos, size, style, name}

(3) Conversion to bool:

'interest_rates.cpp':

    ,NeedSepAcctRates_   {v.Database_->Query(DB_AllowSepAcct) != 0.0}

I did the same thing, but without the '!= 0.0', and it compiled and
passed the full nychthemeral test suite, notably without any warning
about a narrowing conversion. I would understand if gcc warned on
such lines; what I don't understand is why it didn't warn.