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

[David Blaikie]

+mclow

do these even need to be movable?

On Sat, 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"));
>
> 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