Fixing shared_executor_test

[hkaiser]

• flyby: adding missing typedef

[sithhell]

Shouldn't this be rather done in the trait? Is it an error in general to return a reference from the passed function?

[hkaiser]

I tried solving it in the trait, but was not able to do it there. If you have an idea how this could be formulated, I'd appreciate any ideas.

[sithhell]

I tried solving it in the trait, but was not able to do it there. If you
have an idea how this could be formulated, I'd appreciate any ideas.

I am still trying to wrap my around why this is needed in the first place.

[K-ballo]

Tampering with the result like this looks wrong to me. It will also make some valid corner cases ill-formed.

[hkaiser]

I agree. Any suggestions on how to solve this? The patch here was just meant as a stop-gap measure to make tests pass. Any proper solution would be certainly preferred.

[hkaiser]

I tried solving it in the trait, but was not able to do it there. If you
have an idea how this could be formulated, I'd appreciate any ideas.

I am still trying to wrap my around why this is needed in the first place.

shared_future<>::get() returns a const& to the result. This code extracts the result, passing it along, the shared_future itself however goes out of scope.

[K-ballo]

Intuitively I would expect execute_helper::run to have a return type that matches that of the decayed function invocation. Perhaps:

            typename hpx::util::result_of<
                typename hpx::util::decay<F>::type()
            >::type

That might show (real) issues when shared_future is involved though.

[hkaiser]

That might show (real) issues when shared_future is involved though.

That is exactly the problem we're seeing. The current code uses auto-returntype deduction which amounts to the same things as you suggested, causing the code to return a dangling reference to the user of executor_traits<Executor>::execute()

[K-ballo]

I don't think that's the case, if F() returns a reference type then shared_future<R&>::get() will return a reference that will only dangle if F() itself does.

[hkaiser]

I don't think that's the case, if F() returns a reference type then shared_future<R&>::get() will return a reference that will only dangle if F() itself does.

Sure. The problem lies in how the trait emulates the synchronous execute:

{
    return exec.async_execute(std::forward<F>(f)).get();
}

which will - as you said - dangle the reference.

Additionally, as you said in IRC, the shared_future<> will prevent us from using move-only types. So I'm not sure on how to create a generic emulation in the trait class supporting all of the use cases without the ugly hack I proposed in this PR.

[K-ballo]

The ugly hack presented here already prevents the use of move-only types, there's no way around that with shared_future.

Without knowing much about executor_traits, if the point of call is an attempt to implement synchronous calls on top of the existing interface then the implementation doesn't quite work either, as it decays the callable. That might end up picking the wrong overload, or simply be ill-formed (assuming call does not document any non-obvious requirements).

Wrapping the callable with ref might help, but it doesn't solve lack of support for movable only return types.

[hkaiser]

@sithhell, @K-ballo: I think your comments have been addressed.

[sithhell]

LGTM