Temporary sender in `transform_sender` may dangle
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.4k
- Forks
- 270
- Avg merge
- 3d 6h
- Merged PRs (30d)
- 39
Description
transform_sender is defined like this:
struct __transform_sender {
template <class _Self = __transform_sender, class _Domain, class _Sender, class... _Env>
STDEXEC_ATTRIBUTE((always_inline))
/*constexpr*/
decltype(auto)
operator()(_Domain __dom, _Sender&& __sndr, const _Env&... __env) const
noexcept(__nothrow_callable<__transform_sender_1, _Domain, _Sender, const _Env&...>) {
using _Sender2 = __call_result_t<__transform_sender_1, _Domain, _Sender, const _Env&...>;
// If the transformation doesn't change the sender's type, then do not
// apply the transform recursively.
if constexpr (__decay_same_as<_Sender, _Sender2>) {
return __transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...);
} else {
// We transformed the sender and got back a different sender. Transform that one too.
return _Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...);
}
}
};
In general it takes care to "perfectly backward" the value category of any custom transform_sender implementation it calls by using decltype(auto).
But I think there is a lifetime bug in the last else clause. What happens if the return value of __transform_sender_1 is a prvalue? This is given as an argument to the call _Self()(...) and therefore materializes as an xvalue. Thus, _Self()(...) possibly returns a xvalue as well, as inside, it cannot distinguish between xvalue and prvalue. But now we are returning a reference to a temporary! Shouldn't the "prvalue-ness" of the return value of __transform_sender_1 be "perfectly backwarded" with something like this:
if constexpr (std::is_reference_v<decltype(__transform_sender_1()(
__dom, static_cast<_Sender&&>(__sndr), __env...))>) {
return _Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...);
} else {
return auto(_Self()(
__dom,
__transform_sender_1()(__dom, static_cast<_Sender&&>(__sndr), __env...),
__env...));
}
This is the relevant place in the draft: https://eel.is/c++draft/exec#snd.transform-1
Let transformed-sndr be the expression dom.transform_sender(std::forward(sndr), env...)
if that expression is well-formed; otherwise, default_domain().transform_sender(std::forward(sndr), env...)
Let final-sndr be the expression transformed-sndr if transformed-sndr and sndr have the same type ignoring cv-qualifiers; otherwise, it is the expression transform_sender(dom, transformed-sndr, env...).
I think it should say something like:
Let transformed-sndr be the expression dom.transform_sender(std::forward(sndr), env...)
if that expression is well-formed; otherwise, default_domain().transform_sender(std::forward(sndr), env...)
Let final-sndr be the expression transformed-sndr if transformed-sndr and sndr have the same type ignoring cv-qualifiers; otherwise, it is the expression auto(transform_sender(dom, transformed-sndr, env...)) if transformed-sndr is a prvalue, and transform_sender(dom, transformed-sndr, env...) otherwise.
Also, in the sender adaptors like https://eel.is/c++draft/exec#then-3 for example, it should probably say auto(transform_sender(get-domain-early(sndr), make-sender(then-cpo, f, sndr))), right? make-sender is a prvalue, and this should be preserved.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the __transform_sender entry point shown in the issue and compare its return-category handling with the linked exec.transform draft wording. Determine how prvalue results from transform_sender and sender adaptors should be preserved without returning a reference to a temporary; done requires resolved implementation and specification semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100