[C++-sig] Re: indexing_v2 status update
Raoul Gough
RaoulGough at yahoo.co.uk
Tue Nov 25 16:35:24 CET 2003
David Abrahams <dave at boost-consulting.com> writes:
> Raoul Gough <RaoulGough at yahoo.co.uk> writes:
[snip]
> "Safety" is missing from the design goals. Was that not one of the
> goals?
Well that's a tough one. For instance, using return_by_reference when
returning a value held in a container is pretty dangerous (consider
deletions or vector reallocations). On the other hand, I wouldn't like
to rule it out as an option merely on that basis. Certainly the
container_proxy component does backflips to try and prevent any
dangers, so perhaps this needs some mention of safety? All it says at
the moment is this:
Provide an emulation of Python reference semantics for /values/
in vector-like containers.
>
> I was under the impression that this:
>
> Note: the suite hasn't yet been tested on a compiler without partial
> template specialization support. If you have any results with such a
> compiler (good or bad) please report them on the mailing list for
> the Python C++-SIG.
>
> is no longer true. Wrong?
Yes, this is out of date.
>
> I don't like the use of abbrevs. like "algo_...."
You mean algo_selector? I can rename this to algorithm_selector if
you'd prefer, I don't mind either way.
>
> I'm concerned that iterator_range may duplicate functionality
> somewhere else in Boost. An iterator_range class has recently been
> discussed on the main list as part of a library review.
OK - I'll keep an eye out for this and replace it when/if boost gets
its own implementation.
> Overall, after just a quick look, I'm impressed, though I don't think
> the documentation gives a clear picture of how to use all this stuff
> yet. I wouldn't be reeady to approve it until I can read some more
> complete docs, so that I can understand the whole enchilada.
>
> Internal policies detail
>
> The container_suite object typically adds more than one function to
> the Python class, and not all of those functions can, or should, use
> exactly the same policies. For instance, the Python len method, if
> provided, should always return its result by value. The library
> actually uses up to three different sets of policies derived from
> the one provided to the with_policies function. These are:
>
> The supplied policies, unchanged
>
> The supplied precall policy only, using default_call_policies for
> result conversion.
>
> The supplied precall policies, and the supplied result conversion
> policies applied to each element of a returned list
>
> You just drop any postcall component of supplied policies? Rationale?
The supplied postcall currently gets dropped completely for functions
that return integers or void (e.g. __len__ and sort) on the assumption
that it is only intended for use when returning container
elements. For instance, you don't want to use
return_internal_reference on the result of a call to container.size().
I'm not entirely happy with this aspect myself, particularly as it
relates to the absent pop() method which would always need some kind
of by-value policy. I've thought of a clean solution, which would be
to require a CallPolicies "package" that contains typedefs and factory
functions for various different CallPolicies that the suite requires.
For example len_policies, element_policies, pop_policies or maybe a
slightly different level of granularity.
>
> The "Extending and customizing" section isn't understandable, since
> you haven't even introduced the components you're discussing at that
> point.
>
> Is it important to make reference to def_visitor, or is that merely
> an implementation detail? If the latter, it should be dropped as a
> distraction.
OK.
>
> 2] Note that Algorithms and ContainerTraits don't represent
> individual templates in the diagram, but groups of related
> templates. For instance, there are actually templates called
> list_algorithms and assoc_algorithms, among others.
>
> Don't you mean that they represent concepts?
Well that seems to be the Boost term for it. I'd be happier using it
if you could point me to a definition of the term, so I can embed a
hyperlink to the definition. I don't think everyone who uses the
Python or indexing suite documentation will be familiar with it
otherwise (I know I wasn't initially).
>
> "lessthan_comparable" should be "less_than_comparable".
OK.
>
> Why "visitor_helper", and not operator()?
Static function.
>
> There's no description of what visitor_helper is for.
By gum, you're right. Well spotted.
>
> has_copyable_iter can be determined by looking at
> iterator_traits<I>::iterator_category (or better,
> iterator_traversal<I>::type). The user shouldn't supply it.
I guess this needs some explanation in the documentation - there is an
iterator_traits template that deduces many properties automatically
(for instance, has_copyable_iter is indeed determined by iterator
category). I'll add some documentation for the case that client code
wants to use the template as a base class, although there is certainly
no necessity for it to do so.
>
> I don't understand is_reorderable. How is that different from
>
> is_convertible<
> iterator_traits<C::iterator>::iterator_category
> , forward_iterator_tag
> >::value
> && is_non_const_lvalue_iterator<C::iterator>::value
> && is_assignable<C::value_type>::value
>
> ??
>
> I realize we don't have is_assignable, but shouldn't you phrase this
> in terms of something fundamental like value_type assignability? We
> can ask all the other questions (see
> boost/iterator/is_lvalue_iterator.hpp).
The iterator traits template I mentioned above just uses
is_mutable_ref<std_traits::reference> (with an obvious is_mutable_ref
implementation). This actually deduces the wrong answer for std::map
and even std::set with some standard libraries, so I ended up
overriding it manually in set_traits and map_traits.
>
> Once you tell the suite that "has_find" don't you also need to tell
> it how (i.e. via member function or otherwise)?
Yes indeed. The ContainerTraits and Algorithms have to work together
on this - you can't have "has_find" true and then not provide an
implementation of find, count and maybe index in your Algorithms. I
suppose I should really document the exact requirements.
Also, I've recently added two static member functions to the
ValueTraits concept for the less_than and equal_to comparisons. This
is where the automatic shared_ptr handling happens. It's not
documented yet.
>
> The use of static constants in ContainerTraits is unattractive to me,
> because it's not particularly extensible. What about asking the user
> to provide an mpl::set of the property type tags:
>
> typedef mpl::set<mutable,find,insert,push_back> capabilities;
>
> or something?
Looks like a good idea. I'll give this a go.
>
> You actually have the name "value_traits_" in a table. Intended?
> ^
There is a template called value_traits, so the ContainerTraits member
type is called value_traits_ (underscore at end). A bit lame I guess.
>
> Why would you ask the user to supply the Container's size_type and
> iterator instead of deducing them from the container itself?
The client could use one of the existing ContainerTraits templates as
a base class and avoid the need for defining this explicitly (needs
documenting, as noted above). On the other hand, it is also possible
to interface the suite with a container that doesn't provide anything
like an STL-style interface, by completely replacing the traits and
algorithms templates.
>
> I am a bit concerned about how reference
> documentation for various components and headers is grouped all into
> one file, containers.html. I don't believe the precedent set by the
> pickling support in this regard was a good one. We have a
> semi-coherent [;-)] reference manual organization - why should there
> be any supported public headers missing from it? IMO it would be
> better (though not a showstopper) if containers.html were just
> narrative/tutorial, with pointers to the hard-core reference material.
Yes, I think this is inevitable, given the weaknesses you've
identified. I guess I was trying to economize on documentation effort,
but I suppose it needs to be more rigorous.
>
> I'd also like to hear if Joel has any concerns.
Me too. Thanks for your comments so far - I'll work on them as part of
the current updates I'm doing.
--
Raoul Gough.
export LESS='-X'
More information about the Cplusplus-sig
mailing list