[cfe-dev] [libc++] Should IO manipulators be noncopyable?

[Marshall Clow]

On Jul 5, 2014, at 3:05 PM, Jim Porter <jvp4846 at g.rit.edu> wrote:

> Recently, I was trying out the new std::quoted IO manipulator and came across an interesting issue: since libc++'s std::quoted is copyable (as are all the other IO manipulators I looked at), passing a temporary as the first argument introduces a lifetime bug in the following code:
> 
>  auto ensure_printable(const std::wstring &s) {
>    std::wstring_convert<std::codecvt_utf8<wchar_t>, wchar_t> conv;
>    return std::quoted(conv.to_bytes(s));
>  }
> 
>  // ...
> 
>  std::cout << ensure_printable(std::wstring(L"hi"));

There’s an easy fix, btw - the object returned by quoted could capture the string by value, rather than reference.
Make a copy.  :-(

BTW - even simpler example

auto boom ( const std::string &s ) { return std::quoted(s); }
auto foo = boom ( “Hi Mom!” );

foo now has a reference to a destructed string.

— Marshall

> 
> In C++98, it wasn't possible to do something like this without making obviously non-standard code (i.e. explicitly writing out the implementation-defined named for the return type of std::quoted()). In C++11, and even moreso in C++14, this is no longer necessary thanks to the `auto` keyword. Hence, (mostly) innocent-looking code like above can introduce subtle bugs.
> 
> Obviously, since the return type of std::quoted() is unspecified, the above code relies on undefined behavior: there's no guarantee in the standard that it's copyable in the first place! After some discussion, this got me to thinking about why any of the IO manipulators are implemented as copyable types; presumably, this is mostly because it didn't matter back in C++98 when most of them were introduced. However, with deduced return types, this matters quite a bit more.
> 
> If the IO manipulators were written to return a const version of their proxy type, and said type were move-only, I think this would resolve the issue, and only allow the IO manipulators to be used as temporaries (much like they were in C++98). Something like so:
> 
>  struct my_manipulator_proxy {
>    my_manipulator_proxy(/* ... */) {}
>    my_manipulator_proxy(const my_manipulator_proxy &) = delete;
>    my_manipulator_proxy(my_manipulator_proxy &&) = default;
>    /* ... */
>  };
> 
>  std::ostream & operator << (std::ostream &s,
>                              const my_manipulator_proxy &m) {
>    /* ... */
>  }
> 
>  const my_manipulator_proxy my_manipulator(/* ... */) {
>    return my_manipulator_proxy(/* ... */);
>  }
> 
> Having looked at the implementation of std::quoted, this seems like an uncomplicated change (I imagine it's so for the other manipulators as well), and helps prevent undefined behavior. Does this sound like a reasonable way to go? I'd be happy to write a patch if others agree that this is sensible.
> 
> - Jim
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev