[Range] Confusing result of iterator_range::size()
Hi, I try use boost::iterator_range as replacement for some standard container in my code. The function iterator_range::size() has non standart declaration: difference_type size() const; but not: size_type size() const When I compile code in VC++ 2010 I have a some warning and error. For example: #include <boost/range.hpp> #include <vector> #include <algorithm> int main() { int aaa[10] ={1,2,3,4,5,6,7,8,9,10}; std::vector<int> bbb(20); boost::iterator_range<int*> arr(aaa); if(bbb.size() > arr.size()) //warning C4018: '>' : signed/unsigned mismatch { } const size_t lng = std::min(bbb.size(), arr.size()); //error C2780: 'const _Ty &std::min(const _Ty &,const _Ty &,_Pr)' : expects 3 arguments - 2 provided //error C2782: 'const _Ty &std::min(const _Ty &,const _Ty &)' : template parameter '_Ty' is ambiguous // const size_t lng = std::min(bbb.size(), (size_t)arr.size()); //OK for(size_t i = 0; i < lng; ++i) { bbb[i] = arr[i]; } return 0; } I think declaration of boost::iterator_range must be fixed. Or there was some reason for this non standard declaration?
On Wed, Nov 2, 2011 at 11:42 PM, Sergey Voropaev <serge-voropaev@yandex.ru>wrote:
Hi, I try use boost::iterator_range as replacement for some standard container in my code. The function iterator_range::size() has non standart declaration:
difference_type size() const; but not: size_type size() const
[...]
I think declaration of boost::iterator_range must be fixed. Or there was some reason for this non standard declaration?
This came up just recently, e.g., http://comments.gmane.org/gmane.comp.lib.boost.devel/224758 I think the end result was an agreement that typedef typename boost::make_unsigned< difference_type >::type size_type would be a valid definition of size_type, so the only argument against making the change that I can see is breaking existing code (probably unlikely...?) and...inertia. - Jeff
Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:
This came up just recently, e.g.,
Sorry, I miss it. Good discussion. And now we have my example with compilation error.
I think the end result was an agreement that
typedef typename boost::make_unsigned< difference_type >::type size_type
would be a valid definition of size_type, so the only argument against making the change that I can see is breaking existing code (probably unlikely...?) and...inertia.
I did not find authors of "Range" in the discussion. I am afraid your fix will be missed. It is good idea to write formal bug. Can you do it? Sergey Voropaev
Den 03-11-2011 09:12, Sergey Voropaev skrev:
Jeffrey Lee Hellrung, Jr.<jeffrey.hellrung<at> gmail.com> writes:
This came up just recently, e.g.,
Sorry, I miss it. Good discussion. And now we have my example with compilation error.
I think the end result was an agreement that
typedef typename boost::make_unsigned< difference_type>::type size_type
would be a valid definition of size_type, so the only argument against making the change that I can see is breaking existing code (probably unlikely...?) and...inertia.
Well, its /not/ unlikely that it will break code.
I did not find authors of "Range" in the discussion. I am afraid your fix will be missed. It is good idea to write formal bug. Can you do it?
I think the most obvious reason we (probabl me) did it like that is that iterator traits have no notion of unsigned types. The above make_signed<> thing could work, but still it will break code. The min() example that fails to compile is tricky. Even if we make it unsigned, size_types may differ (e.g. a 32 bit unsigned int and a 64 bit size_t). One could change min/max to use mpl::identity<T>::type to avoid a second conflicting type deduction, but then you get a warning inside min/max instead. In the end this means that you might as well make the conversion up-front, in your call to min/max. -Thorsten
On Thu, Nov 3, 2011 at 1:54 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:
Den 03-11-2011 09:12, Sergey Voropaev skrev:
Jeffrey Lee Hellrung, Jr.<jeffrey.hellrung<at> gmail.com> writes:
This came up just recently, e.g.,
Sorry, I miss it. Good discussion. And now we have my example with compilation error.
I think the end result was an agreement that
typedef typename boost::make_unsigned< difference_type>::type size_type
would be a valid definition of size_type, so the only argument against making the change that I can see is breaking existing code (probably unlikely...?) and...inertia.
Well, its /not/ unlikely that it will break code.
Well, okay, sure, *some* code is going to break. Do you have actual code (i.e., not contrived examples) that you predict would fail? Given that this has come up twice in the last month, I think it's worth discussing the fallout from such a change. If one were to redesign boost::iterator_range all over again, is there an argument *against* making size() return an unsigned type? - Jeff
Den 03-11-2011 10:13, Jeffrey Lee Hellrung, Jr. skrev:
On Thu, Nov 3, 2011 at 1:54 AM, Thorsten Ottosen< thorsten.ottosen@dezide.com> wrote:
Well, its /not/ unlikely that it will break code.
Well, okay, sure, *some* code is going to break. Do you have actual code (i.e., not contrived examples) that you predict would fail?
Its not contrieved at all. Did you not read the rest of my answer? Even changing it to unsigned won't remove all compile errors that motivated a change.
Given that this has come up twice in the last month, I think it's worth discussing the fallout from such a change. If one were to redesign boost::iterator_range all over again, is there an argument *against* making size() return an unsigned type?
Well, it used to return an unsigned type. It was then changed because iterator traits have no notion of unsigned types. And , AFAICR, there was a discussion about this on the list, convincing me that using difference_type was the right thing to do. It also makes it easier to write an assertion, if someone constructed an iterator range incorrectly. -Thorsten
On Thu, Nov 3, 2011 at 11:00 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Given that this has come up twice in the last month, I think it's worth discussing the fallout from such a change. If one were to redesign boost::iterator_range all over again, is there an argument *against* making size() return an unsigned type?
Well, it used to return an unsigned type. It was then changed because
What unsigned type?
iterator traits have no notion of unsigned types.
And , AFAICR, there was a discussion about this on the list, convincing me that using difference_type was the right thing to do.
AFAIK the conclusion was the opposite: use make_unsigned. -- Olaf
Den 03-11-2011 11:04, Olaf van der Spek skrev:
On Thu, Nov 3, 2011 at 11:00 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Given that this has come up twice in the last month, I think it's worth discussing the fallout from such a change. If one were to redesign boost::iterator_range all over again, is there an argument *against* making size() return an unsigned type?
Well, it used to return an unsigned type. It was then changed because
What unsigned type?
either size_t or something like make_unsigned<>::type.
iterator traits have no notion of unsigned types.
And , AFAICR, there was a discussion about this on the list, convincing me that using difference_type was the right thing to do.
AFAIK the conclusion was the opposite: use make_unsigned.
If you can find it, I might believe you. It should be in the archieves from some years ago. -Thorsten
On Thu, Nov 3, 2011 at 1:08 PM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Well, it used to return an unsigned type. It was then changed because
What unsigned type?
either size_t or something like make_unsigned<>::type.
Both sound better than the current solution.
iterator traits have no notion of unsigned types.
And , AFAICR, there was a discussion about this on the list, convincing me that using difference_type was the right thing to do.
AFAIK the conclusion was the opposite: use make_unsigned.
If you can find it, I might believe you. It should be in the archieves from some years ago.
I meant the discussion last month. Didn't know it was discussed before. Do you have any idea in what year/month? -- Olaf
Den 03-11-2011 13:16, Olaf van der Spek skrev:
If you can find it, I might believe you. It should be in the archieves from some years ago.
I meant the discussion last month. Didn't know it was discussed before. Do you have any idea in what year/month?
A couple of years maybe. I think it happened at the same time we demanded that size(Rng) only worked for RandomAccessRanges and that this function also returns a signed type. Using the old docs, one can probably track down the approximate time. The bottom line is that it was a deliberate decision. The example put forward std::min(bbb.size(), (size_t)arr.size()); is not portable either. -Thorsten
On Thu, Nov 3, 2011 at 2:51 PM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Den 03-11-2011 13:16, Olaf van der Spek skrev:
If you can find it, I might believe you. It should be in the archieves from some years ago.
I meant the discussion last month. Didn't know it was discussed before. Do you have any idea in what year/month?
A couple of years maybe. I think it happened at the same time we demanded that size(Rng) only worked for RandomAccessRanges and that this function also returns a signed type.
IMO the same arguments apply to that function.
Using the old docs, one can probably track down the approximate time.
The bottom line is that it was a deliberate decision. The example put forward
std::min(bbb.size(), (size_t)arr.size());
is not portable either.
My concern was about a.size() < b.size() where one size() returns a signed type and the other an unsigned type. What about ptr_range? -- Olaf
Den 03-11-2011 16:15, Olaf van der Spek skrev:
A couple of years maybe. I think it happened at the same time we demanded that size(Rng) only worked for RandomAccessRanges and that this function also returns a signed type.
IMO the same arguments apply to that function.
Right. They should behave similarly, no matter what is decided that is the best.
Using the old docs, one can probably track down the approximate time.
The bottom line is that it was a deliberate decision. The example put forward
std::min(bbb.size(), (size_t)arr.size());
is not portable either.
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it. -Thorsten
On Fri, Nov 4, 2011 at 11:02 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it.
We could ensure all size() return an unsigned type. What about ptr_range? Olaf
on Fri Nov 04 2011, Olaf van der Spek <ml-AT-vdspek.org> wrote:
On Fri, Nov 4, 2011 at 11:02 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it.
We could ensure all size() return an unsigned type.
Yes, that is already part of the container requirements, which would otherwise be refinements of the range requirements. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
On Sat, Nov 5, 2011 at 4:26 AM, Dave Abrahams <dave@boostpro.com> wrote:
on Fri Nov 04 2011, Olaf van der Spek <ml-AT-vdspek.org> wrote:
On Fri, Nov 4, 2011 at 11:02 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it.
We could ensure all size() return an unsigned type.
Yes, that is already part of the container requirements, which would otherwise be refinements of the range requirements.
So boost::iterator_range::size() and boost::size() violate this? Olaf
on Sat Nov 05 2011, Olaf van der Spek <ml-AT-vdspek.org> wrote:
On Sat, Nov 5, 2011 at 4:26 AM, Dave Abrahams <dave@boostpro.com> wrote:
on Fri Nov 04 2011, Olaf van der Spek <ml-AT-vdspek.org> wrote:
On Fri, Nov 4, 2011 at 11:02 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it.
We could ensure all size() return an unsigned type.
Yes, that is already part of the container requirements, which would otherwise be refinements of the range requirements.
So boost::iterator_range::size() and boost::size() violate this?
I don't have the spec in front of me, but that's my understanding from the foregoing conversation. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Den 05-11-2011 21:23, Dave Abrahams skrev:
on Sat Nov 05 2011, Olaf van der Spek<ml-AT-vdspek.org> wrote:
So boost::iterator_range::size() and boost::size() violate this?
I don't have the spec in front of me, but that's my understanding from the foregoing conversation.
Well, iterator_range<T> is not a container. Could you elaborate on your view? -Thorsten
On Mon, Nov 7, 2011 at 10:11 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Den 05-11-2011 21:23, Dave Abrahams skrev:
on Sat Nov 05 2011, Olaf van der Spek<ml-AT-vdspek.org> wrote:
So boost::iterator_range::size() and boost::size() violate this?
I don't have the spec in front of me, but that's my understanding from the foregoing conversation.
Well, iterator_range<T> is not a container. Could you elaborate on your view?
1. The intention of the iterator_range class is to encapsulate two iterators so they fulfill the Forward Range concept. 2. Size type boost::range_size<X>::type An unsigned integral type that can represent any nonnegative value of the Range's distance type. 1. http://www.boost.org/doc/libs/1_47_0/libs/range/doc/html/range/reference/uti... 2. http://www.boost.org/doc/libs/1_47_0/libs/range/doc/html/range/concepts/forw...
Den 07-11-2011 11:08, Olaf van der Spek skrev:
1. The intention of the iterator_range class is to encapsulate two iterators so they fulfill the Forward Range concept. 2. Size type boost::range_size<X>::type An unsigned integral type that can represent any nonnegative value of the Range's distance type.
1. http://www.boost.org/doc/libs/1_47_0/libs/range/doc/html/range/reference/uti... 2. http://www.boost.org/doc/libs/1_47_0/libs/range/doc/html/range/concepts/forw...
Yeah, that's just some docs that were not updated when we made the change. -Thorsten
Den 05-11-2011 21:23, Dave Abrahams skrev:
on Sat Nov 05 2011, Olaf van der Spek<ml-AT-vdspek.org> wrote:
So boost::iterator_range::size() and boost::size() violate this?
I don't have the spec in front of me, but that's my understanding from the foregoing conversation.
Well, iterator_range<T> is not a container. Could you elaborate on your view?
I think Dave's trying to say that if the size() of a range in a signed type, but the size() of a container is an unsigned type, then the container concept fails to be a refinement of the range concept, which it should be. Regards, Nate
on Mon Nov 07 2011, Nathan Ridge <zeratul976-AT-hotmail.com> wrote:
Den 05-11-2011 21:23, Dave Abrahams skrev:
on Sat Nov 05 2011, Olaf van der Spek<ml-AT-vdspek.org> wrote:
So boost::iterator_range::size() and boost::size() violate this?
I don't have the spec in front of me, but that's my understanding from the foregoing conversation.
Well, iterator_range<T> is not a container. Could you elaborate on your view?
I think Dave's trying to say that if the size() of a range in a signed type, but the size() of a container is an unsigned type, then the container concept fails to be a refinement of the range concept, which it should be.
Precisely. Although in theory Range could decide not to specify whether size() is signed or unsigned (in which case Container would still be a refinement), I consider that to be premature generalization and needless complexity at best. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
On Fri, Nov 4, 2011 at 12:27 PM, Olaf van der Spek <ml@vdspek.org> wrote:
On Fri, Nov 4, 2011 at 11:02 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
My concern was about a.size()< b.size() where one size() returns a signed type and the other an unsigned type.
Yes, that happens a lot in C++. Not much we can do about it.
We could ensure all size() return an unsigned type.
What about ptr_range?
Thorsten? -- Olaf
On Mon, Nov 7, 2011 at 4:03 PM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
Den 07-11-2011 13:16, Olaf van der Spek skrev:
On Fri, Nov 4, 2011 at 12:27 PM, Olaf van der Spek<ml@vdspek.org> wrote:
We could ensure all size() return an unsigned type.
What about ptr_range?
We do not need a new class for this.
Will you fix it by using make_unsigned then? Also, don't forget data() https://svn.boost.org/trac/boost/ticket/5930
On Thu, Nov 3, 2011 at 9:54 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
I think the end result was an agreement that
typedef typename boost::make_unsigned< difference_type>::type size_type
would be a valid definition of size_type, so the only argument against making the change that I can see is breaking existing code (probably unlikely...?) and...inertia.
Well, its /not/ unlikely that it will break code.
I did not find authors of "Range" in the discussion. I am afraid your fix will be missed. It is good idea to write formal bug. Can you do it?
I think the most obvious reason we (probabl me) did it like that is that iterator traits have no notion of unsigned types.
Maybe you could you provide a new class, ptr_range? I'd derive from iterator_range, use ::std::size_t for size() and provide a data() member? It's only a partial solution. -- Olaf
Thorsten Ottosen <thorsten.ottosen <at> dezide.com> writes:
The min() example that fails to compile is tricky...
This example is derived from real valid code.
Even if we make it unsigned, size_types may differ (e.g. a 32 bit unsigned int and a 64 bit size_t). Yes, but mostly all well-known containers use size_t as size_type.(I can imagine container with 64 bit unsigned size in 32 bit programm, but it is not common case). But using 32 bit int for size in programm will be error of programmer but not error of container design. Container with signed int as size_type is very unusual container. iterator_range is used for modifying existing code too and mimicry with usual container is important.
Sergey Voropaev
Den 03-11-2011 10:35, Sergey Voropaev skrev:
Thorsten Ottosen<thorsten.ottosen<at> dezide.com> writes:
The min() example that fails to compile is tricky...
This example is derived from real valid code.
Even if we make it unsigned, size_types may differ (e.g. a 32 bit unsigned int and a 64 bit size_t). Yes, but mostly all well-known containers use size_t as size_type.(I can imagine container with 64 bit unsigned size in 32 bit programm, but it is not common case). But using 32 bit int for size in programm will be error of programmer but not error of container design. Container with signed int as size_type is very unusual container.
Why? Its not uncommon to use 32 bits for storing size/capacity in vector like containers, even for 64 bit development. -Thorsten
Thorsten Ottosen <thorsten.ottosen <at> dezide.com> writes:
Den 03-11-2011 10:35, Sergey Voropaev skrev:
Thorsten Ottosen<thorsten.ottosen<at> dezide.com> writes:
Yes, but mostly all well-known containers use size_t as size_type.(I can imagine container with 64 bit unsigned size in 32 bit programm, but it is not common case). But using 32 bit int for size in programm will be error of programmer but not error of container design. Container with signed int as size_type is very unusual container.
Why? Its not uncommon to use 32 bits for storing size/capacity in vector like containers, even for 64 bit development.
Sorry, but I do not say it. I say its common to use size_t for storing size/capacity. And its non common to use signed. It is very confusing for me as user of your library. Sergey Voropaev
On Thursday, November 03, 2011 08:12:16 Sergey Voropaev wrote:
I did not find authors of "Range" in the discussion. I am afraid your fix will be missed. It is good idea to write formal bug. Can you do it?
There's this ticket already: <https://svn.boost.org/trac/boost/ticket/5971>
participants (8)
-
Andrey Semashev -
Dave Abrahams -
Jeffrey Lee Hellrung, Jr. -
michi7x7 -
Nathan Ridge -
Olaf van der Spek -
Sergey Voropaev -
Thorsten Ottosen