On Jul 9, 2013, at 10:10 AM, Marshall Clow <[email protected]> wrote:

> A more substantial patch than the last two.
> Reference:
>       http://wiki.edg.com/twiki/pub/Wg21bristol/FormalMotions/N3654.html
> 
> Needs more tests - and I'm working on those.

Thanks Marshall!

Comments:

*  Comments section in <iomanip> needs to be tagged with C++14.

*  The spec says to compare characters using operator== instead of _Traits::eq. 
 I have no idea why. I think what you did is what the spec should say.  But we 
need to follow the spec, or submit an issue, and implement the most likely 
outcome of that issue.

*  Everything starting with this line:

   template <class _CharT, class Iter, class _Traits=char_traits<_CharT>>

Needs to be macro-protected (e.g. Iter, quoted_output_proxy, first, etc.).

*  Not exception safe:

    ios_base::fmtflags saved_flags = __is.flags ();
    __is >> noskipws;

    ...

    __is.flags ( saved_flags );


Search for "__save_flags" in <random> for an example of how this should be 
handled.  We may want to move __save_flags to a common ancestor include so that 
both iomanip and random can use it.

*      __is >> noskipws;

This is the most expensive way to turn off skipws.  noskipws(__is) is faster.

*  Stylistic:  I prefer !__is.fail() to !__is.good().  The former says I failed 
to read in a character.  The latter says the stream is no longer in a state 
where it can read any more characters.  The difference can be eof().  In 
practice, the last character that gets read in does not set eof().  But the 
last int that gets read in does set eof() if there is no white space beyond it. 
 This is a very subtle distinction.  The upside is that you can get away with 
!is.good() only for characters, and not for any other type.  !is.fail() always 
works.

*  Tests: looking good.  Agree they need to be filled out a little more.  The 
missing quote cases should be tested for their correct result.  Test with 
skipws on and off and ensure it gets set back to the original.  Test output of 
non-const lvalue std::string.  Test std::wstring.  Test custom delimeter and 
escape.  Test unquote for zero-sized strings.

Thanks for working this!

Howard

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to