Implementation relies on std::result_of, which is deprecated in C++17 and removed in C++20

[Amir Kamil]

We use std::result_of in a few places in the UPC++ implementation (completions, bind, and future apply). Due to various defects, std::result_of was replaced by std::invoke_result in C++17. It was deprecated in C++17 and removed in C++20.

We should switch to using std::invoke_result when the underlying compiler supports it.

We may also consider backporting invoke_result for use with older compiler versions to avoid the defects in result_of, but I don’t consider this a priority.

[Paul Hargrove]

AI me ( @Paul Hargrove ): survey the compilers we use in nightly tests to see if any reject std::result_of when passed --std=c++2a.

[Colin MacLean]

I also noticed that some of the UPC++ code does the same thing of applying arguments to a function pointer, Fn(Args...), where Fn is a function pointer type, rather than piecing together a function signature from its return type R(Args...), where R is the return type. While the latter puts together a function type from components, the former applies arguments to a function pointer type and has some side effects that can change things from the original types (and can cause things to break with abstract types and such). This is why std::result_of was deprecated, but this same sort of code that lead to std::result_of being removed from the standard for its bad behavior exists within UPC++ as well (in the templating of rpc,for instance). I’m not sure if the way we use it can actually result in broken behavior, but it’s worth testing and perhaps worth changing to piece together the function signature from a return type. It would be worth putting together some test cases where std::result_of is known to be wonky and see if it breaks things with either UPC++'s use of std::result_of or with UPC++ forming of function types. It may also be worth providing an actual implementation of std::invoke_result (such as using the implementation from cppreference) that’s compliant with std::invoke_result rather than aliasing std::result_of when std::invoke_result is unavailable.

[Dan Bonachea]

The list of "bad behaviors" in std::result_of is here

I agree in principle with Colin's point, but I'm skeptical that it matters in practice for any use cases of interest.

For example, array types are not Serializable and even asymmetric deserialization cannot produce them, so RPC callback arguments will never have array type. Similarly RPC arguments receive deserialized copies that are passed as rvalues to the callback, so I think top-level const qualification is irrelevant, and we explicitly prohibit use of volatile for most cases. One probably could use asymmetric deserialization to generate an abstract type argument by erasing type information during deserialization, but that seems like a decidedly "weird" thing to do.

In short, I think migrating more of the implementation to std::invoke_result (or a look-alike) should remain very low priority until/unless we have a compelling use case that breaks.

[Colin MacLean]

Agreed. The only thing I’d be worried about are cases like where a function rather than a lambda are passed to rpc and that function has a signature that might lead to wonky behavior. The situation I’m imagining is something like calling void foo(AbstractBase&) directly with rpc rather than wrapping it in a lambda and passing the Derived to rpc as the argument. I still haven’t explored how UPC++ handles covariance of function parameters, which I know is something that can take quite a bit of effort to do correctly. The concern is bigger for the asymmetric case like what we could do for runtime function pointers, since a mismatch in the type information would cause things to miss looking up the stored function correctly. Being exacting in that case is a lot more important than in symmetrical usage as covariance is much more likely to be encountered. This is more of a “best practices” sort of issue and one that should probably be kept in mind while writing new code. Low priority, I agree.

[Amir Kamil]

From what I understand, fixing the problem with std::result_of would require not only proving a real implementation of invoke_result, but also changing the calls themselves to use the equivalent of std::invoke. That’s a pretty invasive change, and one that I’m reluctant to pursue until we have a compelling reason to do so.

The point of PR 334 is just to ensure that we are C++20-compliant, without necessarily fixing the issues with std::result_of.

[Dan Bonachea]

PR 334 merged at 1cc343d fixes the runtime defect to handle theoretical C++20 compilers that might lack an implementation std::result_of.

The remaining discussion in this issue concerns unsubstantiated use cases that might potentially break result_of-style expressions in the runtime. If someone can show a compelling use case that breaks our current runtime in this manner, please open a new issue to track that defect.