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

Add support for member function to member_ptr #449

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

sdebionne
Copy link
Contributor

This PR adds an overload of operator() of member_ptr to support member function so that the following use case is now supported:

namespace ns {
    struct Person {
        std::string name;
        int age;

        void init(std::string name_arg, int age_arg) {
            age = age_arg;
            name = name_arg;
        };

        bool is_old() const { return age > 30; };
    };
}

BOOST_HANA_ADAPT_STRUCT(ns::Person,
    name,
    age,
    init,
    is_old
);


// The member names are hana::strings:
auto names = hana::transform(hana::accessors<ns::Person>(), hana::first);
BOOST_HANA_CONSTANT_ASSERT(
    names == hana::make_tuple(BOOST_HANA_STRING("name"), BOOST_HANA_STRING("age"), BOOST_HANA_STRING("init"), BOOST_HANA_STRING("is_old"))
);

int main() {
    ns::Person john{ "John", 30 };

    constexpr auto accessors = hana::accessors<ns::Person>();
    // Call john.init()
    hana::second(accessors[hana::size_c<2>])(john, "Johnny", 31);
    // Call john.is_old()
    auto old = hana::second(accessors[hana::size_c<3>])(john);
}

Could you give me some guidance about adding unit tests? I had a look in test/concept/struct but I am not sure where the new tests belongs...

@sdebionne
Copy link
Contributor Author

For symmetry with the member object version, it maybe be better to have member_ptr returning a binded member function that would be used something like:

// Call john.init()
hana::second(accessors[hana::size_c<2>])(john)("Johnny", 31);
                                              ^ member call

An other advantage of this solution is that one can introspect the type of the member functions in the same way than the member objects, e.g .

template <typename S>
constexpr auto types = decltype(hana::unpack(std::declval<S>(),
      hana::on(hana::make_tuple, hana::compose(hana::typeid_, hana::second)))){};

I have an implementation but that uses C++20 std::bind_front().

@ricejasonf
Copy link
Collaborator

ricejasonf commented Jun 7, 2019

Why does it need std::bind_front? Would a lambda suffice, and how would it capture the object? by reference? That might be the only issue here is that the user needs to know of possible dangling or copying.

For tests, I'd say test/concept/struct/macro.* files are the appropriate place to test this. Just add a member function to the test Data struct and check that it can be called.

Your statement about "symmetry" is correct. The interface to accessors should not change. What you are modifying is a "data-type" that implements the Struct concept.

Currently the instance generated by the macros explodes if you give it a member function so this appears it would be a backwards compatible improvement provided you don't modify the interface to accessors.

Here is your above code with the unmodified use of accessors against Boost 1.70. So I think this confirms member functions currently don't work with the macro.
https://godbolt.org/z/aCTQ00

@sdebionne
Copy link
Contributor Author

@ricejasonf Thanks for your feedback and for the GodBolt code, this is how I should have started this PR.

Why does it need std::bind_front? Would a lambda suffice and how would it capture the object?

I did not mean to say that std::bind_front is required, actually a variadic lambda would do, I guess, see last commit. Regarding the capture, my immediate answer would be by reference.

Ultimately I would like the function object returned by member_ptr to be compatible with Boost.CallableTraits args, see contraints, and both solutions fail to comply with the requirements. I need to do some research to see if this is possible with a little bit of meta-programming.

For now, I reverted my code to return the member function pointer (that breaks the symmetry, but works with CallableTraits):

template <typename T, std::enable_if_t<std::is_member_function_pointer_v<Memptr>, int> = 0>
constexpr decltype(auto) operator()(T &&) const
{ return ptr; }

Boost.Hana is not really a reflection library, I believe it's more a side effect, so I don't know if this PR is pushing in the right direction...

@sdebionne sdebionne force-pushed the add-struct-memfun-support branch 2 times, most recently from 22dfd8a to 3cc1a71 Compare June 14, 2019 09:29
@sdebionne sdebionne force-pushed the add-struct-memfun-support branch from 3cc1a71 to 238714a Compare June 17, 2019 09:11
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