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

[Marshall Clow]

On Jul 7, 2014, at 11:23 AM, David Blaikie <dblaikie at gmail.com> wrote:

> +mclow
> 
> do these even need to be movable?

They need to be moveable, because they get returned from std::quoted.

As to Jim’s question; I don’t have an answer off the top of my head.
Though I will point out that string_view has the exact same lifetime problem, 
since it doesn’t own its’ storage, but rather holds a reference to other storage.

— Marshall

> 
> 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"));

Great example, btw.


>> 
>> 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