[range] questions about documentation and usage
I want to use Boost.Range in a bigger way. But I have a few problems with it. I think most of them are just issues of understanding and the writting of the documentation a) I see examples like std::vector<T> v; boost::range::find(v, 0); find(SinglePassRange& rng, Value val); BUT when looking at the documentation at http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... I see only two valid expressions boost::begin(a) boost::end(a) where in this case a would be v which is of type std::vector<int>. BUT boost:begin(v) is NOT a valid expression for the above. Spelunking into the range header code reveals the following: template< class T > inline BOOST_DEDUCED_TYPENAME range_iterator<T>::type begin( T& r ) { #if !BOOST_WORKAROUND(__BORLANDC__, BOOST_TESTED_AT(0x564)) && \ !BOOST_WORKAROUND(__GNUC__, < 3) \ /**/ using namespace range_detail; #endif return range_begin( r ); } and template< typename C > inline BOOST_DEDUCED_TYPENAME range_iterator<C>::type range_begin( C& c ) { // // If you get a compile-error here, it is most likely because // you have not implemented range_begin() properly in // the namespace of C // return c.begin(); } This effectively means that the "Valid Expressions" for SinglePassRange part of the documenation should read: a.begin() a.end() Doesn't it. I would be interested to know if I got this wrong. b) I see the template iterator_range<ForwardTraversalIterator> - which seem to be to in an instance of what the ForwardRangeConcept should be. To me, the range documentation is out of whack. I can figure it out - but it's misleading - atleast to me. c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict. Robert Ramey
a) I see examples like
std::vector<T> v; boost::range::find(v, 0);
find(SinglePassRange& rng, Value val);
BUT when looking at the documentation at http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... I see only two valid expressions
boost::begin(a) boost::end(a)
where in this case a would be v which is of type std::vector<int>. BUT
boost:begin(v) is NOT a valid expression for the above.
Spelunking into the range header code reveals the following:
template< class T > inline BOOST_DEDUCED_TYPENAME range_iterator<T>::type begin( T& r ) { #if !BOOST_WORKAROUND(__BORLANDC__, BOOST_TESTED_AT(0x564)) && \ !BOOST_WORKAROUND(__GNUC__, < 3) \ /**/ using namespace range_detail; #endif return range_begin( r ); }
and
template< typename C > inline BOOST_DEDUCED_TYPENAME range_iterator<C>::type range_begin( C& c ) { // // If you get a compile-error here, it is most likely because // you have not implemented range_begin() properly in // the namespace of C // return c.begin(); }
This effectively means that the "Valid Expressions" for SinglePassRange part of the documenation should read:
a.begin() a.end()
Doesn't it. I would be interested to know if I got this wrong.
Nope. Suppose I have a third-party library with a class named Vector which has methods named Begin() and End() rather than begin() and end(). I can't change Vector, but I'd still like to use it as a SinglePassRange. Boost.Range allows me to do that, by overloading range_begin() and range_end() in the namespace of Vector (so that boost::begin() and boost::end() finds them by ADL) as described in [1]. namespace namespace_of_Vector { Vector::Iterator range_begin(Vector& v) { return v.Begin(); } Vector::Iterator range_end(Vector& v) { return v.End(); } // overloads for const Vector& } Now if v is of type Vector, boost::begin(v) and boost::end(v) will be valid expressions, but v.begin() and v.end() will not. So this extra layer allows us to adapt third-party types that we have no control over to model Boost.Range concepts such as SinglePassRange. For this design to work, consumers of the SinglePassRange concept must obey the interface of the concept and only use boost::begin(), not the begin() member function, on models of SinglePassRange.
b) I see the template iterator_range<ForwardTraversalIterator> -
which seem to be to in an instance of what the ForwardRangeConcept should be.
I'm not sure I understand what the question/problem here is. Could you elaborate?
c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict.
Agreed. Regards, Nate [1] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext...
Nathan Ridge wrote:
This effectively means that the "Valid Expressions" for SinglePassRange part of the documenation should read:
a.begin() a.end()
Doesn't it. I would be interested to know if I got this wrong.
Nope.
Suppose I have a third-party library with a class named Vector which has methods named Begin() and End() rather than begin() and end(). I can't change Vector, but I'd still like to use it as a SinglePassRange. Boost.Range allows me to do that, by overloading range_begin() and range_end() in the namespace of Vector (so that boost::begin() and boost::end() finds them by ADL) as described in [1].
namespace namespace_of_Vector { Vector::Iterator range_begin(Vector& v) { return v.Begin(); } Vector::Iterator range_end(Vector& v) { return v.End(); } // overloads for const Vector& }
Now if v is of type Vector, boost::begin(v) and boost::end(v) will be valid expressions, but v.begin() and v.end() will not.
Of course you can do this. But looking at the documenation you wouldn't expect copy(std::vector<int>, 0) to compile without error. In fact, looking at the documentation you would conclude that you have to do this - when in fact you don't. The concept classes of Boost.Range test success for something like vector<int> which suggests that a container is a range. Although it's technically correct within the confines of boost.range - it's extremely unintuitive and confusing. For example, it's not at all obvious int x[10] boost::find(x, 0); should compile or not.
So this extra layer allows us to adapt third-party types that we have no control over to model Boost.Range concepts such as SinglePassRange.
I can see that by looking at the implementation but it's not clear from looking at the documentation. The fact that there is an extra layer is sort of hidden from the person using the library.
b) I see the template iterator_range<ForwardTraversalIterator> -
which seem to be to in an instance of what the ForwardRangeConcept should be.
I'm not sure I understand what the question/problem here is. Could you elaborate?
It just illustrates the source of the confusion. iterator_range<ForwardTraversalIterator> is a direct implementation of ForwardTransversalRange> and sort of what I exect to see. It's not clear how a std::vector gets "transformed" - (Please don't explain it to me, I've seen how it works). This intermediate transformation goes directly from vector->ForwardTransversalRange without passing through and interator_range. It's all very confusing, an unintuitive which makes it much harder to figure out how to use it than it should be. d) I forgot to add this. The exposition of each function, template etc, could benefit by including a small example. This is common practice among other similar libraries. It is generally very helpful. Robert Ramey
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
This effectively means that the "Valid Expressions" for SinglePassRange part of the documenation should read:
a.begin() a.end()
Doesn't it. I would be interested to know if I got this wrong.
Nope.
Suppose I have a third-party library with a class named Vector which has methods named Begin() and End() rather than begin() and end(). I can't change Vector, but I'd still like to use it as a SinglePassRange. Boost.Range allows me to do that, by overloading range_begin() and range_end() in the namespace of Vector (so that boost::begin() and boost::end() finds them by ADL) as described in [1].
namespace namespace_of_Vector { Vector::Iterator range_begin(Vector& v) { return v.Begin(); }
Vector::Iterator range_end(Vector& v) { return v.End(); }
// overloads for const Vector&
}
Now if v is of type Vector, boost::begin(v) and boost::end(v)
will be
valid expressions, but v.begin() and v.end() will not.
Of course you can do this. But looking at the documenation you wouldn't expect copy(std::vector<int>, 0) to compile without error. In fact, looking at the documentation you would conclude that you have to do this - when in fact you don't.
The concept classes of Boost.Range test success for something like vector<int> which suggests that a container is a range. Although it's technically correct within
the confines
of boost.range - it's extremely unintuitive and confusing.
For example, it's not at all obvious
int x[10] boost::find(x, 0); should compile or not.
So this extra layer allows us to adapt third-party types that we have no control over to model Boost.Range concepts such as SinglePassRange.
I can see that by looking at the implementation but it's not clear from looking at the documentation. The fact that there is an extra layer is sort of hidden from the person using the
library. This is documented both in the Introduction page of the documentation: "This library therefore provides the means to adapt standard-like containers, null terminated strings, std::pairs of iterators, and raw arrays (and more), such that the same generic code can work with them all. The basic idea is to add another layer of indirection using metafunctions and free-standing functions so syntactic and/or semantic differences can be removed." [1] and in the page that documents Method 1 of making types model ranges: "The primary templates in this library are implemented such that standard containers will work automatically and so will boost::array. Below is given an overview of which member functions and member types a class must specify to be useable as a certain Range concept." [2] If you believe there are additional places in the documentation where this should be mentioned, I am sure the Boost.Range maintainers will be happy to accept a patch to the documentation.
b) I see the template iterator_range<ForwardTraversalIterator> -
which seem to be to in an instance of what the ForwardRangeConcept should be.
I'm not sure I understand what the question/problem here is. Could you elaborate?
It just illustrates the source of the confusion. iterator_range<ForwardTraversalIterator> is a direct implementation of ForwardTransversalRange> and sort of what I exect to see. It's not clear how a std::vector gets "transformed" - (Please don't explain it to me, I've seen how it works). This intermediate
transformation>
goes directly from vector->ForwardTransversalRange without passing through and interator_range.
It's all very confusing, an unintuitive which makes it much harder to figure out how to use it than it should be.
d) I forgot to add this. The exposition of each function, template etc, could benefit by including a small example. This is common
I don't see what iterator_range has to do with vector or with "transforming" anything. Indeed, the documentation for iterator_range doesn't even mention the words "vector" or "transform". *You* are confusing yourself by trying to draw a connection where there isn't one. iterator_range is simply a utility for when you have a pair of iterators that delimit a range and you want to package them up as a range, and this pair of iterators is not a begin/end pair for an object that is already a range. For example, if you had a vector with 5 elements: vector<int> v = {1, 2, 3, 4, 5}; and you wanted a range representing the last 3 elements, a convenient way to write down such a range would be: make_iterator_range(v.begin() + 2, v.end()) practice
among other similar libraries. It is generally very helpful.
Agreed. Once again, I'm sure the maintainers would welcome patches. Regards, Nate [1] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/introduction.... [2] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext...
Nathan Ridge wrote:
This is documented both in the Introduction page of the documentation:
"This library therefore provides the means to adapt standard-like containers, null terminated strings, std::pairs of iterators, and raw arrays (and more), such that the same generic code can work with them all. The basic idea is to add another layer of indirection using metafunctions and free-standing functions so syntactic and/or semantic differences can be removed." [1]
and in the page that documents Method 1 of making types model ranges:
"The primary templates in this library are implemented such that standard containers will work automatically and so will boost::array. Below is given an overview of which member functions and member types a class must specify to be useable as a certain Range concept." [2]
This is all beside the point. My concern is very simple: The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange. This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner. Looking at this some more, It does raise questions to me about the design of the library and extenstion mechanism. But it would be pre-mature to consider them now..
d) I forgot to add this. The exposition of each function, template etc, could benefit by including a small example. This is common practice among other similar libraries. It is generally very helpful.
Agreed. Once again, I'm sure the maintainers would welcome patches.
lol - so by making this observation it's incumbant on me to to create and test 100 examples of each feature of the library? Robert Ramey
Le 02/11/12 18:20, Robert Ramey a écrit :
Nathan Ridge wrote:
This is documented both in the Introduction page of the documentation:
"This library therefore provides the means to adapt standard-like containers, null terminated strings, std::pairs of iterators, and raw arrays (and more), such that the same generic code can work with them all. The basic idea is to add another layer of indirection using metafunctions and free-standing functions so syntactic and/or semantic differences can be removed." [1]
and in the page that documents Method 1 of making types model ranges:
"The primary templates in this library are implemented such that standard containers will work automatically and so will boost::array. Below is given an overview of which member functions and member types a class must specify to be useable as a certain Range concept." [2] This is all beside the point.
My concern is very simple:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange.
This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner. I disagree here. The requirement is on the expression boost::begin(a) not on how this expression can be provided. I suspect that it could help if the documentation has a section "Models of" for each Concept" which includes the models the library provided already. Looking at this some more, It does raise questions to me about the design of the library and extenstion mechanism. But it would be pre-mature to consider them now..
d) I forgot to add this. The exposition of each function, template etc, could benefit by including a small example. This is common practice among other similar libraries. It is generally very helpful. Agreed. Once again, I'm sure the maintainers would welcome patches. lol - so by making this observation it's incumbant on me to to create and test 100 examples of each feature of the library?
I don't think Nathan was suggesting that you provide these patches. Best, Vicente
Vicente J. Botet Escriba wrote:
My concern is very simple:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange. This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner.
I disagree here. The requirement is on the expression boost::begin(a) not on how this expression can be provided.
Ahhh - this is the crux of my complaint. I would be curious to hear the opinions of some of our well know "concept gurus" on this page and the question I've raised about it. Robert Ramey
On Fri, Nov 2, 2012 at 11:20 AM, Robert Ramey <ramey@rrsd.com> wrote:
Nathan Ridge wrote:
"This library therefore provides the means to adapt standard-like containers, null terminated strings, std::pairs of iterators, and raw arrays (and more), such that the same generic code can work with them all. The basic idea is to add another layer of indirection using metafunctions and free-standing functions so syntactic and/or semantic differences can be removed."
This is all beside the point.
My concern is very simple:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange.
On the page you reference, it describes that a particular expression must be valid for types modeling the SinglePassRange concept. It doesn't claim that you must implement it. I admit that it's a fine distinction. I thought that the "See Also" section on that same page was helpful, however -- the link titled "Extending the library for UDTs" seems to provide the information you're looking for.
This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner. I'd be interested to hear opinions on this also -- it made sense to me, but I didn't start adapting my own ranges until I was fairly comfortable with using the library with the standard containers.
Nate
Nathan Crookston wrote:
On Fri, Nov 2, 2012 at 11:20 AM, Robert Ramey <ramey@rrsd.com> wrote:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange.
On the page you reference, it describes that a particular expression must be valid for types modeling the SinglePassRange concept. It doesn't claim that you must implement it.
I admit that it's a fine distinction. I thought that the "See Also" section on that same page was helpful, however -- the link titled "Extending the library for UDTs" seems to provide the information you're looking for.
OK - I see you're point here. And I'll even agree with it. The requirement that boost::begin(a) can be specified as part of the concept of the type of a. But from a practical standpoint this causes a lot of problems. I have a type std::vector<int>. Apparently the authors of Boost.Range have inmplemented boost::begin<std::vector<int> > somewhere. But how can I possible know this? And if this is defined - what header do I have to include to get it? or what. Basically, whether or not boost::range<T> is going to function is dependent upon information not readily available to the reader - hence the need to spelunk through the code to find stuff and figure it out. A plain reading of the documentation will lead one to expect that the following should compile: class my_class {}; namespace boost { int * begin(my_class & b); int * end(my_class & e); } // namespace boost void test3(){ my_class x; boost::begin(x); boost::end(x); BOOST_CONCEPT_ASSERT((boost::SinglePassRangeConcept<my_class>)); // error (1) boost::find(x, 0); // error (2) } when in fact it does not. There is no way that both the documentation and the implementation of constant checking can be correct. This is illustrated by error(1). error(2) doesn't not fail within a concept definition. In other words, the library fails to use it's own concepts to check the types of parameters passed to its own templates. The chief benefit is usage of concepts is to help users passing the wrong types to template functions and classes. This is apparently not done in this library. Robert Ramey
My concern is very simple:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange.
This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner.
It is common practice in Boost libraries for a concept to be associated with two sets of interfaces: an interface that the consumer of the concept uses to work with models of the concept, and the interface that the provider of the concept (that is, the programmer who is writing a type to model the concept or adapting a third-party type to model the concept) uses to make his type model the concept. It is of course possible for the two interfaces to be the same, but in some cases this is problematic. If this single interface uses member functions, then third-party types which do not have those member functions cannot be adapted to model the concept. An interface that uses non-member functions avoids this problem, but sometimes there are pre-existing conventions (e.g. container classes having begin() and end() member functions) that that are already in wide use and it would be annoying if every class that already uses those conventions would now have to write member function wrappers to model the interface. Having two different interfaces solves this problem: consumers of the concept use the interface meant for them, and providers of models of the concepts use mechanisms meant for them to model the concept. The library defining the concept provides the glue between the two. This is the case with Boost.Range: the interface for consumers is boost::begin() and boost::end(). Model providers have a choice of either using member begin() and end() functions, or non-member range_begin() and range_end() functions found using ADL. The C++0x concepts proposal was going to alleviate this need for two interfaces by providing a language construct named "concept_map" which allowed writing the glue code that allows adapting a type with a different interface to model a concept with the required interface. Sadly, that proposal did not pass, so in Boost today we have to keep using this work-around with two interfaces.
d) I forgot to add this. The exposition of each function, template etc, could benefit by including a small example. This is common practice among other similar libraries. It is generally very helpful.
Agreed. Once again, I'm sure the maintainers would welcome patches.
lol - so by making this observation it's incumbant on me to to create and test 100 examples of each feature of the library?
Not at all. I'm just pointing out that *someone* has to do it, and in the world of free/open-source software, often the person with sufficient motivation to implement a feature (or write documentation snippet etc.) is the one who asks for it. Regards, Nate
Nathan Ridge wrote:
My concern is very simple:
The page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... is crystal clear. It says that any SinglePassRange must implement the expression boost::begin(a) where a is a model of a SinglePassRange.
This is not actually a requirement on any model of a SinglePassRange but rather a requirement on boost::begin. But boost::begin is not supplied by the user! This documentation doesn't tell me what operation the type argument has to support in order to be used as argument to a range. It's just non-sensical and therefore confusing. There is nothing on this page which would indicate to me that something like <vector> would satisify the concept. The page is does not document a "concept" in the C++ manner.
It is common practice in Boost libraries for a concept to be associated with two sets of interfaces: ...
I confess I didn't understand any of this. It doesn't seem to address my (simple?) observation: There is no way to read the documentation of SinglePassRange concept and know whether some specific type will work with a range algorithm. Robert Ramey
It is common practice in Boost libraries for a concept to be associated with two sets of interfaces:
...
I confess I didn't understand any of this.
It doesn't seem to address my (simple?) observation:
There is no way to read the documentation of SinglePassRange concept and know whether some specific type will work with a range algorithm.
Frankly, I re-read this documentation and its completely obvious whether a type will work with a range algorithm to me. I then got a small sample of developers I know to read it, and none had an issue. If the type is a proper model of the Concept it will work. I would hazard a guess that you are finding it hard to determine if a type will work with a range algorithm because you haven't yet got a clear understanding of Concepts and what it means for a type to model a Concept. I recommend studying Concepts they formalise generic programming in a manner that makes it easier to reason about complex large-scale generic designs. The information is correctly stated in the documentation AFAICT and with the appropriate background knowledge I believe it is clear and unambiguous. The algorithm documentation clearly defines the minimum Range Concept that the template types must model. The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this. Boost.Range has had these Concepts and fundamentally the same documentation about the fundamental aspects of Ranges for many versions. It does not appear to have been a barrier to entry, or caused much confusion among the general Boost user population. Boost.Range is heavily designed around Concepts to great advantage. It is therefore inherently necessary to comprehend Concepts to understand Boost.Range. The only thing I can think of doing to the documentation to address this is to perhaps make more clear the importance of understanding Concepts. Robert Ramey
HTH, Neil Groves
Neil Groves wrote:
Frankly, I re-read this documentation and its completely obvious whether a type will work with a range algorithm to me. I then got a small sample of developers I know to read it, and none had an issue. If the type is a proper model of the Concept it will work.
I'm not disputing that I'm in the minority here.
I would hazard a guess that you are finding it hard to determine if a type will work with a range algorithm because you haven't yet got a clear understanding of Concepts and what it means for a type to model a Concept.
I would disagree with this as well - not that it's relevant.
I recommend studying Concepts they formalise generic programming in a manner that makes it easier to reason about complex large-scale generic designs.
Thanks for this advice.
The information is correctly stated in the documentation AFAICT and with the appropriate background knowledge I believe it is clear and unambiguous.
Of course this is the crux of our disagreement.
The algorithm documentation clearly defines the minimum Range Concept that the template types must model.
I would disagree that it's clear.
The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this.
I would suggest that each template class/function have a small example. The fusion library in particular does this and I've found it very helpful. I also suspect it helps detect errors in the documentation.
Boost.Range has had these Concepts and fundamentally the same documentation about the fundamental aspects of Ranges for many versions. It does not appear to have been a barrier to entry, or caused much confusion among the general Boost user population.
I'm suggesting improvements for those of that aren't so smart.
Boost.Range is heavily designed around Concepts to great advantage.
I'm suggesting that Boost.Range has this wrong.
It is therefore inherently necessary to comprehend Concepts to understand Boost.Range.
I don't believe that it is necessary for users to comprehend Concepts to use a library. Definitely helpful - but not necessary. I personally find it helpful - that is why I carefully read the documentation concerning them - and found the documentation misleading and in need of improvement.
The only thing I can think of doing to the documentation to address this is to perhaps make more clear the importance of understanding Concepts.
To whom? Robert Ramey
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
Frankly, I re-read this documentation and its completely obvious whether a type will work with a range algorithm to me. I then got a small sample of developers I know to read it, and none had an issue. If the type is a proper model of the Concept it will work.
I'm not disputing that I'm in the minority here.
My comment was partly to invite others that have struggled with the same area to come forward and tell me I'm wrong. They might yet do so.
I would hazard a guess that you are finding it hard to determine if a type will work with a range algorithm because you haven't yet got a clear understanding of Concepts and what it means for a type to model a Concept.
I would disagree with this as well - not that it's relevant.
I put this forward as a hypothesis based on the previous email where you suggested that it wasn't obvious which types worked with range algorithms. The types need to be a model of the appropriate concepts. It's as simple as that from my perspective. Therefore I can only imagine someone that finds it hard to comprehend which types work with range algorithms having difficulty with the language used to expression the requirements.
I recommend studying Concepts they formalise generic programming in a manner that makes it easier to reason about complex large-scale generic designs.
Thanks for this advice.
The information is correctly stated in the documentation AFAICT and with the appropriate background knowledge I believe it is clear and unambiguous.
Of course this is the crux of our disagreement.
Please demonstrate what you would consider an improvement. I think many of us have tried to obtain something from you to help us. I appreciate that negative criticism is to some extent useful, but you aren't giving me clear enough suggestions for positive corrective action. I can't do anything for you until I understand potential solutions. I accept that my failure to understand your suggestions may simply be that I'm doing a poor job of understanding your suggestions.
The algorithm documentation clearly defines the minimum Range Concept that the template types must model.
I would disagree that it's clear.
I'm comfortable to disagree with you on this. Boost.Range being largely Concept-based is a major reason the library is so general and interoperable with existing standard Containers and containers from other libraries. This evidently contributes to usefulness. Perhaps being an early adopter of Concept-based approaches comes with some learning-curve disadvantage. I believe the learning-curve is well worth the effort for using this library, for using other libraries, and for writing your own libraries. Of course, that's just my opinion.
The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this.
I would suggest that each template class/function have a small example. The fusion library in particular does this and I've found it very helpful. I also suspect it helps detect errors in the documentation.
That's a good suggestion. I'm happy to try and find time to do this. I'm also happy to accept your patches to the documentation.
Boost.Range has had these Concepts and fundamentally the same documentation about the fundamental aspects of Ranges for many versions. It does not appear to have been a barrier to entry, or caused much confusion among the general Boost user population.
I'm suggesting improvements for those of that aren't so smart.
I was not suggesting you were less smart. I think it is fair to point out that your criticism of the documentation is the first of this nature despite the library being adopted by many. It doesn't make you wrong, but it is evidence that the documentation is not lacking to the point of making the library extremely difficult to use. It might be that you have less experience with Concepts; that is an entirely different suggestion to suggesting that you aren't as smart. I assume there are a very large number of subjects where you would understand fundamental material that I am absolutely clueless about.
Boost.Range is heavily designed around Concepts to great advantage.
I'm suggesting that Boost.Range has this wrong.
Yes you are, but frustratingly again without any positive suggestive action which is annoying and unhelpful. I'm trying very hard to comprehend your issues and I've asked previously for you to put forward something constructive. I think this would help me comprehend your points. It's perhaps harder for me to see deficiencies in the documentation that any random person since I've been so immersed in the library for so long. I'm perhaps simply assuming something, but I require more constructive feedback to understand the issue you having and the solution you are proposing. Where you have provided positive suggestions such as increased examples I have immediately been able to understand and put this onto my todo list.
It is therefore inherently necessary to comprehend Concepts to understand Boost.Range.
I don't believe that it is necessary for users to comprehend Concepts to use a library. Definitely helpful - but not necessary. I personally find it helpful - that is why I carefully read the documentation concerning them - and found the documentation misleading and in need of improvement.
The only thing I can think of doing to the documentation to address this is to perhaps make more clear the importance of understanding Concepts.
To whom?
To the readers. Many documents / papers I read have a better enumeration of the pre-requisite subject matter that is expected to be understood as foundational material. I was trying to find actions to make the documentation better. I wondered if this might help address your concerns. However your response shows that I have misunderstood the source of your confusion anyhow, so this isn't a valid solution.
Robert Ramey
Regards, Neil Groves
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
Please demonstrate what you would consider an improvement. I think many of us have tried to obtain something from you to help us. I appreciate that negative criticism is to some extent useful, but you aren't giving me clear enough suggestions for positive corrective action.
I pointed out things that I thought were wrong and asked to what I was missing. The feedback that I got back was that no one else thinks anything is wrong and don't see my point. From my stand point, I'm just pointing out mistakes.I didn't think of it as negative criticism. The feedback I've gotten has convinced me that I didn't get any thing wrong and that in fact my observations point to real mistakes in the documentation 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. 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. 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 express and X::iterator as associated types. 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. d) Each function template documentation page should describe the concepts of it's template parameters. the range find function described here: http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/alg... refers to a type "range_return_value" which I can't find anywhere. I'm guessing that the type Value can't be any type but is likely restricted to some type related to SinglePassRangeConcept? I don't know what it is. A function template page should look more like this example: http://rrsd.com/blincubator.com/function/ Note that this includes a small example which can be compiled. Note it should contain the header file to be included. e) A type or class template documentation page should have a little bit more information. Take your example: http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/uti... which isn't too bad. But it would be helpful if it included the following: Template parameter. I can guess that the "ForwardTransversalIterator" parameter should model the ForwardTransversalIterator concept of the iterator library. But it should be stated explicitly. Model of: SinglePassRange concept Valid Expressions (AKA member functions) should state the concepts (type requirements on member function template parameters. For example, if I see: template< class ForwardRange > iterator_range& operator=( ForwardRange& r ); It's not clear what type I can plug into ForwardRange. Can I use any type which models SinglePassRange like this: std::vector<int> x; boost::range::iterator_range<std::vector::iterator> v(x); or do have to use another iterator range as an argument? I can't tell from reading the documentation. Here is a model for a class template page http://rrsd.com/blincubator.com/type/ ==== 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<my_class>)); // error // above ASSERT Traps even thoug my class matches SinglePassRange according // to the documentatino 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. 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. 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.
The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this.
The examples I'm refering to are not full blown tutorial type examples (which are helpful but a lot of work to make). But rather 5-15 lines mini-programs written at the time the documentation is written. They are not hard to write when the code is being developed and very helpful to users of the documentation.
I'm suggesting that Boost.Range has this wrong.
Yes you are, but frustratingly again without any positive suggestive action which is annoying and unhelpful.
I hope this is helpful. For what it's worth, I don't mean to pick on anyone personally. I am sympathetic - I doubt anyone ever received more heat than I have for making the serialization library. My advice above is fruit of bitter personal experience - which I'm hoping to help other people avoid. To this end, I've been working on my latest project: www.blincubator.com I would hope anyone who has followed this discussion will find this interesting. It still has a few loose ends, but then I haven't received any feedback on it. Robert Ramey
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
Please demonstrate what you would consider an improvement. I think many of us have tried to obtain something from you to help us. I appreciate that negative criticism is to some extent useful, but you aren't giving me clear enough suggestions for positive corrective action.
I pointed out things that I thought were wrong and asked to what I was missing. The feedback that I got back was that no one else thinks anything is wrong and don't see my point. From my stand point, I'm just pointing out mistakes.I didn't think of it as negative criticism. The feedback I've gotten has convinced me that I didn't get any thing wrong
I'm not sure how you came to that conclusion...
and that in fact my observations point to real mistakes in the documentation
Deficiencies, yes, but I don't think you've pointed out any mistakes.
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. Is the documentation under [1] and/or [2] unclear? 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. 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. 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 express and X::iterator as associated types.
That's the option described by [1].
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? d) Each function template documentation page should describe the
concepts of it's template parameters.
the range find function described here:
http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/alg... refers to a type "range_return_value" which I can't find anywhere.
Valid criticism. Go up a couple levels in the documentation and you might find [4].
I'm guessing that the type Value can't be any type but is likely restricted to some type related to SinglePassRangeConcept? I don't know what it is.
A reference to existing documentation on std::find would be the easiest/cheapest way to address such documentation deficiencies; AFAIK, boost::find is implemented directly in terms of std::find. A function template page should look more like this example:
http://rrsd.com/blincubator.com/function/
Note that this includes a small example which can be compiled. Note it should contain the header file to be included.
That's a good model.
e) A type or class template documentation page should have a little bit more information. Take your example:
http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/uti...
which isn't too bad. But it would be helpful if it included the following:
Template parameter. I can guess that the "ForwardTransversalIterator" parameter should model the ForwardTransversalIterator concept of the iterator library. But it should be stated explicitly.
Agreed. And, actually, the documentation states that the template parameter actually need not satisfy the Forward Traversal concept, but then only a subset of the documented interface is available. What that subset is, though, is not explicitly documented, I think :/
Model of: SinglePassRange concept
Valid Expressions (AKA member functions) should state the concepts (type requirements on member function template parameters. For example, if I see:
template< class ForwardRange > iterator_range& operator=( ForwardRange& r );
It's not clear what type I can plug into ForwardRange. Can I use any type which models SinglePassRange like this:
std::vector<int> x; boost::range::iterator_range<std::vector::iterator> v(x);
or do have to use another iterator range as an argument?
I can't tell from reading the documentation.
The documentation on boost::iterator_range is, indeed, incomplete. I think it has been around for quite some time and hasn't seen much love for a while :/
Here is a model for a class template page
http://rrsd.com/blincubator.com/type/
==== 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<my_class>)); // 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]), 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". 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)?
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? Seems like a perfectly legitimate way to control ADL. I know this technique is used for the operator| overloads of the Boost.Range adaptors. If this is used also for boost::find (defined as boost::range::find and brought into the boost namespace by a using declaration), it's likely to avoid boost::find being found by ADL. Whether boost::find should really be boost::range::find is another issue.
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.
The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this.
The examples I'm refering to are not full blown tutorial type examples (which are helpful but a lot of work to make). But rather 5-15 lines mini-programs written at the time the documentation is written. They are not hard to write when the code is being developed and very helpful to users of the documentation.
Simple examples for the Boost.Range functions should be easy to come by and, agreed, would enhance the documentation. - Jeff [1] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... [2] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... [3] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... [4] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/alg...
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> 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<my_class>)); // 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 <boost/range/find.hpp> 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
On Sun, Nov 4, 2012 at 10:10 AM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
and perhaps in the library as well. Here are a few constructive suggestions.
[...]
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.
IMHO, a strict reading of [5] does not necessitate that a concept provide ways to allow the user to create models (e.g., write archetypes). Indeed, I can imagine situations where all models of a concept are provided entirely by a library (see Boost.Parameter's concepts). I could be wrong on the intent of concepts but this seems to jive with the concept documentation of Boost.Range, Boost.Fusion, and Boost.MPL. They all have extension mechanisms you need to go through to adapt UDTs to the concepts documented by the respective library.
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.
Then...why are you assuming that? In any case, these should be part of the
"valid expressions" section of the SinglePassRange concept.
*That* would be a mistake; begin/end member functions and an iterator typedef are just sufficient for a type to model SPR, but they aren't necessary. E.g., std::pair< int*, int* > is a valid range. 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.
"totally incomprehensible" is unnecessary hyperbole, and is not helpful regarding improving the documentation of this section. There's only a couple hundred words there. Come back with something concrete.
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.
For the record, I do not. 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.
No spelunking through code is necessary, you did that on your own. If you want to understand what you can do and how to use a concept, the concept documentation is sufficient (IMO). If you want to create an archetype or adapt an existing UDT to a concept, the extenstion mechanism documentation is sufficient (IMO). This is also how Boost.Fusion and Boost.MPL (at least) are structured, AFAIK. [...snip more discussion on what belongs in the concept documentation...]
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.
I'm sorry, I'm still not following. Can you be more concrete? I don't know what you mean by building a library "concurrently with the documentation", and I'm not sure what your suggestion about "how to go about these things" actually is. I snip the immediately following discussion, as it basically boils down to you asserting that the documentation of a concept ought to allow one to directly write a model or archetype of the concept. [...]
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).
Hmmm, works for me, as in, it fails to compile. -------- #include <boost/concept/assert.hpp> #include <boost/range/algorithm/find.hpp> #include <boost/range/concepts.hpp> struct X { }; namespace boost { int const * begin(X&); int const * end(X&); } // namespace boost int main(int argc, char* argv[]) { X x; //boost::find(x, 0); // compiler error boost::range::find(x, 0); // compiler error return 0; } -------- Same error comes up as in a BOOST_CONCEPT_ASSERT, pointing to a line referencing range_const_iterator and range_mutable_iterator, suggesting that Boost.Range cannot associate an iterator type with X.
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.
Really, a "whole bunch"? It has a single "using range::find" that pulls boost::range::find into the boost namespace. How can this be any worse than defining boost::find directly in the boost namespace? It's actually way better this way as now boost::find will be less likely found via ADL. 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 <boost/range/find.hpp>
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.
You're conflating 2 "issues": (a) it's boost::find and not boost::range::find; and (b) boost/range/algorithm/find.hpp has a using declaration. (a) is legitimately debatable (I don't have strong opinions, other than there is a std::find so I can see the rationale for boost::find). There really is no issue (that I can see) with (b) independent of (a). [...]
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.
Confirmed now that I've looked at the code.
obviously to me it's much more than an implementation detail - but I've already made my case.
AFAIK, boost::find is documented, and boost::range::find is not. It's an implementation detail that, as I've said above, is no worse than defining boost::find directly in the boost namespace.
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.
Yes, you have brought up some deficiencies in the documentation. But I don't agree with many of your alleged "mistakes". Someone with authority on concepts may agree with you, but AFAICT, the Boost.Range documentation on concepts is complete and on par with other Boost library documentation on concepts. 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.
- Jeff [5] http://www.boost.org/community/generic_programming.html#concept
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 10:10 AM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
IMHO, a strict reading of [5] does not necessitate that a concept provide ways to allow the user to create models (e.g., write archetypes).
I believe a concept definition describes those valid expressions which necessary and SUFFICIENT to define the concept. That is, I think we'll agree that any type which models the concept should support all the valid expressions list. I belive also that any type which supports all the valid expressions listed is necessarily a model of the concept. Again, would appreciate input form a concept guru here.
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.
Then...why are you assuming that?
Because the documentation says they are member functions but it doesn't say of what type. One has no choice to guess or assume.
In any case, these should be part of the "valid expressions" section of the SinglePassRange concept.
*That* would be a mistake; begin/end member functions and an iterator typedef are just sufficient for a type to model SPR, but they aren't necessary. E.g., std::pair< int*, int* > is a valid range.
SinglePassRangeConcept requires the valid expression boost::begin(a) if a instance the type which models the concept. I've been directed to "method 1" of the documentation. There one is directed to implement a member begin() of some unspecified type. I'm sorry, if I want to make my own range, I just don't know what to do to from reading this.
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.
My comments above illustrate that the description/design of the SinglePassRange concept is not comprehensible on its own. Sounds like you agree with that.
For the record, I do not.
I'm sorry I've failed to convince you, my attempt has been sincere though.
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.
No spelunking through code is necessary, you did that on your own.
Only because I had to.
If you want to understand what you can do and how to use a concept, the concept documentation is sufficient (IMO).
Of course that's where we can't agree.
If you want to create an archetype or adapt an existing UDT to a concept, the extenstion mechanism documentation is sufficient (IMO).
To my mind, the way the "extension mechanism" is formulated is a symptom of the problem. Much of what's in there - should be part of the the SinglePassRange concept if the concept were correctly formulated.
This is also how Boost.Fusion and Boost.MPL (at least) are structured, AFAIK.
I don't think that's accurate. If you want to make your own model of a concept, you just make sure your types implement all the valid expressions.
[...snip more discussion on what belongs in the concept documentation...]
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.
I'm sorry, I'm still not following. Can you be more concrete? I don't know what you mean by building a library "concurrently with the documentation", and I'm not sure what your suggestion about "how to go about these things" actually is.
rsd.com/blincubator.com/advice/
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).
Hmmm, works for me, as in, it fails to compile.
-------- #include <boost/concept/assert.hpp> #include <boost/range/algorithm/find.hpp> #include <boost/range/concepts.hpp>
struct X { };
namespace boost { int const * begin(X&); int const * end(X&); } // namespace boost
int main(int argc, char* argv[]) { X x; //boost::find(x, 0); // compiler error boost::range::find(x, 0); // compiler error return 0; } --------
Same error comes up as in a BOOST_CONCEPT_ASSERT, pointing to a line referencing range_const_iterator and range_mutable_iterator, suggesting that Boost.Range cannot associate an iterator type with X.
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.
Really, a "whole bunch"? It has a single "using range::find" that pulls boost::range::find into the boost namespace.
OK - it imports names I don't know about into my application.
How can this be any worse than defining boost::find directly in the boost namespace?
I object to that as well. But at least if I include boost::find I'm getting what I'm asking for - not some other stuff I don't know about unless I look at the library implementation.
It's actually way better this way as now boost::find will be less likely found via ADL.
Better - but not good
AFAIK, boost::find is documented, and boost::range::find is not. It's an implementation detail that, as I've said above, is no worse than defining boost::find directly in the boost namespace.
which I also object to. Robert Ramey
On Sun, Nov 4, 2012 at 1:42 PM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 10:10 AM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
Neil Groves wrote:
On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey <ramey@rrsd.com> wrote:
IMHO, a strict reading of [5] does not necessitate that a concept provide ways to allow the user to create models (e.g., write archetypes).
I believe a concept definition describes those valid expressions which necessary and SUFFICIENT to define the concept. That is, I think we'll agree that any type which models the concept should support all the valid expressions list. I belive also that any type which supports all the valid expressions listed is necessarily a model of the concept. Again, would appreciate input form a concept guru here.
Okay.
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.
Then...why are you assuming that?
Because the documentation says they are member functions but it doesn't say of what type. One has no choice to guess or assume.
What do you mean "of what type"? Why do you need to guess or assume anything? Method 1 just explains one way for a UDT to satisfy the Single Pass Range concept, but it doesn't imply this is the *only* way (see Method 2). Let me try to help. Method 1 can be summarized as: -------- One way for class X to model the Single Pass Range concept is for X to provide member functions X::begin/X::end and typedefs X::iterator/X::const_iterator. -------- This implies that all STL-style containers are automatically useable with Boost.Range, since they fulfill these member requirements. But it does *not* state the converse, i.e., that all classes which model Single Pass Range must have begin/end member functions and iterator/const_iterator typedefs. These member function requirements are sufficient, but not necessary. For example, UDTs may hook into Boost.Range non-intrusively via Method 2. Does that help?
In any case, these should be part of the
"valid expressions" section of the SinglePassRange concept.
*That* would be a mistake; begin/end member functions and an iterator typedef are just sufficient for a type to model SPR, but they aren't necessary. E.g., std::pair< int*, int* > is a valid range.
SinglePassRangeConcept requires the valid expression boost::begin(a) if a instance the type which models the concept. I've been directed to "method 1" of the documentation. There one is directed to implement a member begin() of some unspecified type. I'm sorry, if I want to make my own range, I just don't know what to do to from reading this.
If you want to adapt your own UDT to be a Single Pass Range, either (a) Use Method 1, i.e., define begin/end member functions and iterator/const_iterator typedefs. By "some unspecified type", are you referring to unspecified *signatures*? If so, yes, the documentation could be more precise here. Honestly, familiarity with STL containers should get most people past this underspecification. or (b) Use Method 2, in the event that your type does not meet the requirements of Method 1 and you must adapt the type extrinsically. It may be beneficial for the documentation to explicitly make the correspondence boost::begin(x) <=> x.begin() [Method 1] boost::begin(x) <=> range_begin(x) [Method 2] within the respective documentation for Methods 1 and 2, but I would think the intent is clear. Here, I mean that boost::begin(x) from the Single Pass Range concept definition is equivalent to x.begin() if X fulfills the requirements of Method 1, and likewise for Method 2, and likewise for the range_iterator metafunction. [...]
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.
No spelunking through code is necessary, you did that on your own.
Only because I had to.
If you want to understand what you can do and how to use a concept, the concept documentation is sufficient (IMO).
Of course that's where we can't agree.
Actually, I haven't seen any argument on this point, it's the next one that's contentious.
If you want to create an
archetype or adapt an existing UDT to a concept, the extenstion mechanism documentation is sufficient (IMO).
To my mind, the way the "extension mechanism" is formulated is a symptom of the problem. Much of what's in there - should be part of the the SinglePassRange concept if the concept were correctly formulated.
Yes, your position is clear. See next response.
This is also how Boost.Fusion and Boost.MPL (at least) are structured, AFAIK.
I don't think that's accurate. If you want to make your own model of a concept, you just make sure your types implement all the valid expressions.
Maybe in your world. And maybe your world is the correct one. But compare Boost.Fusion's Forward Sequence concept documentation [6] with its extension mechanism documentation [7]; it parallels Boost.Range's documentation. Boost.MPL is similar, only the documentation on its tag dispatching mechanism is harder to find and (IMO) under-documented. And, again, Boost.Parameter's documentation on its concepts [8] doesn't give any indication that a UDT can or should model these concepts. I don't think any of these concept documentations fit your idea of what a concept is. And maybe they're all wrong (or I'm all wrong), I don't know. IMHO, I think it's appropriate for the documentation to separate the concept definition (how you use and what you can do with a model of a concept) from any extension mechanisms (how you create a model of a concept), if said extension mechanisms are non-trivial.
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.
I'm sorry, I'm still not following. Can you be more concrete? I don't know what you mean by building a library "concurrently with the documentation", and I'm not sure what your suggestion about "how to go about these things" actually is.
rsd.com/blincubator.com/advice/
page not found
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).
Hmmm, works for me, as in, it fails to compile.
-------- #include <boost/concept/assert.hpp> #include <boost/range/algorithm/find.hpp> #include <boost/range/concepts.hpp>
struct X { };
namespace boost { int const * begin(X&); int const * end(X&); } // namespace boost
int main(int argc, char* argv[]) { X x; //boost::find(x, 0); // compiler error boost::range::find(x, 0); // compiler error return 0; } --------
Same error comes up as in a BOOST_CONCEPT_ASSERT, pointing to a line referencing range_const_iterator and range_mutable_iterator, suggesting that Boost.Range cannot associate an iterator type with X.
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.
Really, a "whole bunch"? It has a single "using range::find" that pulls boost::range::find into the boost namespace.
OK - it imports names I don't know about into my application.
It's documented to define boost::find, which it does, so you must be objecting to it also defining boost::range::find. Fine. Maybe it should have been boost::range_detail::find. Anyways, your entire complaint about this using declaration is a moving target. Let's drop it as...
How can this be
any worse than defining boost::find directly in the boost namespace?
I object to that as well. But at least if I include boost::find I'm getting what I'm asking for - not some other stuff I don't know about unless I look at the library implementation.
It's actually way better this way as now boost::find will be less likely found via ADL.
Better - but not good
AFAIK, boost::find is documented, and boost::range::find is not. It's an implementation detail that, as I've said above, is no worse than defining boost::find directly in the boost namespace.
which I also object to.
...the real issue you have is that find (and other Boost.Range algorithms) is directly in the boost namespace, which is a legitimate complaint. I at least give credit to Neil and/or Thorsten that they were at least wise enough to limit ADL problems via the using declaration. Also, boost::find being a direct analogue of std::find is at least some rationale (some may not think sufficient) for it residing directly in the boost namespace. - Jeff [6] http://www.boost.org/doc/libs/1_51_0/libs/fusion/doc/html/fusion/sequence/co... [7] http://www.boost.org/doc/libs/1_51_0/libs/fusion/doc/html/fusion/extension/e... [8] http://www.boost.org/doc/libs/1_51_0/libs/parameter/doc/html/reference.html#...
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:42 PM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 10:10 AM, Robert Ramey <ramey@rrsd.com> wrote:
Jeffrey Lee Hellrung, Jr. wrote:
On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey <ramey@rrsd.com> wrote:
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.
Then...why are you assuming that?
Because the documentation says they are member functions but it doesn't say of what type. One has no choice to guess or assume.
What do you mean "of what type"?
from page http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... Hmmm - upon a closer reading I see the page contains the sentence "Below is given an overview of which member functions and member types a class must specify to be useable as a certain Range concept." So my assumption is correct - if unnecessary.
I don't think that's accurate. If you want to make your own model of a concept, you just make sure your types implement all the valid expressions.
Maybe in your world. And maybe your world is the correct one. But compare Boost.Fusion's Forward Sequence concept documentation [6] with its extension mechanism documentation [7]; it parallels Boost.Range's documentation.
damn.. your right about fusion describing an extention mechanism. I either forgot about this or never had occasion to need it. It's a little unclear whether this is a requirement or a convenience to make it easier to "get it right". My view so far would be that it's not a requirement to use the extension mechanism but rather a convenience which helps the author of the extention to "get it right". I confess I don't really know though. Another thing about fusion is that it doesn't have any concept checking classes. I've written some for work I was doing but the lack of them in the library makes it less interesting for comparison than I thought.
I'm sorry, I'm still not following. Can you be more concrete? I don't know what you mean by building a library "concurrently with the documentation", and I'm not sure what your suggestion about "how to go about these things" actually is.
wrong link - try this one: rrsd.com/blincubator.com/advice/
#include <boost/concept/assert.hpp> #include <boost/range/algorithm/find.hpp> #include <boost/range/concepts.hpp>
struct X { };
namespace boost { int const * begin(X&); int const * end(X&); } // namespace boost
int main(int argc, char* argv[]) { X x; //boost::find(x, 0); // compiler error boost::range::find(x, 0); // compiler error return 0; } --------
Same error comes up as in a BOOST_CONCEPT_ASSERT, pointing to a line referencing range_const_iterator and range_mutable_iterator, suggesting that Boost.Range cannot associate an iterator type with X.
Hmmm - I just tried your example above on both my msvc 9.0 and gcc 4.5.3 compilers. The program fails to compile. It's slightly more complex on msvc. In this case only the first find fails. If you switch the order of the two find statements, still it's the first one. I'm guessing this is what I was seeing before. In any case, the find statements fail to compile even though type X models the concept SinglePassRange according to the documentation. Now this is the crux of my complaint. To me, it's an error on it's face. I understand you don't agree with this but think we just have to agree to disagree. Robert Ramey
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.
Then...why are you assuming that?
Because the documentation says they are member functions but it doesn't say of what type. One has no choice to guess or assume.
I re-read this. Turns out it DOES say that that the member functions correspond to an instance of a type which models SinglePassRange. So my assumption was correct - though unnecessary.
This is also how Boost.Fusion and Boost.MPL (at least) are structured, AFAIK.
I don't think that's accurate. If you want to make your own model of a concept, you just make sure your types implement all the valid expressions.
Maybe in your world. And maybe your world is the correct one. But compare Boost.Fusion's Forward Sequence concept documentation [6] with its extension mechanism documentation [7];
Damn - I didn't realze the fusion had an extention mechanism for Sequence. I don't know if this is necessary or some sort of convenience to help creators "get it right". Previously I made new algorithms just by implementing the relevant concept.
I'm sorry, I'm still not following. Can you be more concrete? I don't know what you mean by building a library "concurrently with the documentation", and I'm not sure what your suggestion about "how to go about these things" actually is.
rsd.com/blincubator.com/advice/
page not found
Lost a character - rrsd.com/blincubator.com/advice/
#include <boost/concept/assert.hpp> #include <boost/range/algorithm/find.hpp> #include <boost/range/concepts.hpp>
struct X { };
namespace boost { int const * begin(X&); int const * end(X&); } // namespace boost
int main(int argc, char* argv[]) { X x; //boost::find(x, 0); // compiler error boost::range::find(x, 0); // compiler error return 0; } --------
Same error comes up as in a BOOST_CONCEPT_ASSERT, pointing to a line referencing range_const_iterator and range_mutable_iterator, suggesting that Boost.Range cannot associate an iterator type with X.
I compiled this example on both MSVC 9.0 and GCC 4.5.3 Failed to compile on MSVC Compiled with no error on GCC. BTW - my view is that it SHOULD compile without error since struct X fullfills the requirements of the SinglePassConcept as written in the documentation. I think my original question was motivated by the fact that it didn't compile with my MSVC system. Of course I forget now. Robert Ramey
I believe a concept definition describes those valid expressions which necessary and SUFFICIENT to define the concept. That is, I think we'll agree that any type which models the concept should support all the valid expressions list. I belive also that any type which supports all the valid expressions listed is necessarily a model of the concept.
I agree with this. The problem is that you are equating "supporting all the valid expressions" with "implementing the functions that appear in the valid expressions yourself". Implementing the functions that appear in the valid expressions is *one* conceivable way a library can allow supporting all the valid expressions, but it's not the only way. Boost.Range provides a different mechanism to support the valid expressions: they implement the function that appears in the valid expression (boost::begin()) themselves, and you implement a helper function that this function calls (range_begin() or .begin()). I don't see anything wrong with that. Regards, Nate
Le 04/11/12 09:37, Robert Ramey a écrit :
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. I don't know from where you got that "valid expressions" should use member types. Concepts can concern non-member functions, think for example of a Swapable concept.
Best, Vicente
Vicente J. Botet Escriba wrote:
Le 04/11/12 09:37, Robert Ramey a écrit :
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.
I don't know from where you got that "valid expressions" should use member types. Concepts can concern non-member functions, think for example of a Swapable concept.
Actually it's worse than that. The serialization library uses them for non-intrusive serialization implementation. I realized this when i was doing some double checking before posting. (yes, I sometimes to that!). And I did moderate my insistence on this particular point. But still.... looking through http://en.cppreference.com/w/cpp/concept it seems that swappable/valueswappable is a very unusual and perhaps even unique case. I suspect this would be for the same reason that boost::begin(A) as a required expression raised a red flag when I looked at it. This technique/practice creates a concept which I can't easily verify. A couple of other points. a) I don't think it (boost::begin()) is necessary to implement the library. b) I don't find any mention of boost::begin() anywhere in the table of contents of the documentation. I guess it might be implemented somewhere for some types like stl containers (extending the library Method 1: refers to begin() member functions of a user type to be supported - but that would be a different thing) So I'll concede your point that including boost::begin() as a required valid expression does not violate any rules. But I don't think such a thing belongs here. Robert Ramey
Hi,
looking through http://en.cppreference.com/w/cpp/concept it seems that swappable/valueswappable is a very unusual and perhaps even unique case. I suspect this would be for the same reason that boost::begin(A) as a required expression raised a red flag when I looked at it. This technique/practice creates a concept which I can't easily verify. A couple of other points.
a) I don't think it (boost::begin()) is necessary to implement the library. Why do you think that? in fact the implementation using free standing begin() and end() functions as a mean for adapting third party containers was deemed so necessary that the standard commitee implemented the c++11 range based for in terms of begin(v) and end(v) and not in v.begin() and v.end(). Think about that: adding the required specialisation of boost::begin() and boost::end() at one central point of the program is much easier than writing a wrapper for every such container and use that everywhere where boost.range is used in conjunction with the container.
b) I don't find any mention of boost::begin() anywhere in the table of contents of the documentation. I guess it might be implemented somewhere for some types like stl containers (extending the library Method 1: refers to begin() member functions of a user type to be supported - but that would be a different thing) The default implementation of begin(v) is just "return v.begin();" so it is not a different thing.
Greetings, Oswin
Oswin Krause wrote:
Hi,
looking through http://en.cppreference.com/w/cpp/concept it seems that swappable/valueswappable is a very unusual and perhaps even unique case. I suspect this would be for the same reason that boost::begin(A) as a required expression raised a red flag when I looked at it. This technique/practice creates a concept which I can't easily verify. A couple of other points.
a) I don't think it (boost::begin()) is necessary to implement the library.
Why do you think that? in fact the implementation using free standing begin() and end() functions as a mean for adapting third party containers was deemed so necessary that the standard commitee implemented the c++11 range based for in terms of begin(v) and end(v) and not in v.begin() and v.end(). Think about that: adding the required specialisation of boost::begin() and boost::end() at one central point of the program is much easier than writing a wrapper for every such container and use that everywhere where boost.range is used in conjunction with the container.
actually what would have occurred to me would have been to just use X.begin() ... which containers already have. Other cases would be handled by a free standing function - similar to the way it does now I beleive.
b) I don't find any mention of boost::begin() anywhere in the table of contents of the documentation. I guess it might be implemented somewhere for some types like stl containers (extending the library Method 1: refers to begin() member functions of a user type to be supported - but that would be a different thing)
The default implementation of begin(v) is just "return v.begin();" so it is not a different thing.
I don't want this thread to pivot to the design of boost.range itself. That would require I spend a lot more time. My original observation was that it was no possible to use the library from a plain reading of the documentation - that it was confusing and misleading. I'd like to stick to the topic if we can. Robert Ramey
On 05-11-2012 05:39, Robert Ramey wrote:
I don't want this thread to pivot to the design of boost.range itself. That would require I spend a lot more time. My original observation was that it was no possible to use the library from a plain reading of the documentation - that it was confusing and misleading. I'd like to stick to the topic if we can.
Robert, We agree that the documentation could be improved. I think some better initial explanations of what and how it works would address most of your concerns. As always, we should strive for littering the documentation with examples. -Thorsten
Hi, a) this is the range extension mechanism. By default the usual v.begin() and v.end() is allowed and thus boost::begin(v) is of course a valid expression. For classes which don't adhere to the usual convention, you have to specialize the templates you found: http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... so boost::begin(v) is more flexible than v.begin() and that's why boost::begin is required instead of v.begin. But you re right that it should be stated, that this is a valid expression for containers. b) "The intention of the iterator_range class is to encapsulate two iterators so they fulfill the Forward Range concept". First line. http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/uti... c) The convention is: big libraries get their own namespace, small libraries don't need to. Also in this case it is convenient as one can replace std::find with boost::find instead of boost::range::find which - espcially in conjunction with the adaptors - leads to a lot of noise for a simple thing. On 2012-11-02 07:11, Robert Ramey wrote:
I want to use Boost.Range in a bigger way. But I have a few problems with it. I think most of them are just issues of understanding and the writting of the documentation
a) I see examples like
std::vector<T> v; boost::range::find(v, 0);
find(SinglePassRange& rng, Value val);
BUT when looking at the documentation at
http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... I see only two valid expressions
boost::begin(a) boost::end(a)
where in this case a would be v which is of type std::vector<int>. BUT
boost:begin(v) is NOT a valid expression for the above.
Spelunking into the range header code reveals the following:
template< class T > inline BOOST_DEDUCED_TYPENAME range_iterator<T>::type begin( T& r ) { #if !BOOST_WORKAROUND(__BORLANDC__, BOOST_TESTED_AT(0x564)) && \ !BOOST_WORKAROUND(__GNUC__, < 3) \ /**/ using namespace range_detail; #endif return range_begin( r ); }
and
template< typename C > inline BOOST_DEDUCED_TYPENAME range_iterator<C>::type range_begin( C& c ) { // // If you get a compile-error here, it is most likely because // you have not implemented range_begin() properly in // the namespace of C // return c.begin(); }
This effectively means that the "Valid Expressions" for SinglePassRange part of the documenation should read:
a.begin() a.end()
Doesn't it. I would be interested to know if I got this wrong.
b) I see the template iterator_range<ForwardTraversalIterator> -
which seem to be to in an instance of what the ForwardRangeConcept should be.
To me, the range documentation is out of whack. I can figure it out - but it's misleading - atleast to me.
c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict.
Robert Ramey
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
Oswin Krause wrote:
c) The convention is: big libraries get their own namespace, small libraries don't need to.
range is not a small library
Also in this case it is convenient as one can replace std::find with boost::find instead of boost::range::find which - espcially in conjunction with the adaptors - leads to a lot of noise for a simple thing.
one can always alias it in any application. We might disagree on what convention is. But I'll state that the boost namespace is getting two crowded. We are now encouraging new library to use the boost/<library name>/.....hpp and boost::<library name>:: ... convention. This really necessary to be able to use typenames such as find, etc in different libraries. Robert Ramey
Le 02/11/12 18:00, Robert Ramey a écrit :
Oswin Krause wrote:
c) The convention is: big libraries get their own namespace, small libraries don't need to. range is not a small library It was one at the beginning. Also in this case it is convenient as one can replace std::find with boost::find instead of boost::range::find which - espcially in conjunction with the adaptors - leads to a lot of noise for a simple thing. one can always alias it in any application.
We might disagree on what convention is. But I'll state that the boost namespace is getting two crowded. We are now encouraging new library to use the boost/<library name>/.....hpp and boost::<library name>:: ... convention.
This really necessary to be able to use typenames such as find, etc in different libraries.
Boost.Range was introduced in 1.32 and I think there were no such rule at this time. Of course we can move to a specific namespace and add a using declaration temporarily so that user can be able to disambiguate using the specific namespace. This is not always simple if as it is the case of Boost.range the user needs to specialize/overload some classes/functions on a specific namespace. I would like to add a new rule. Any library that has its counterpart in the C++ standard should/could use the boost namespace. Best, Vicente
Vicente J. Botet Escriba wrote:
Boost.Range was introduced in 1.32 and I think there were no such rule at this time.
I'm not saying there was. I'm not finding fault with anyone. I'm just saying this is an inconvenient feature of the library that I would like to see changed someday.
I would like to add a new rule. Any library that has its counterpart in the C++ standard should/could use the boost namespace.
hmmm - is std::find the counter part of boost::find ? boost::find works on range and has a different type signature. That's why I prefer boost::range::find - the little extra reduncancy catches coding errors. Robert Ramey
On 02-11-2012 07:11, Robert Ramey wrote:
I want to use Boost.Range in a bigger way. But I have a few problems with it. I think most of them are just issues of understanding and the writting of the documentation
Ok.
To me, the range documentation is out of whack. I can figure it out - but it's misleading - atleast to me.
I think some of the original up-front examples disappeared in version 2. You can still browse them under "Examples".
c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict.
Maybe so, but how would you change that without breaking code? -Thorsten
Thorsten Ottosen wrote:
On 02-11-2012 07:11, Robert Ramey wrote:
c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict.
Maybe so, but how would you change that without breaking code?
No question that fixing this would break existing code. The real question is whether making this change would be worth breaking code. a) The breakage in users of range wouldn't be a big deal in my opinion. Just rebuild the ap and fix the compilation errors. b) Making such a change would be a fairly big job - not a tweak. Of course this is not a big concern to me as I wouldn't be doing the job. Here are a few miscelaneous personal observations on the range library a) To me, the range library/concept is not appreciated to the extent it should be. I don't think that potential users appreciate the utility of being able to compose adaptors. I think the documentation would benefit from more examples. b) The documentation is pretty regular - I like this. But each page needs a small example to help clarify things. See fusion library for an example. c) I'm not crazy about the '|' syntax - OK I'm out voted here but I don't have to use it so I don't have any real objection. d) When I've used the library - I've found that I've had to spelunk through the code to understand how to "make it work". Of course now I don't recall the specific cases other than the one which motivated this post, but they are common. I believe that more examples and tests would smoke these out. e) Somethings are not quite right - the thing that motivated this post is the confluence of containers and ranges. There might be other things. I believe that this library could have a big future but that it needs more work to "get it right". This raises the question of how such work should get done. Of course this situation apply's to many libraries. Robert Ramey
Thorsten Ottosen wrote:
On 02-11-2012 07:11, Robert Ramey wrote:
c) Another really annoying thing is that every thing is directly in the boost namespace. This breaks the convention that library headers are in boost/<library name> and in the namespace boost::<library name>. This creates a lot of opportunity for conflict. Maybe so, but how would you change that without breaking code? No question that fixing this would break existing code.
The real question is whether making this change would be worth breaking code.
a) The breakage in users of range wouldn't be a big deal in my opinion. Just rebuild the ap and fix the compilation errors. This is not an option IMHO. b) Making such a change would be a fairly big job - not a tweak. Of course this is not a big concern to me as I wouldn't be doing the job. I agree that while possible, this will take a lot of time.
Here are a few miscelaneous personal observations on the range library
a) To me, the range library/concept is not appreciated to the extent it should be. I don't think that potential users appreciate the utility of being able to compose adaptors. I think the documentation would benefit from more examples. How do you know what potential users appreciate or not. Almost any Boost library could improve its documentation I I think some of them are already doing it.
b) The documentation is pretty regular - I like this. But each page needs a small example to help clarify things. See fusion library for an example.
c) I'm not crazy about the '|' syntax - OK I'm out voted here but I don't have to use it so I don't have any real objection. Here we agree ;-)
d) When I've used the library - I've found that I've had to spelunk through the code to understand how to "make it work". Of course now I don't recall the specific cases other than the one which motivated this post, but they are common. I believe that more examples and tests would smoke these out. I'm sure that if you post here the specific example that was confusing,
Le 02/11/12 17:54, Robert Ramey a écrit : the authors will try to correct it.
e) Somethings are not quite right - the thing that motivated this post is the confluence of containers and ranges. There might be other things.
What do you mean?
I believe that this library could have a big future but that it needs more work to "get it right". I think Boost.Range is an excellent library and that a lot of people are already taking advantage of it. This raises the question of how such work should get done. Of course this situation apply's to many libraries.
We agree again. Robert, I understand that you would like to see a lot of Boost libraries improved but that you can not do too much as you have a lot of work maintaining your own library. This is the case for most of the Boost library authors. Best, Vicente
Vicente J. Botet Escriba wrote:
a) To me, the range library/concept is not appreciated to the extent it should be. I don't think that potential users appreciate the utility of being able to compose adaptors. I think the documentation would benefit from more examples.'
How do you know what potential users appreciate or not. Almost any Boost library could improve its documentation I I think some of them are already doing it.
lol - well of course I can't KNOW what other users would appreciate. I'll agree that this is conjecture on my part. Let's give me a break and call it "informed conjecture".
d) When I've used the library - I've found that I've had to spelunk through the code to understand how to "make it work". Of course now I don't recall the specific cases other than the one which motivated this post, but they are common. I believe that more examples and tests would smoke these out. I'm sure that if you post here the specific example that was confusing, the authors will try to correct it.
lol - That's what I'm doing here!
e) Somethings are not quite right - the thing that motivated this post is the confluence of containers and ranges. There might be other things.
What do you mean?
That's what I've been trying to explain here. I concede with limited success. The documentation suggests that a SinglePassRange is sort of a "pair of interators". But it turns out that any Container is also a SinglePassRange. I'm sorry that's confusing to a plain reading. (note: understand how things are implemented so please don't re-explain it to me). I would expect something more along the lines of Nomenclature R SinglePassRange type r instance of a type R valid expressions r.begin() r.end() In this case then a any container would full fill the requirements of a SinglePassRange. To be conforming, something like "struct iterator_range" would have to supply the same functions. The notion boost::begin(r) and boost::end(r) seems out of place to me. I haven't thought through all the implications of this as it would have rippled through the library design. So cut me some slack here. Basically, I don't think the range library exploits the "concept" concept properly and the library suffers for it.. (off topic - I would love to hang the person who came up the with the name "concept" for what to me would better be called "type requirement".) Robert Ramey
Hi,
That's what I've been trying to explain here. I concede with limited success. The documentation suggests that a SinglePassRange is sort of a "pair of interators". But it turns out that any Container is also a SinglePassRange.
As it can be interpreted as a pair of iterators...why not? It would be confusing if standard containers would not model a SinglePassRange. Ignore for a moment the write up in the documentation. And instead think about the sentence "Every squence that can be represented by an iterator to the beginning and the end, is a SinglePassRange". This already encompasses standard containers and many things more. Now looking back at the documentation it is about how boost range formalises this concept in it's most abstract form. This is the reason why the following:
Nomenclature
R SinglePassRange type r instance of a type R
valid expressions
r.begin() r.end()
is not a correct description of the abstraction, as the library is in fact more genral. So this description would be truely misleading as it would encourage the thinking of a SPR as something having methods begin() and end(). In fact the biggest achievement of boost range is that it strays from this path saying that it is boost::begin(r) and boost::end(r). It is a different way of thinking about the ranges. You are thinking about "how can i see that my class conforms to the range concept" and the document is about "what is a range?"
To be conforming, something like "struct iterator_range" would have to supply the same functions. The notion boost::begin(r) and boost::end(r) seems out of place to me. I haven't thought through all the implications of this as it would have rippled through the library design. So cut me some slack here.
person who came up the with the name "concept" for what to me would better be called "type requirement".) This is your biggest misunderstanding is that you think about the range concept as type requirements even though th concept of a range is meant as a concept of a range.
Oswin Krause wrote:
person who came up the with the name "concept" for what to me would bettetbe called "type requirement".)
This is your biggest misunderstanding is that you think about the range concept as type requirements
This is the crux of our disagreement. I DO consider "type requirement" as a synonym for the word "concept" as used in C++ documentation. The adoption of the word "concept" for this purpose has led to untold amounts of confusion. Again - I invite any C++ concept guru is free to chime in on this.
even though th concept of a range is meant as a concept of a range.
I rest my case. Robert Ramey
On 02-11-2012 20:56, Robert Ramey wrote:
Vicente J. Botet Escriba wrote:
e) Somethings are not quite right - the thing that motivated this post is the confluence of containers and ranges. There might be other things.
What do you mean?
That's what I've been trying to explain here. I concede with limited success. The documentation suggests that a SinglePassRange is sort of a "pair of interators". But it turns out that any Container is also a SinglePassRange. I'm sorry that's confusing to a plain reading. (note: understand how things are implemented so please don't re-explain it to me). I would expect something more along the lines of
Nomenclature
R SinglePassRange type r instance of a type R
valid expressions
r.begin() r.end()
In this case then a any container would full fill the requirements of a SinglePassRange.
To be conforming, something like "struct iterator_range" would have to supply the same functions. The notion boost::begin(r) and boost::end(r) seems out of place to me. I haven't thought through all the implications of this as it would have rippled through the library design. So cut me some slack here.
Basically, I don't think the range library exploits the "concept" concept properly and the library suffers for it.. (off topic - I would love to hang the person who came up the with the name "concept" for what to me would better be called "type requirement".)
Well, the documentation was changed several times until we arrived at the current version that says boost::begin(rng) and boost::end(rng) has to be valid expressions. How types are mapped such that those expressions become valid is a different matter. It depends on what headers you include. You are welcome to suggest that containers should not be ranges and should require an explicit transformation to a range. Besides breaking tons of code, generate slower code, the it's just plain inconvenient. A range is any type you can iterate over using two iterators. Surely you would expect that from containers. regards -Thorsten
participants (8)
-
Jeffrey Lee Hellrung, Jr. -
Nathan Crookston -
Nathan Ridge -
Neil Groves -
Oswin Krause -
Robert Ramey -
Thorsten Ottosen -
Vicente J. Botet Escriba