Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support default implementation of a dispatch #79

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

mingxwa
Copy link
Collaborator

@mingxwa mingxwa commented Mar 29, 2024

Changes

  • Added macro PRO_DEF_MEMBER_DISPATCH_WITH_DEFAULT and PRO_DEF_FREE_DISPATCH_WITH_DEFAULT to allow specifying an addition default implementation of a dispatch.
  • Added support for compilers to avoid generating duplicated code for the default implementation of a dispatch.
  • Removed macro PRO_DEF_COMBINED_DISPATCH due to incompatibility of semantics.
  • Revised semantics of concept facade and proxiable: Before this change, implementation of a dispatch is required to provide overloads of operator(), and a facade is required to be trivially default constructible. It is now required that a dispatch D shall be trivially default constructible, and shall model typename D::template invoker<...>. proxy will try to instantiate invoker with target-type-of<P> (implemented as pro::details::ptr_traits<P>::target_type) and see if it is invocable in the context; otherwise, proxy will try to fallback to invoker<void> without trying to dereference the pointer at runtime.
  • Added another 2 unit test cases ProxyInvocationTests.TestMemberDispatchDefault and ProxyInvocationTests.TestFreeDispatchDefault (converted from previous ProxyInvocationTests.TestCombinationWithIncompleteDispatch) to cover this change.

Validation

Except for the correctness of the two new macros that can be covered with unit tests, the ultimate goal of this change is to generate better code for default implementation of a dispatch. This has been verified by inspecting the generated code before and after this change.

We came up with the following test code:

namespace spec {

std::size_t GetSizeImpl(auto& self) {
  if constexpr (requires { { self.size() } -> std::convertible_to<std::size_t>; }) {
    return self.size();
  } else {
    throw std::runtime_error{"Not implemented"};
  }
}
PRO_DEF_FREE_DISPATCH(size, GetSizeImpl, std::size_t());
PRO_DEF_FACADE(BasicContainer, size);

}  // namespace spec

It models a basic container that has a member function size(), while throw an exception if it is invoked on an unsupported type. Here is the generated code (link).

image

We can see the code for the exception path was duplicated for every type that does not have member function size(). With this change, we can use PRO_DEF_MEMBER_DISPATCH_WITH_DEFAULT instead, with simpler syntax and better code generation (link).

image

Closes #78

@mingxwa mingxwa closed this Apr 1, 2024
@mingxwa mingxwa deleted the user/mingxwa/dispatch-default branch April 1, 2024 02:11
@mingxwa mingxwa restored the user/mingxwa/dispatch-default branch April 1, 2024 02:16
@mingxwa mingxwa reopened this Apr 1, 2024
@mingxwa mingxwa force-pushed the user/mingxwa/dispatch-default branch from 5c31dbb to b2f475d Compare April 1, 2024 02:20
@mingxwa mingxwa marked this pull request as ready for review April 1, 2024 11:07
@mingxwa mingxwa merged commit 4013b00 into microsoft:main Apr 2, 2024
4 checks passed
@mingxwa mingxwa deleted the user/mingxwa/dispatch-default branch April 2, 2024 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have a syntax to declare default implementation of a dispatch easier and avoid generating duplicated code
4 participants