Re: [boost] [review][mp11] Formal review of Mp11

Subject: Re: [boost] [review][mp11] Formal review of Mp11
From: Joaquin M López Muñoz (joaquinlopezmunoz_at_[hidden])
Date: 2017-07-17 07:40:56

El 17/07/2017 a las 1:06, Peter Dimov via Boost escribió:
> Joaquin M López Muñoz wrote:
>
>> 1.1 I really don't like the mp_ prefix. I understand this is meant to 
>> minimize clashes
>> when using namespace::mp11, but the same purpose can be served by simply
>> using namespace mp=boost::mp11.
>
> Well... it's also meant to make it possible to move mp11 into std 
> without changes. :-)

We already have subnamespaces in std (e.g. std::chrono), so your 
standardization plan could
be realized as s/boost/std.

> 1.3 I find that these metafunctions are missing/misplaced:
>>    pop_back
>
> pop_back is not provided for the same reason std::vector doesn't 
> provide pop_front - it's
> much less efficient than its siblings, and providing it could mislead 
> people into thinking that
> it's equally efficient.

Understood.

>>    at/at_c (should be in list.hpp)
>>    insert/insert_c/erase/erase_c (should be in list.hpp)
>>    replace_at/replace_at_c
>
> The partitioning is roughly based on whether the operation can be 
> trivially expressed
> (f.ex. mp_second<L<T1, T2, T...>> -> T2), or requires a more elaborate 
> implementation
> (as all primitives taking an arbitrary index require.)

I see. My assumption was that list.hpp would contain mp equivalents for 
std container
member functions, while algorithm.hpp was home to mp translations of
<algorithm> functions. Maybe an intro for each header in the reference 
can help
orient readers.

> [...]
>
>> 1.5 Treatment of sets and maps is very dissimilar. Why mp_map_find but no
>> mp_set_find? Why mp_set_push_(back|front) but no 
>> mp_map_push_(back|front)?
>> Why mp_map_erase but no mp_set_erase? I think both interfaces should  
>> have
>> the same functions, except possibly mp_map_(replace|update).
>
> Set operations are only provided for the cases where the generic 
> list-based algorithm
> would be unsuitable or less efficient.

I'm not sure this observation entirely addresses my question.

* mp_set_find is not provided because mp_find does the job --OK, but 
consider my
point 1.5.1 then.
* If mp_set_push_(back|front) is provided (no suitable list-based 
algorithm), why not
mp_map_push_(back|front)?

>> Porting from / coexisting with Boost.MPL would be greatly aided by 
>> some utility
>> function to convert Boost.MPL sequences to Mp11 lists:
>
> ...
>
> This unfortunately doesn't work in general - as Bruno Dutra explained 
> to me - because
> MPL operations on, f.ex. mpl::vector, do not necessarily return an 
> mpl::vector.

Something more generic can be provided

   struct mp_mpl_sequence_folder
   {
     template<typename T,typename L>
     struct apply{using type=mp_push_front<T,L>;};
   };

   template<typename MplSequence>
   struct mp_mpl_sequence_impl:boost::mpl::reverse_fold<
     MplSequence,
     mp_list<>,
     mp_mpl_sequence_folder
   >{};

   template<typename MplSequence>
   using mp_mpl_sequence=typename mp_mpl_sequence_impl<MplSequence>::type;

with faster specializations for boost::mpl::vector and other MPL sequences.

> [...]
>
>>   - It is not "template class" but "class template".
>
> Yes, it's actually "template class", meaning a class template 
> instantiation. A template
> class is a class; a class template is a template. Too subtle, perhaps, 
> but this is correct
> terminology.

I fail to see "template class" defined in N4296 (one ocurrence only in 
[sequences.general],
and seems an editorial typo as it obviously refers to class templates, 
and a handful of
ocurrences of "non-template class"). Additionally:

*  "A metafunction is a class template or a template alias [...]":
s/"template alias"/"alias template"?
* "mp_unique<L> returns a list of the same type as L [...]": being "the 
same type" needs
definition. This is found in other places throughout the documentation.

Joaquín M López Muñoz