Re: [boost] Review Request: poly_collection

Subject: Re: [boost] Review Request: poly_collection
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2017-03-02 17:46:37

Le 02/03/2017 à 10:41, Joaquin M López Muñoz via Boost a écrit :
> El 01/03/2017 a las 23:15, Vicente J. Botet Escriba escribió:
>> Le 01/03/2017 à 09:32, Joaquin M López Muñoz via Boost a écrit :
>>> Hello,
>>>
>>> I'd like to ask for formal review of (candidate) 
>>> Boost.PolyCollection [...]
>>
>> Hi,
>>
>> thanks for proposing this library Joaquîn.
>
> Hi Vicente, thanks for your interest.
>
>> I've surely missed why base_collection is named this way. Is it 
>> because the parameter is a base class. Could you confirm?
>
> Exactly. base_collection<Base> pretty much behaves as a container of 
> Base objects.
>
>> wondering if a single class with specializations wouldn't help
>>
>>     collection<polymorphic<T>>
>>     collection<function<T(U)>>
>>     collection<any<Concept>>
>>
>> In this way we could have
>>
>>     collection<Model, Allocator>
>>
>> BTW, I see that you have in the implementation a 
>> detail::poly_collection<Model, Allocator> that is the base of the 3 
>> collections.
>
> This is mostly a naming issue. Indeed the three collections derive 
> from the same
> implementation and could be publicly provided as specializations of 
> the same class,
> if this is what reviewers lean towards. Personally I like it better to 
> keep names
> separated as in the current proposal.
Ok, no problem.
>
>> In your blog there were some comments about a collection of variants. 
>> Have you considered adding a variant_collection<Ts...>
>
> I've thought about it. Such a collection would deviate from the others 
> both in terms
> of its interface (no type registration, for instance) and 
> implementation --the data
> structure detail::poly_collection relies on could be used to support 
> variants, but
> more efficient implementations exist for this particular case.
Ok, I see. Even if it needs a specific implementation I believe it is 
worth having it. I say that because more and more people are using 
variant as an alternative way of polymorphism (See Andrzej blog - 
Another polymorphism)
https://akrzemi1.wordpress.com/2016/02/27/another-polymorphism/
>
>> As the collections are closer to multisets (unordered/not-hash), 
>> maybe it is worth using this name.
>
> I'd say similarities with unordered_multiset are superficial (segments 
> resemble
> buckets) and the interfaces ultimately are too different. I chose 
> "collection" because
> the term is not overloaded in the STL or Boost.
My intention was to confirm we have something close to a multiset. 
Anyway, it would be great to see a comparison table. Aside construction, 
what is provided by std::unordered_multiset that is not provided by the 
poly_collections (or it is provided in a different way) ?

>
>> Have you think about using the same design for maps/unordered maps of 
>> polymorphic objet (where the key is not polymorphic)?
>
> Same-type contiguity is an essential feature of PolyCollection: I fail 
> to see how this
> could be preserved for an associative container. Care to elaborate?
I was thinking in having the a map of (keys,poly_collection.::iterator) 
and store the values using your poly_collection. After thinking a little 
bit more on that maybe there is a problem with the iterators stability.
>
>> There is a C++ proposal for a polymorphic_value 
>> (https://github.com/jbcoe/polymorphic_value/blob/master/draft.md). 
>> Your base_collection<T> corresponds quite well to this 
>> polymorphic_value<T>, isn't it?
>> Maybe polymorphic_collection<T> or collection<polymorphic<T>>
>> The main difference is the memory management, polymorphic_value can 
>> be adapted to use an Allocator.
>
> base_collection<T> behaves approximately as a 
> std::vector<polymorphic_value<T>>,
> the crucial difference being that the former groups elements by 
> concrete type in
> contiguous chunks of memory, while the latter holds indirection 
> pointers behind the
> scenes, so there is no real memory contiguity.
I talked of polymorphic_value as it close to std::function and 
boost::type_erasure::any. At the end all of them are type erasures for 
some concept.
IMO, you base_collection<T> is closer to vector<polymorphic_value<T>> 
than to ptr_vector<T>. Of course the layout is not the same, and this 
makes the difference of your poly_collections.

>
>> What are the function types passed to for_each whee we have a 
>> function_collection or a any_collection? Is a reference to the
>> |value_type? |Do we need generic lambdas?
>
> These functions are passed a (const) value_type&, where
>
> value_type=function_collection_value_type<Sig>  for 
> function_collection<Sig>,
> value_type=any_collection_value_type<Concept>  for 
> any_collection<Concept>
>
> (see http://tinyurl.com/hmywnea , http://tinyurl.com/zh9ugl8 ). Now, 
> when type
> restitution is used (http://tinyurl.com/gopdpuz ), and only then, the 
> functions are
> passed a (const) Ti& for each of the restituted types Ti: in this 
> case, a generic lambda
> is a very convenient way of taking advantage of this, but you can also 
> resort to a
> polymorphic functor if you wish.

Let me see if I understood.
For the normal algorithms, the function has as parameter the const 
value_type and needs to use dynamic polymorphism.
When we use the “restituted" algorithm overload, the algorithm 
"restitutes" the specific types and call to an heterogeneous function, 
an overload set  for the reconstituted types, as if it were a visitor, 
isn't it?

hana::overload function should be useful here.

Vicente