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

[Vadim Zeitlin]

On Thu, 23 Aug 2018 19:52:57 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-08-23 18:20, Vadim Zeitlin wrote:
[...]
GC> >  I do get quite a few errors after switching to {}. Especially worrisome
GC> > are those that resulted from initially incorrectly changing std::deque and
GC> > std::vector ctors to use {} instead of (): this actually changed the code
GC> > meaning, as with () it used the "traditional" ctor, from C++98, while with
GC> > {} it started using ctor taking std::initializer_list.
GC> 
GC> Are there any other types of errors, aside from those especially
GC> worrisome ones?

 Yes, size_t to int conversions in the initializations of int members with
x.size(). I've fixed those for now using lmi::ssize(x) instead, but this is
not really a totally correct fix as under Win64 with MSVC, ssize_t is
strictly greater than int. However I think that if we want to support this
platform (note that the problem doesn't arise with gcc), we should rather
change ssize_t to be int there as the whole point of ssize_lmi.hpp is to
make size_t interoperable with int, not with some possibly-larger-than-int
signed integer type.

GC> >  This raises 2 questions:
GC> > 
GC> > 1. How should the code using these ctors, e.g. the initialization of
GC> >    seriatim_keywords_ and seriatim_numbers_ in input_sequence.cpp be
GC> >    actually written? For now I'm keeping () as it seems the most robust 
and
GC> >    also, arguably, the clearest things to do (the latter because it draws
GC> >    attention to the fact that this is not just a simple initialization but
GC> >    a call to a ctor which does something less trivial).
GC> 
GC> Yes, that sounds best. There isn't any reasonable alternative, is there?

 AFAIK you can't prevent ctor taking std::initializer_list from being used
if it can be used, so the only alternative I see is to have a function like
this:

        template <typename T>
        std::vector<T>
        make_vector_of_size(std::size_t size, T value = T{})
        {
            return std::vector(size, value);
        }

and using

        :vec_{make_vector_of_size(10, 11)}

to make a vector of size 10 filled with 11 instead of a vector of size 2
with 10 and 11 in it as we'd get from

        :vec_{10, 11}

 Of course, this is (a) cheating, as we just hide the parentheses inside a
helper function and (b) more verbose. But OTOH this is arguable more clear.
I must admit that I initially wrote this as a proof that there is indeed no
reasonable alternative, but the more I think it, more I actually like this
idea as it's just much more readable than a raw ctor call. And, due to
guaranteed RVO in C++11, it shouldn't be any less efficient than the
current code.


GC> > 2. How to ensure that I didn't miss any vectors (or deques, as there is
GC> >    at least one of those, suffering from exactly the same problem, in
GC> >    mc_enum.hpp) that shouldn't use {} initialization but do now, because
GC> >    changing them didn't result in a compilation problem?
GC> 
GC> The only option is thorough review. How would you like to handle any
GC> lines that I think are changed incorrectly? I don't want to commit a
GC> changeset that introduces defects, and then fix them in a separate
GC> commit: excluding them from the repository sounds better. I could
GC> propose a patch to your patch, and then cherry-pick a revised version.

 Sure. Of course, I do hope that there won't be any lines changed
incorrectly as I've just spent some time reviewing all the changes and I'll
re-review them again tomorrow as I don't fully trust myself to review
almost 1000 changes without missing anything from the first try.

GC> I suspect that I've often instantiated 'std::vector v(cardinality)'
GC> and sometimes even 'std::vector v(cardinality, default_element)'.

 Not often, but there are a few occurrences of both of those, yes.

GC> >  Would you have any thought about either of these points?
GC> 
GC> One idea is not to modernize container construction. I'm guessing
GC> that 'ContainerType z(anything)' will rarely if ever be compatible
GC> with '{}' in the lmi corpus.

 AFAICS, even never. I guess I can live with an inconsistency between
containers and everything else, but maybe it's worth introducing
make_container_of_size() to avoid it?

VZ