On Jul 9, 2013, at 11:27 AM, Howard Hinnant <[email protected]> wrote:
> 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! thanks for the quick feedback. > Comments: > > * Comments section in <iomanip> needs to be tagged with C++14. Done. > * 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. I left this alone, b/c I think I'm right. I've pinged Beman to find out why he specified it this way. > > * 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.). I think I've done this. > > * 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. Moved into <ios> in revision 185968, and now used. > * __is >> noskipws; > > This is the most expensive way to turn off skipws. noskipws(__is) is faster. Fixed. > * 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. Changed. [ except I used __is.fail() instead of !__is.fail() ;-) ] > * 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. More tests added. Still mulling over what other tests need to be added. Wondering about failing tests, too. Revised patch attached. -- Marshall Marshall Clow Idio Software <mailto:[email protected]> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
n3545-2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
