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

[Jim Porter]

In my sample implementation, I think they would need to be movable so 
that the wrapper function (my_manipulator) can move-construct its return 
value from the proxy object (my_manipulator_proxy). Then the wrapper 
makes sure that its return value is const so it can't be used by another 
move-constructor, effectively making it non-copyable and non-movable.

If you have a better idea for how to implement this, that's ok too, as 
long as the following are disallowed:

   auto foo() {
     return my_manipulator(/* ... */);
   }

   auto x = my_manipulator(/* ... */);

- Jim

On 7/7/2014 1:23 PM, David Blaikie wrote:
> +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