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

[Jim Porter]

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