On 05/06/2024 11:28, Frederick Virchanza Gotham via Std-Proposals wrote:
> NOTE: This below post is not about std::elide, it is about my paper
> "PRvalue Parameters" :
> http://www.virjacode.com/papers/prvalue_params.htm
>
>
> On Tue, Jun 4, 2024 at 3:39 PM Lénárd Szolnoki wrote:
>>
>> The proposed change as motivated by optional::emplace breaks the
>> following example:
>>
>> https://godbolt.org/z/ejYT4jG5K
>>
>> void foo(std::optional<std::string>& opt_str) {
>> if (opt_str.has_value()) {
>> opt_str.emplace(
>> std::move(*opt_str) + " World!"
>> );
>> }
>> }
>>
>> The example is a bit tortured with std::optional<std::string>, but would
>> probably make more sense with a `std::optinal<HardToMutate>` that's
>> easier to mutate by emplacing over than through non-const member functions.
>
>
> I don't think that's right. If I understand correctly the point you're
> making, you're saying that "foo" is supposed to clear the value of its
> argument, however if my proposed new feature, "PRvalue parameters",
> were to make it into C++, then the argument would no longer be
> cleared. Is that what you're saying?

No, that's not what I am saying. In your paper optional::emplace is
implemented the following way:

template<typename T>
T &optional<T>::emplace(T &&&arg)
{
     this->reset();
     T &retval = *::new(this->buffer) T( arg() );
     this->has_value_bool = true;
     return retval;
}

This is what happens when called with the "deferred" or "lazy" argument
`std::move(*opt_str) + " World!`.

1. this->reset()
After this the optional doesn't hold a value anymore.

2. T& retval = *::new(this->buffer) T( arg() );
Here `arg()` evaluates `std::move(*opt_str) + " World!`. But `opt_str`
has no value anymore, therefore `*opt_str` is UB.

This is a breaking change of semantics to the current implementation of
optional::emplace for this example, as when function arguments are
sequenced before the function body there is no issue with the code.

UB can manifest in more direct ways for other containers, like
`vector::emplace_back` or `map::try_emplace`, where when evaluating the
lazy argument the class's invariants might not even hold (like having a
"hole" in the vector, unordered maps in the middle of rehashing, etc...).

I experienced issues like this first hand when implementing a function
memoizing class that also dealt with recursive functions. I implemented
it on top of absl containers, and at an early implementation it ended up
calling try_emplace recursively (I used a std::elide-like class). It
blew up spectacularly. This is not a theoretical issue in general.

Having said that I also see upsides of lazy arguments, but changing
existing member overload sets might be a hard sell. I could envision
something like `emplace_lazy`, not unlike `emplace_invoke` proposed
elsewhere, but a bit more ergonomic to actually use.

There are also cases where there are no broken class invariant issues
caused by a lazy argument, like `std::construct_at`.

> If so, then I don't think that's
> right.
>
> Focusing on the following expression:
>
> std::move(*opt_str) + " World!"
>
> Looking at "operator+" for "std::string" on this page:
> https://en.cppreference.com/w/cpp/string/basic_string/operator%2B
>
> If you look at No. 10, the following gets called:
>
> template< class CharT, class Traits, class Alloc >
> std::basic_string<CharT,Traits,Alloc> operator+(
> std::basic_string<CharT,Traits,Alloc>&& lhs, const CharT* rhs );
>
> which has the effect of:
>
> template< class CharT, class Traits, class Alloc >
> std::basic_string<CharT,Traits,Alloc> operator+(
> std::basic_string<CharT,Traits,Alloc>&& lhs, const CharT* rhs )
> {
> lhs.append(rhs);
> return std::move(lhs);
> }
>
> The return statement is given an XRvalue (note that it is not a
> PRvalue). This comes under "non-mandatory copy/move elision". The
> return value might be the same object as was passed in the first
> parameter, or it might be a newly-created object which was
> move-constructed from the first argument (which would result in the
> original string being cleared). I therefore think that it's
> "unspecified behaviour" as to whether or not the original string gets
> cleared by the addition operation. Adding "PRvalue parameters" to the
> language will have no effect on "non-mandatory copy/move elision", and
> so after PRvalue Parameters get added to the language, it will still
> be unspecified behaviour as to whether the string gets cleared.