search

brunocodutra / metal

Love template metaprogramming
https://brunocodutra.github.io/metal/
MIT License
324 stars 23 forks source link

Consider renaming apply to unpack #69

Open pfultz2 opened 7 years ago

pfultz2 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.