AMDG I vote to ACCEPT PolyCollection into Boost. Details: General: Missing .gitattributes index.html: No comments. an_efficient_polymorphic_data_st.html: "interspersedly" This word sounds a bit awkward to me. "the information is typically available at compile time in the point of insertion in the vector" I would say "at the point" instead of "in the point" tutorial.html: "Imagine we are developing a role game in C++" Do you mean "role playing game"? For choosing which type to insert, std::discrete_distribution is slightly more appropriate than std::uniform_real_distribution. "...by the user code --we will see more about this later" This should be an em-dash. "Insertion mechanisms are rather..." -> "The insertion mechanisms are rather..." "to explicitly forbid" split infinitive "...it is possible to only address the elements of a given segment ..." Address here is a bit confusing as "address" has a specific meaning in programming. Access is probably a better term to use. reference.html: "...in the same order, and viceversa." s/viceversa/vice versa/ "cc.empty(index)... Throws: If the indicated type is not registered. " Does this really make sense? I think it would be more useful to treat non-registered segments as empty. Throwing here is somewhat inconsistent with operator== which considers non-registered segments to be equivalent to empty segments. "cc.max_size() REVIEW THIS DESIGN DECISION" "the minimum return value of max_size for each of the segments of the collection" I think that the fundamental invariant of max_size should be that size() <= max_size(). Both max_size and capacity are a bit strange. My inclination would be to remove capacity entirely (or rename it) and change max_size to mean the maximum possible size of the whole container. Class template base_collection "subobject(x) = static_cast<Derived&>(x) with typeid(x)==typeid(Derived)" static_cast excludes virtual inhertance, which I presume is unsupported. algorithm.hpp: 61: segment_split: ADL (Contrived test case in test_adl1.cpp) any_collection.hpp: "See http://www.boost.org/libs/any_collection for library home page." s/any_collection/poly_collection/ operator!= should be defined in the same way as operator==. Overload resolution is subtly different for a non-template friend function defined inline vs. a regular free function template. This is especially problematic because any_collection_fwd.hpp declares a different operator== which is not implemented. any_collection_fwd.hpp: http://www.boost.org/libs/any_collection again. base_collection/function_collection have identical issues. exception.hpp: No comments. detail/any_iterator.hpp: line 74: explicit operator Concrete*()const noexcept This really should have an assert that the type is correct. detail/any_model.hpp: line 53: !type_erasure::is_subconcept<type_erasure::assignable<>,Concept2>::value Boost.TypeErasure was recently extended to support move assignment (assignable<_self, _self&&>). Should this also be handled here? For the record, I'm planning to fix the constructors and assignment operators of any so that is_constructible and is_assignable, etc. work, but it's quite annoying to implement. line 66, 94, 112, 123: type_erasure::is_subconcept<type_erasure::typeid_<>,Concept>::value This isn't quite correct. You need to check typeid_<remove_cv_t<remove_reference_t<T>>> Also it looks like this code is trying to allow an any that doesn't use typeid_ to be stored in a segment of anys (at least that's the impression that I get from the definition of subobject(x) for any_collection in reference.html). Trying to do this will error out in split_segment.hpp:303 in build_index when constructing the value_type. auto_iterator.hpp: No comments. base_model.hpp: line 45: static void* subaddress(T& x){return boost::addressof(x);} This assumes that the address of the base is the address of the whole object which is wrong and can fail in the case of multiple inheritance. You need to use dynamic_cast<void*>. (Failing test case included: test_multiple_inheritance.cpp) callable_wrapper.hpp: I'm somewhat curious about the rationale for using callable_wrapper instead of std::function w/ std::ref. Ah, I see: - data() is needed by function_model::subaddress - std::function can waste a lot of memory, depending on the implementation. line 83: return r(std::forward<Args>(args)...); This fails to handle R = void correctly. The return value of the function should be ignored. (Failing test case included: test_void_return.cpp) callable_wrapper_iterator.hpp: function_model.hpp: No comments. functional.hpp: No comments. integer_sequence.hpp: This can't possibly be the best implementation. make_index_sequence<N> and make_index_sequence<M> with N != M generate N+M totally independent instantiations of make_int_seq_impl.
From the source: "...but make_index_sequence is not hard to implement *(if efficiency is not a concern)*" [emphasis added]
is_acceptible.hpp, is_callable.hpp, is_constructible.hpp, is_final.hpp, is_nothrow_eq_comparable.hpp, iterator_impl.hpp, iterator_traits.hpp, packed_segment.hpp: No comments. poly_collection.hpp: If poly_collection is defined in namespace detail, that means that your internal functions can be found by ADL for user defined types that mention poly_collection. (I always avoid putting types that are part of the public interface in namespace detail to avoid this problem.) Test case in test_adl2.cpp line 1109: "// TODO: can we do better than two passes?" This comment seems obsolete. segment.hpp, segment_backend.hpp, split_segment.hpp, stride_iterator.hpp, type_restitution.hpp, value_holder.hpp No comments. In Christ, Steven Watanabe