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

Simplify proxy_invoke and proxy_reflect #226

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

mingxwa
Copy link
Collaborator

@mingxwa mingxwa commented Dec 30, 2024

Motivation

In the current design, proxy_invoke() and the accessor of a convention type (specifically details::conv_impl) take the convention type as their template parameter. Similarly, proxy_reflect() and the accessor of a reflection type (specifically details::refl_impl`) also take the reflection type as their template parameter. The abuse of convention types and reflection types on these definitions makes their semantics more complex than necessary. Simplifying their definitions makes it easier to reason the code.

For example, for the following facade definition:

PRO_DEF_MEM_DISPATCH(MemFoo, Foo);

struct MyFacade : pro::facade_builder
    ::add_convention<MemFoo, void(), void(int)>
    ::build {};

Before this change, proxy_indirect_accessor<MyFacade> inherits the following two types:

MemFoo::accessor<MyFacade, pro::details::conv_impl<false, MemFoo, void(), void(int)>, void()>
MemFoo::accessor<MyFacade, pro::details::conv_impl<false, MemFoo, void(), void(int)>, void(int)>

After this change, the two types become:

MemFoo::accessor<MyFacade, false, MemFoo, void()>
MemFoo::accessor<MyFacade, false, MemFoo, void(int)>

Although this change has no runtime side effects, it can potentially improve compilation speed especially when a convention type has multiple overloads.

Changes

  • Updated the definition of proxy_invoke. The first template parameter (convention type C) was replaced by a boolean flag (equivalent to prior C::is_direct) and a dispatch type (equivalent to prior typename C::dispatch_type).
  • Updated the definition of proxy_reflect. The first template parameter (reflection type R) was replaced by a boolean flag (equivalent to prior R::is_direct) and a reflector type (equivalent to prior typename R::reflector_type).
  • Updated the definition of the accessors in the library.
  • Revised the implementation of concept facade<F> to detect correctness of each reflection type defined in typename F::reflection_types. Also fixed the corresponding test case in proxy_traits_tests.cpp.
  • Revised the semantics of the ProAccessible requirements.
  • Updated unit tests accordingly.
  • Updated spec accordingly.

@mingxwa mingxwa requested review from tian-lt and guominrui December 30, 2024 08:20
@mingxwa mingxwa merged commit 62c052e into microsoft:main Jan 2, 2025
7 checks passed
@mingxwa mingxwa deleted the user/mingxwa/refactor-proxy-invoke branch January 2, 2025 06:03
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.

2 participants