Re: [lmi] Stream cast problems with trailing spaces and more

[Vadim Zeitlin]

On Wed, 29 Jan 2020 14:29:38 +0000 Greg Chicares <address@hidden> wrote:

GC> I measure an Input::Input() speedup of a few microseconds

 That's indeed much less than I thought. Knowing how slow streams are, I
suspected avoiding using them could result in a bigger gain. Thanks for
checking this!

GC> >  BTW, now that we use C++17, I'd seriously consider defining a couple of
GC> > custom literal operators for calendar_date, that would allow us to write
GC> > 2440588_jdn or even 19700101_ymd instead of calendar_date(jdn_t(2440588)).
GC> 
GC> I think user-defined literals are like specialized kitchen gadgets--e.g.,
GC> cherry-pitters or strawberry-hullers. If you need to remove the pits from
GC> ten kilograms of cherries, a specialized tool might be a good investment.
GC> But for ten grams of cherries, the benefit isn't enough to justify the
GC> cost of learning to use it. I think our time is better spent elsewhere
GC> in this case.

 I just think using 19700101_ymd is much more clear than using "2440588",
so it's not so much about saving time as about making the code easier to
read. I wouldn't mind using

        ,LastBillDate(1970, 1, 1)

neither.

GC> > GC> Please take a look at how I changed that (once I push the commit).
GC> > 
GC> >  I'll have a look at the commit once it's done and will test it with clang
GC> > and MSVC.
GC> 
GC> Thanks--I'm not using those tools, so I might have again make a mistake
GC> that's not readily visible with gcc.

 I didn't have time to really look into this yet, but the new test fails
with clang:

???? test failed: Caught exception
    'converting 'INF' from type 'char const*' to type 'double'.'
  when
    '^Output failed'
  was expected.
[file stream_cast_test.cpp, line 85]

  Speed tests...
  stream_cast:

???? uncaught exception: std::runtime_error: converting '2.71828' from type 
'double' to type 'std::__1::basic_string<char, std::__1::char_traits<char>, 
std::__1::allocator<char> >'.


 The cause of the first problem seems relatively clear, but I'm not sure if
it's a libc++ defect or not.

 I don't understand at all why does the second problem happens in
assay_speed() however. And this one also happens with MSVC (with almost
exactly the same, except for the different full name of std::string, error
message). I'll look into it later, but I just already wanted to let you
know about these problems in case you immediately see something I don't.


GC> I've pushed my commits, so go ahead and show exactly what you have
GC> in mind, if you like. When I tried the simplest change I could think
GC> of along these lines (above), things broke; but maybe this really is
GC> a worthwhile change and I simply haven't understood the idea.

 I think you do understand it correctly. The proposed change is a
simplification, so it's to be expected that it breaks the unnecessarily
complicated existing use cases. But as I see no possible rationale for
allowing

        to = stream_cast(from, to);

to work, i.e. any reason to prefer it to

        to = stream_cast<To>(from);

IMO it's a worthwhile trade off.

 Regards,
VZ

pgpt1zuRuh9HB.pgp
Description: PGP signature