[boost] Re: Complex type failure with boost::lexical_cast

From: Preston A. Elder (prez_at_[hidden])
Date: 2005-01-30 16:28:10

On Sun, 30 Jan 2005 13:45:21 +0100, Kevlin Henney wrote:

> Yes, lexical_cast unsets the skipping of whitespace, but that does not,
> to my knowledge, introduce problems for correctly written stream
> extraction operators.
As per my other post, most people write stream operators assuming a
default stream (ie. one that skips whitespace) is passed to them, and will
write special code to check/modify the behavior if they themselves use
non-standard operations.  ie. if their operator<< turns off skipws, then
they will turn it off on their operator>>.  Pretty much every operator>>
I've seen does not check to see if skipws is set or not.

> If reading a representation in and writing it out for a custom type
> depends on the skipping of whitespace, the stream extraction operator
> must guarantee this -- it cannot assume the state of the input stream
> will be what it needs. In other words, there is a bug in the
> implementation of operator>>. Here is a simplified (untested and not
> exception safe) sketch of the basic logic:
> 
>         ... operator>>(... &is, my_type &mt)
>         {
>                 std::ios::fmtflags old_fmt = is.flags();
>                 is.setf(std::ios::skipws);
>                 is >> mt.a >> mt.b;
>                 is.flags(old_fmt);
>                 return is;
>         }
OK - I agree, this is safe.
Now, what happens when client programmer wants to use lexical_cast on a
type they did not create, that is in a library (even a closed-source
library) where they have no control over operator>> - and that operator>>
assumes the defaults (like most people writing operator>> does).

Is it so hard to have lexical_cast have an OPTIONAL second template
argument of the std::ios::fmtflags to use (defaulting to the default
fmtflags, which is skipws) - and only do the ">> std::ws" which we do if
skipws is turned on in the fmtflags we are to use? For probably >95% of
cases, no code using lexical_cast will ever need to change, and any code
that DOES need to change, they will have a big marker in their code that
the operator>> in this case expects skipws to be turned off (ie. does not
do it itself).  And for correctly written code, as you said, it will have
no effect (they can still use a single template argument).

I'm sending this too you via. mail because I've been told you don't read
the boost-dev list, thus did not see my suggestions before.

My suggestion (sorry for those who have seen it) is to have:

template<typename Target, typename Source, std::ios::fmtflags flags =
std::ios::skipws>
class lexical_stream
{
public:
    lexical_stream()
    {
        stream.flags(flags);

        if(std::numeric_limits<Target>::is_specialized)
            stream.precision(std::numeric_limits<Target>::digits10 + 1)
        else if(std::numeric_limits<Source>::is_specialized)
            stream.precision(std::numeric_limits<Source>::digits10 + 1)
    }
    // ...

    template<typename InputStreamable>
    bool operator>>(InputStreamable &output)
    {
        if (is_pointer<InputStreamable>::value)
           return false;
        if (!stream >> output)
           return false;
        if (flags & std::ios::skipws)
           stream >> std::ws;
        return (stream && stream.eof());
    }
    // ...
};

Though this would require:
template<typename Target, typename Source, std::ios::fmtflags flags =
std::ios::skipws>
Target lexical_cast(Source in) {
    detail::lexical_stream<Target, Source, flags> interpreter;
    // ... 
};

So possibly a better solution would be:

template<typename Target, typename Source>
class lexical_stream
{
public:
    lexical_stream(std::ios::fmtflags flags = std::ios::skipws)
    {
        stream.flags(flags);

        if(std::numeric_limits<Target>::is_specialized)
            stream.precision(std::numeric_limits<Target>::digits10 + 1)
        else if(std::numeric_limits<Source>::is_specialized)
            stream.precision(std::numeric_limits<Source>::digits10 + 1)
    }
    // ...

    template<typename InputStreamable>
    bool operator>>(InputStreamable &output)
    {
        if (is_pointer<InputStreamable>::value)
           return false;
        if (!stream >> output)
           return false;
        if (stream.flags() & std::ios::skipws)
           stream >> std::ws;
        return (stream && stream.eof());
    }
    // ...
};

which would require:
template<typename Target, typename Source>
Target lexical_cast(Source in, std::ios::fmtflags flags =
std::ios::skipws) {
    detail::lexical_stream<Target, Source> interpreter(flags);
    // ...
};

Either solution will still allow:
  MyType a = boost::lexical_cast<MyType>(s);

However for the first, to turn off skipws, I would do:
  MyType a = boost::lexical_cast<MyType, std::string, 0>(s);
and for the second:
  MyType a = boost::lexical_cast<MtType>(s, 0);

Obviously the second is preferable - and as I said, it adds flexibility to
lexical_cast without too much complexity - while maintaining backward
compatibility for >95% of usages, and adds compatibility with I'd say the
majority of classes that use operator>> (most of which don't check the
state of skipws) for complex classes.

That is my $.02 - and I'll say no more on the subject (unless questioned
directly), since I now sound like a zealot enough as it is :P

-- 
PreZ :)
Founder. The Neuromancy Society (http://www.neuromancy.net)