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

[Vadim Zeitlin]

On Thu, 23 Aug 2018 14:36:02 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-08-23 12:39, Vadim Zeitlin wrote:
GC> > On Thu, 23 Aug 2018 00:48:49 +0000 Greg Chicares <address@hidden> wrote:
GC> [...]
GC> > GC>         :html_{html}
GC> > GC> is perfectly fine, and needn't be changed to something like this:
GC> > GC>         :html_(html)
GC> > GC> , right? Sure, I don't see any objection to that.
GC> > 
GC> >  I thought we already agreed that "{html}" was fine, so the question was
GC> > rather whether you agreed that its advantage was sufficient to justify
GC> > changing the existing code and replacing all "(html)"s with "{html}".
GC> > You seem to have already answered it affirmatively below, but let me just
GC> > show what kind of changes I'm speaking about:
GC> > 
GC> >   https://github.com/vadz/lmi/compare/uniform-init?expand=1
GC> 
GC> That looks great. The only thing I'd change is whitespace, e.g.:
GC> 
GC> -        :image_{image}
GC> -        ,src_{src}
GC> -        ,scale_factor_{scale_factor}
GC> +        :image_        {image}
GC> +        ,src_          {src}
GC> +        ,scale_factor_ {scale_factor}
GC> 
GC> to facilitate reading it as two columns.

 I'll do it, but in a separate commit as I prefer not mixing non-trivial
and white-space-only changes together. BTW, this is not at all the only
place where things are not aligned using this style, so I assume you'd like
me to fix all of those.

GC> - I'm working intensively on these files:
GC>     report_table*.?pp wx_table_generator.?pp
GC> and would prefer to modify those myself in order to avoid a merge.

 OK, I've left them out, but they both do contain member initializers using
parentheses.

GC> >  It only won't work if there is a narrowing conversion. In this case we 
can
GC> > still perform the conversion explicitly or try to get rid of it. And 
either
GC> > would be an improvement compared to an implicit conversion IMHO.
GC> 
GC> Any narrowing conversions might pose unexpected challenges, but
GC> AFAICS that's only if they reveal unrecognized implicit conversions,
GC> which may reasonably be regarded as defects anyway.

 I do get quite a few errors after switching to {}. Especially worrisome
are those that resulted from initially incorrectly changing std::deque and
std::vector ctors to use {} instead of (): this actually changed the code
meaning, as with () it used the "traditional" ctor, from C++98, while with
{} it started using ctor taking std::initializer_list.

 This raises 2 questions:

1. How should the code using these ctors, e.g. the initialization of
   seriatim_keywords_ and seriatim_numbers_ in input_sequence.cpp be
   actually written? For now I'm keeping () as it seems the most robust and
   also, arguably, the clearest things to do (the latter because it draws
   attention to the fact that this is not just a simple initialization but
   a call to a ctor which does something less trivial).

2. How to ensure that I didn't miss any vectors (or deques, as there is
   at least one of those, suffering from exactly the same problem, in
   mc_enum.hpp) that shouldn't use {} initialization but do now, because
   changing them didn't result in a compilation problem? There are plenty
   of std::vector<int>s in lmi code that could be affected by this. I'm
   going to try to review their use, but this risks taking much more time
   than I thought it would and still is error-prone.

 Would you have any thought about either of these points?


GC> > GC>     -    std::vector<int> const expected = {3, 4, 5};
GC> > GC>     -    BOOST_TEST(widths(v) == expected);
GC> > GC>     +    BOOST_TEST(widths(v) == std::vector<int>({3, 4, 5}));
GC> > GC>     but writing the parentheses that the preprocessor then requires is
GC> > GC>     too high a price to pay.
GC> > 
GC> >  It should be possible to avoid the extra parentheses with the variadic
GC> > macros, shouldn't it?
GC> 
GC> Maybe I'm missing something good, but I haven't tried to learn anything
GC> about preprocessor improvements since approximately C89.

 Would you want me to try to change lmi testing macros to allow not using
the parentheses? I don't know how difficult is it going to be, but
hopefully not too much.

 OTOH I must admit that I'm not very enthusiastic about writing our own
macros when other people have already done it, and done it well. I still
think that we should just add catch.hpp (yes, it's just a single header) to
lmi source tree and use it instead of lmi weirdly named custom macros, as
CATCH provides many more features (some of which are very useful) and a
much better test running experience. Just compare the result that you'd get
from a failure in BOOST_TEST() above with CATCH output for my example shown
in the previous email after modifying it to trigger a test failure:

-------------------------------------------------------------------------------
Comma without parentheses
-------------------------------------------------------------------------------
tst.cpp:6
...............................................................................

tst.cpp:11: FAILED:
  CHECK( v == vec{1, 2} )
with expansion:
  { 1, 2, 3 } == { 1, 2 }

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

 I wonder if there is any chance of convincing you to use it instead of the
current test_tools.hpp? I'd be very enthusiastic about this...

VZ