Skip to content

Conversation

@baldapps
Copy link

No description provided.

marco added 3 commits March 20, 2022 13:20
Proxy has two template parameters but they are always the
same so they don't provide any kind of uniqueness.
@hliberacki
Copy link
Owner

@baldapps Thanks for your contribution!

Looks good to me, apart from the removal of the template parameter

using type = R (C::*)(Args...) const;
};

template<class Tag, class T>
Copy link
Owner

Choose a reason for hiding this comment

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

This parameter is needed due to its uniqueness - that's why there is Tag and T - here is a description https://github.com/insooth/insooth.github.io/blob/master/accessing-private-members.md#uniqueness

Copy link
Author

Choose a reason for hiding this comment

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

I read it but what you have in the code it's not what you describe in the article. In the code the parameters are always the same, they're always equal, so I can't see how uniqueness can be applied currently. In addition after the modification all tests are executed without any problem.

@HDembinski
Copy link

The change to use a real reference instead of std::reference_wrapper is very good, also the forwarding of arguments. I think it would be great to have this PR merged with just these changes and leave the controversial one for later.

How do you intend to deal with the API break, though? cpp-member-accessor has no releases, but this is a breaking change. I think you want to make a 0.1 release with the old version and give this a 0.2 version.

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.

3 participants