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

Consider renaming apply to unpack #69

Open
pfultz2 opened this issue Mar 24, 2017 · 4 comments
Open

Consider renaming apply to unpack #69

pfultz2 opened this issue Mar 24, 2017 · 4 comments
Labels

Comments

@pfultz2
Copy link

pfultz2 commented Mar 24, 2017

Generally, apply is synonymous with invoke. MPL library uses apply to invoke a metafunction or lambda. Boost.Hana's apply works just like std::invoke.

However, your apply unpacks the elements of a list and then invokes the lambda, which I found to be surprising. It seems like unpack would be a better name and would avoid the confusion.

@brunocodutra
Copy link
Owner

I agree naming conventions in Metal are far from ideal, but it's unclear to me what the best approach would be.

One one hand, Metal follows the standard library by naming its algorithms after std::invoke and std::apply. On the other hand, well known metaprogramming libraries such as Hana and MPL, which were released long before C++17 shaped up, follow different conventions for one reason or another.
To make things even more complex, Hana and MPL disagree on what unpack should mean. While Hana defines it as a metafunction applied to the contents of a tuple, MPL uses unpack_args for something slightly different, namely an adaptor that lifts Metafunction Classes to operate on Sequences.

Since there doesn't seem to be a universal source of truth when it comes to naming these algorithms, I'd rather stick to the conventions used in the standard library. Now, to make things less surprising to someone coming from Hana, I could provide metal::unpack as an alias to metal::apply , does that sound reasonable?

@pfultz2
Copy link
Author

pfultz2 commented Mar 24, 2017

MPL uses unpack_args for something slightly different, namely an adaptor that lifts Metafunction Classes to operate on Sequences.

This is actually similar to the Fit library where fit::unpack is a function adaptor.

Now, to make things less surprising to someone coming from Hana

Its not just people coming from Hana, but also coming MPL, Fit, or Egg.

I could provide metal::unpack as an alias to metal::apply , does that sound reasonable?

The surprise comes from using metal::apply for function application. Still providing the same metal::apply doesn't help alleviate the surprise. This is why I suggested a rename.

@brunocodutra
Copy link
Owner

The surprise comes from using metal::apply for function application. Still providing the same metal::apply doesn't help alleviate the surprise.

I understand the issue, the proposal to add metal::unpack would only solve one half of the problem, namely, users looking for a way to call a metafunction to the contents of a List coming from a library where the term unpack is used.

The real problem at hand really boils down to changing the meaning of apply to match that of std::invoke, but, since we now have std::apply in the standard library, I feel it would still be confusing just the same way.

To avoid making hasty decisions, I'll leave this issue open and marked as a Question, so other users may chime in if they have any strong opinion on this naming convention.

@pfultz2
Copy link
Author

pfultz2 commented Mar 24, 2017

The real problem at hand really boils down to changing the meaning of apply to match that of std::invoke

Well that's backwards. C++ has had a long history of using apply to mean apply arguments to function(which is what it commonly means in other programming languages as well). It should be:

The real problem at hand really boils down to changing the meaning of apply to match that of unpack.

Because of the confusion around apply, it would be better to just avoid it and just provide metal::unpack and metal::invoke with no metal::apply at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants