[C++-sig] Re: indexing_v2 status update

[David Abrahams]

Raoul Gough <RaoulGough at yahoo.co.uk> writes:

> David Abrahams <dave at boost-consulting.com> writes:
>
>> Raoul Gough <RaoulGough at yahoo.co.uk> writes:
>>
>>> So I have two loosely related questions - what do people thing about
>>> including the current indexing_v2 suite in release 1.31, 
>>
>> 2 questions:
>>
>>   a. Where can I browse the documentation?
>>
>>   b. Where can I review the test code?
>
> Both are available on the indexing_v2 branch in libs/python, so you
> could just do an update -r indexing_v2 in that directory. The docco is
> then in libs/python/doc/v2/containers.html and the test code is in
> files like test_<container>_*.cpp and .py in libs/python/test/
>
> Alternatively, the html-only portion of the documentation is available
> at:
>
> http://cvs.sourceforge.net/viewcvs.py/*checkout*/boost/boost/libs/python/doc/v2/Attic/containers.html?rev=1.1.2

"Safety" is missing from the design goals.  Was that not one of the
goals?

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?

I don't like the use of abbrevs. like "algo_...."

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.


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 "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.

  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?

"lessthan_comparable" should be "less_than_comparable".  

Why "visitor_helper", and not operator()?

There's no description of what visitor_helper is for.

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 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).
  
Once you tell the suite that "has_find" don't you also need to tell
it how (i.e. via member function or otherwise)?

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?

You actually have the name "value_traits_" in a table.  Intended?
                                        ^

Why would you ask the user to supply the Container's size_type and
iterator instead of deducing them from the container itself?

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.

I'd also like to hear if Joel has any concerns.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com