Re: [Boost-users] [range] questions about documentation and usage

Jeffrey Lee Hellrung, Jr. wrote:

On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey wrote:

Neil Groves wrote:

...

On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey wrote:

and perhaps in the library as well. Here are a few constructive suggestions.

a) For each concept in your documentation define a concept checking class. OK you've done this - so far so good.

b) Declare a concept class using the BCCL - you've done this too - so far so good.

c) Declare concept archtypes from reading your documentation. This is described in the Boost Concept library documentation. This archtype class looks like:

template <class T> SinglePassRangeArchType { // examples of "valid expressions" from the documentation go here };

OK First red flag - the "valid expressions" are members of type T so it's not at all clear to me how to fill this in.

If one wants to create archetypes of the various concepts, I would think to read [1] and/or [2] (extending the library to UDTs), both of which are linked from [3], the documentation on the Single Pass Range concept.

This is the red flag I'm talking about. If you have to refer to another page it means that the concept documentation doesn't contain enough information to actually write an archtype - or a class which models the concept. In other words, it's not an actual concept.

Is the documentation under [1] and/or [2] unclear?

It is. Method 1 provides information I would have expected to find in the SinglePassRange concept. I'm assuming that the member functions and types would be members of any class modeling SinglePassRange. I say "assuming becaus it doesn't actually say that. In any case, these should be part of the "valid expressions" section of the SinglePassRange concept. Method 2: I confess the explanation is totally incomprehensible to me. I might be able to parse the example were I to invest the time.

...

On might think to add something like: namespace boost { template <class T> boost::range_iterator<X>::type begin(T &t){ return; } } // namespace boost

But that raises a few design questions of it's own. 1) What should go into X? 2) this will match just about anything - which will likely create other design problems.

Right, but I can only think that one might think to do the above based on an incomplete reading of the documentation.

It may be a fair criticism of the documentation that this pitfall is exists, but it's difficult to use all parts of any library correctly without reading all relevant parts of the associated documentation.

My comments above illustrate that the description/design of the SinglePassRange concept is not comprehensible on its own. Sounds like you agree with that. I content that it should be considered a mistake. The fact that it raises these "other questions" proves that it's a mistake. The whole idea of concepts, functional programming or whatever one want's to call it is to permit the the composition of components which can be demonstrated correct when considered one at at time. If you have to read the whole documentation to understand one one concept - or spelunk through the code - it's a red flag that some things are coupled where they shouldn't be.

In this particular case, the documentation on the various range concepts, like most (all) documentation on concepts that I can think of, only tells you what you can do with a given range concept, e.g., if you were to write a generic algorithm. A separate topic is creating models of the various concepts, and that's what [1] and [2] describe.

As I said before [1] should be part of the concept. I don't know what [2] is.

...

Given that we've decided (perhaps too soon) we want to support boost::range::find(std::vector<int>, 0) we likely want to provide

a.begin() as a valid expression and X::iterator as associated types.

...

That's the option described by [1].

It's in the wrong place.

Of course this will ripple to boost::iterator_range so that we'll need template<class T> iterator_range { ... T begin(); ... };

But we already have that. So far so good.

...

I don't follow. What relevance does iterator_range have here, exactly?

I'm trying to demonstrate what I would expect the natural development of the library would be were it built concurrently with the documentation as I think is the best way to do it. It's meant as a constructive suggestion as to how to go about these things.

====

...

More or less unrelated to my questions regarding the documentation there are a couple of other observations I came upon while looking into this.

a) The example from a another post

class my_class {}; namespace boost { const int * begin(my_class & b); const int * end(my_class & e); } // namespace boost void test3(){ my_class x; boost::begin(x); boost::end(x); BOOST_CONCEPT_ASSERT((boost::SinglePassRangeConcept)); // error // above ASSERT Traps even thoug my class matches SinglePassRange according // to the documentatino

Even given your above overloads of boost::begin/boost::end (which is not the correct way to extend a range interface to my_class; see [1] and [2]),

The SinglePassRange concept page says it should work - if it doesn't it's a mistake in the page regardless of what any other page in the documentation says. If in fact it doesn't work then the page should be changed. That's the essential point of this whole discussion.

Boost.Range doesn't presently deduce the iterator types from the begin/end overloads as, say, an STL implementation would for range-based-for. So there's no way for Boost.Range to know that the associated iterator types of my_class are int*'s. There's really nothing in the SPR concept documentation directly to indicate *how* Boost.Range gets those associated types, so it would seem difficult to assert that my_class models the SPR concept "according to the documentation".

This whole purpose of stating a concept is to decouple the issue of "what it requires" from "how it provides what is required". If the concept makes some assumption about how something like boost::begin(a) is provided by some type attempting to model the concept - then the concept is wrongly formulated. Here is a test. a) Take any concept page b) write some code which declares and/or implements the list of valid expressions. If you feel you need to look elsewhere in the manual, the documentation is incomplete. c) compile it - if it fails, the documentation is wrong. d) run it - if it doesn't produce the expected result then the implementation doesn't match the "semantics" section of the documentation. The SinglePassRange concept fails the above test as the "my_class" demonstrates. Therefore the page is either incomplete and/or wrong.

...

boost::range::find(x, 0); // error // Even the though the assertion above traps, this code compiles }

At first I assumed that the second error was due to failing to include the concept checks in the code for the range::find template. I looked at the code and saw that they were there so I can't explain why the find doesn't trap with the same assert as above.

Sorry, not following. Are you complaining that the boost::find call compiles, and you think it shouldn't (I agree, it probably shouldn't)?

Correct. if for some type T, BOOST_CONCEPT_ASSERT((SinglePassRangeConcept<T>)) traps then any function which requires it's parameters to model the concept SinglePassRange should also fail to compiler. This is because the implementation of f find<T> should include the statement BOOST_CONCEPT_ASSERT((SinglePassRangeConcept<T>)). I see that find<T> does in fact include this. I can't see why it traps when called directly but doesn't when called from within find. There's a mistake in the implementation somewhere - (or maybe a dumb blunder in syntax).

b) I thought that all the stuff was built directly in the boost namespace which I object to. I see it's really built in boost::range (much better) but hoisted up to namespace boost via a wrapper and using declaration. I have to observations about this: 1) I don't think using should every be in any header file.

Why not?

because now a user is importing a whole bunch of names into his lookup set without being informed of it. This can easily create the situation where a working program can all of a sudden fail to compiler - or worse change it's behavior when the following statement is added to the code. #include It's especially bad if the name is something like "find" It can be incredibly time consuming to track down a mistake like this and shapes the programmers confidence in using libraries whose code he as not personally examined in detail.

Seems like a perfectly legitimate way to control ADL. I know this technique is used for the operator| overloads of the Boost.Range adaptors.

Another bad idea - but off topic.

...

2) Any user can hoist those things he wants into the some higher namespace - but he can't go the other way. That is, once he uses your header, he can't undo the "hoisting". If you really want to provide this "help" at least make it optional. I think you could do that by providing an optional header which would hoist all the names.

Just a guess: This hoisting is an implementation detail, only used to control ADL.

obviously to me it's much more than an implementation detail - but I've already made my case.

Simple examples for the Boost.Range functions should be easy to come by and, agreed, would enhance the documentation.

I started off with some questions about the documentation and things (as they sometimes do) turned into something more than that. I've offered suggestions which I think have been constructive. I hope someone agrees with them. I realize that implementing them would be a lot of work - beyond normal maintainence so I don't realistically expect them to be incorporated. Maybe they show up somewhere - perhaps in the working group looking into incorporating range into the C++ standard library. Robert Ramey