Skip to content

Add member function implementation stubs using reflection - #168

Open
jbcoe wants to merge 16 commits into
mainfrom
jbcoe-implementation-stubs
Open

Add member function implementation stubs using reflection#168
jbcoe wants to merge 16 commits into
mainfrom
jbcoe-implementation-stubs

Conversation

@jbcoe

@jbcoe jbcoe commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Adds stubs for member function to both protocol and protocol_view. Stub member functions cannot be called (there is currently no vtable to forward calls), but their signatures can be checked at compile-time.

@jbcoe jbcoe changed the title Jbcoe implementation stubs Add member function implementation stubs using reflection Aug 24, 2026
@jbcoe
jbcoe changed the base branch from main to jbcoe-rename-protocol-hh August 24, 2026 01:26
@jbcoe
jbcoe force-pushed the jbcoe-implementation-stubs branch 2 times, most recently from 979e672 to 51dc367 Compare August 24, 2026 01:33
Base automatically changed from jbcoe-rename-protocol-hh to main August 24, 2026 15:41
@jbcoe
jbcoe force-pushed the jbcoe-implementation-stubs branch 2 times, most recently from c55ff80 to 9c390f3 Compare August 24, 2026 21:17
@jbcoe
jbcoe changed the base branch from main to jbcoe-docker-uses-ubuntu-26.04 August 24, 2026 21:17
@jbcoe
jbcoe force-pushed the jbcoe-docker-uses-ubuntu-26.04 branch from 88589f9 to e80adf5 Compare August 24, 2026 22:25
@jbcoe
jbcoe force-pushed the jbcoe-implementation-stubs branch from a6962a4 to f0ded1c Compare August 24, 2026 22:25
Base automatically changed from jbcoe-docker-uses-ubuntu-26.04 to main August 24, 2026 22:25
@jbcoe
jbcoe force-pushed the jbcoe-implementation-stubs branch from f0ded1c to 553b460 Compare August 24, 2026 22:25
@jbcoe
jbcoe marked this pull request as ready for review August 24, 2026 22:25
@jbcoe
jbcoe requested a review from RyanJK5 August 24, 2026 22:26
@jbcoe
jbcoe marked this pull request as draft August 25, 2026 14:53
@jbcoe
jbcoe removed the request for review from RyanJK5 August 25, 2026 14:53
@jbcoe
jbcoe force-pushed the jbcoe-implementation-stubs branch from b1fbb5d to 8dbc192 Compare August 25, 2026 16:21
@jbcoe
jbcoe marked this pull request as ready for review August 25, 2026 17:10
@jbcoe
jbcoe requested review from RyanJK5 and hanickadot August 25, 2026 17:14

@RyanJK5 RyanJK5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! The core of it seems to be functioning nicely :)

Comment thread reflection/protocol.hh
Comment thread reflection/protocol.hh Outdated
Comment thread reflection/protocol.hh Outdated
Comment thread reflection/protocol.hh Outdated
Comment thread reflection/protocol.hh Outdated
Comment thread reflection/protocol.hh
Comment thread reflection/protocol_test.cc
Comment thread reflection/protocol_test.cc
Comment thread reflection/protocol_test.cc
Comment thread reflection/protocol.hh Outdated
@jbcoe
jbcoe requested a review from RyanJK5 August 27, 2026 08:44

@RyanJK5 RyanJK5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good progress, I tagged a few things that I missed in my initial review.

Comment thread reflection/protocol.hh
Comment on lines +194 to +206
template for (constexpr std::meta::info member : std::define_static_array(
std::define_static_array(
members_of(InterfaceType,
std::meta::access_context::unprivileged())) |
std::views::filter(std::meta::is_function) |
std::views::filter(
std::not_fn(std::meta::is_special_member_function)) |
std::views::filter(
std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier))) {
member_base_types.push_back(
^^typename member_base_generator<member>::member_base);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is a bit less noisy and reduces some of the heavyweight template instantiations that come from template for (may not be perfect):

Suggested change
template for (constexpr std::meta::info member : std::define_static_array(
std::define_static_array(
members_of(InterfaceType,
std::meta::access_context::unprivileged())) |
std::views::filter(std::meta::is_function) |
std::views::filter(
std::not_fn(std::meta::is_special_member_function)) |
std::views::filter(
std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier))) {
member_base_types.push_back(
^^typename member_base_generator<member>::member_base);
}
for (std::meta::info member : members_of(InterfaceType, std::meta::access_context::unprivileged()) |
std::views::filter(std::meta::is_function) |
std::views::filter(std::not_fn(std::meta::is_special_member_function)) |
std::views::filter(std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier)) {
member_base_types.push_back(dealias(substitute(^^member_base_generator_t, {reflect_constant(member)})));
}

It would require adding:

template <std::meta::info Member>
using member_base_generator_t = member_base_generator<Member>::member_base

Comment on lines +412 to +421
TEST(ReflectionProtocolViewTest, NonConstMemberFunctionNotInvocableFromConst) {
struct Interface {
void update(int value);
};

static_assert(
!std::is_invocable_v<
decltype((std::declval<const protocol_view<Interface>&>().update)),
int>);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this behavior incorrect? I thought protocol_view used shallow const, so a const protocol_view should be able to call update, whereas a protocol_view<const Interface> should not.

Comment thread reflection/protocol.hh
Comment on lines +107 to +114
consteval auto protocol_interface_functions_of() {
return std::define_static_array(
std::define_static_array(
members_of(Type, std::meta::access_context::unprivileged())) |
std::views::filter(std::meta::is_function) |
std::views::filter(std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is producing a static array anyway, I think it makes sense to make this a variable template. That way the results are cached if this is ever called again.

Suggested change
consteval auto protocol_interface_functions_of() {
return std::define_static_array(
std::define_static_array(
members_of(Type, std::meta::access_context::unprivileged())) |
std::views::filter(std::meta::is_function) |
std::views::filter(std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier));
}
constexpr inline auto protocol_interface_functions_of = std::define_static_array(
members_of(Type, std::meta::access_context::unprivileged()) |
std::views::filter(std::meta::is_function) |
std::views::filter(std::not_fn(std::meta::is_static_member)) |
std::views::filter(std::meta::has_identifier));

NOTE: I also dropped the inner define_static_array, which I believe was unnecessary. Please add it back if I am mistaken.

Comment thread reflection/protocol.hh

template <typename T>
class protocol_view {
class protocol_view : public detail::protocol_stubs_t<T> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to my comment on the NonConstMemberFunctionNotInvocableFromConst test: I believe this is incorrect. protocol_view<I> should inherit from a slightly different version of protocol_stubs_t where every member function is const regardless of whether or not it was const in the interface. Later on, protocol_view<const I> would inherit from a protocol_stubs_t that filters out all of the non-const functions as well.

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