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

[Vadim Zeitlin]

On Wed, 29 Jan 2020 02:15:53 +0000 Greg Chicares <address@hidden> wrote:

GC> [Although I replied just hours ago to a more recent message on the same
GC> topic, this older message contains some intriguing detail that deserves
GC> to be addressed however belatedly.]

[I decided to answer this message in parallel too, but please let me know
 if you'd prefer to merge these 2 threads in a single one]

GC> On 2018-11-16 01:00, Vadim Zeitlin wrote:
GC> > 
GC> >  I've been looking at stream_cast() code because it started throwing
GC> > "Trailing whitespace remains" exception with the latest MSVS version
GC> > (15.9.0) when initializing Input::ListBillDate field from its JDN given in
GC> > the string form in this class ctor, which means that no illustrations can
GC> > be created when using lmi built this compiler, which is, as you can
GC> > imagine, somewhat problematic. Leaving aside the question of why does this
GC> > member need to be initialized from a string in the first place,
GC> 
GC> Initialization from string for uniformity:
GC> 
GC> Input::Input()
GC>     :IssueAge                         {"45"}
GC>     ,RetirementAge                    {"65"}
GC>     ,Gender                           {"Male"}
GC> ...
GC>     ,ListBillDate                     {"2440588"} // Assume no inforce so 
old

 I guess the question could be formulated as "why is uniformity important
here?". I don't really like specifying numbers (or dates) as strings and I
don't see why do we need to convert strings to their actual type during
run-time instead of doing it during compile-time. I admit I haven't done
any profiling here, but I think Input objects are created relatively often,
so using numbers could be not only simpler (fewer quotes!), but also
faster.

 BTW, now that we use C++17, I'd seriously consider defining a couple of
custom literal operators for calendar_date, that would allow us to write
2440588_jdn or even 19700101_ymd instead of calendar_date(jdn_t(2440588)).


GC> Probably we should treat clang as favorably as any other free toolchain.

 I think it would be a good idea, yes. The compiler itself is capable of
giving some useful warnings that gcc doesn't give and libc++ is quite
different from libstdc++, so testing with it can discover problems due to
relying on libstdc++-specific behaviour.

GC> >  Note that the matters are further confused by the use of the special
GC> > blank_is_not_whitespace_locale() for the interpreter stream.
GC> 
GC> BTW, that locale is used so that natural-language strings like:
GC> 
GC> #define solve_type_NAMES \
GC>     {"No solve" \
GC>     ,"Specified amount" \
GC>     ,"Employee premium" \
GC> 
GC> can be extracted as a unit, regardless of embedded '0x020' ASCII spaces.

 Ah, I see, thanks. This is a quite ingenuous solution to this problem.

GC> Please take a look at how I changed that (once I push the commit).

 I'll have a look at the commit once it's done and will test it with clang
and MSVC.

GC> "interpreter >> std::ws" is a valid part of the customary idiom;
GC> the error was testing stream state with operator!() after it.
GC> 
GC> There's no need to distinguish these erroneous cases:
GC>   "Allow MEC   "
GC>   "Allow MEC sometimes"

 This makes sense to me.

GC> >  Finally, while I'm looking at this code, I can't help wondering why do we
GC> > have a dummy "To" parameter in this function template? Is it some
GC> > workaround for ancient and not supported any more compiler(s)? Because it
GC> > doesn't seem to be needed any longer.
GC> 
GC> Isn't it needed because the template-parameter list and the argument list
GC> have different orders?

 Sorry, I might be missing something obvious here, but I don't see how can
this be a problem? My point was, I think, that we can, and should, call
stream_cast<> with explicit To, just for any other "casts", i.e. write

        To to = stream_cast<To>(from);

instead of

        To to;
        to = stream_cast(from, to);

which is currently allowed (and used in a couple of places), but seems
somewhat confusing to me, because it's natural to write

        To to;
        stream_cast(from, to);

expecting that this would modify "to".

GC>   template<typename To, typename From>
GC>   To stream_cast(From from, To = To())
GC> 
GC> With
GC> -  To stream_cast(From from, To = To())
GC> +  To stream_cast(From from)
GC> I get:
GC> 
GC> i686-w64-mingw32-g++ ... ... /opt/lmi/src/lmi/datum_string.cpp 
-odatum_string.o
GC> In file included from /opt/lmi/src/lmi/value_cast.hpp:29,
GC>                  from /opt/lmi/src/lmi/datum_string.hpp:29,
GC>                  from /opt/lmi/src/lmi/datum_string.cpp:24:
GC> /opt/lmi/src/lmi/stream_cast.hpp:153:20: error: template-id 
‘stream_cast<std::__cxx11::string>’ for
GC>  ‘std::__cxx11::string stream_cast(char*, std::__cxx11::string)’ does not 
match any template declaration
GC>  inline std::string stream_cast<std::string>(char* from, std::string)
GC>                     ^~~~~~~~~~~~~~~~~~~~~~~~
GC> /opt/lmi/src/lmi/stream_cast.hpp:99:4: note: candidate is: ‘template<class 
To, class From> To stream_cast(From)’
GC>  To stream_cast(From from)
GC>     ^~~~~~~~~~~

 This template specialization would need to be adjusted and its
"std::string" dummy parameter removed, of course.

 If you'd like, I could make (and test) a patch removing the "To = To()"
dummy template parameter to show more precisely what I suggest to do (once
you commit your modifications to stream_cast<>, to avoid any conflicts).

 Please let me know if I should do this and, to return to the beginning of
this message, if you think it could be worthwhile defining _jdn/_ymd custom
literals for the dates.

 Thanks,
VZ

pgpkSVD3STWQ5.pgp
Description: PGP signature