[Boost-users] [Range] invalid initialization of non-const type with adaptors

[Denis Taniguchi]

Hi,

I was trying to compile the following simple piece of code:

#include <boost/range/algorithm.hpp>
#include <boost/range/adaptors.hpp>

#include <vector>

int main(int argc, char *argv[])
{
std::vector<int> vec(10);
boost::fill(vec | boost::adaptors::reversed, 1);

return 0;
}

But I get the following erros with gcc (Ubuntu 4.3.3-5ubuntu4) 4.3.3:

test_fill.cpp: In function ‘int main(int, char**)’:
test_fill.cpp:9: error: invalid initialization of non-const reference of
type ‘boost::range_detail::reverse_range<std::vector<int,
std::allocator<int> > >&’ from a temporary of type
‘boost::range_detail::reverse_range<std::vector<int, std::allocator<int>
> >’
/usr/local/include/boost/range/algorithm/fill.hpp:29: error: in passing
argument 1 of ‘ForwardRange& boost::range::fill(ForwardRange&, const
Value&) [with ForwardRange =
boost::range_detail::reverse_range<std::vector<int, std::allocator<int>
> >, Value = int]’

Am I missing something here?

Best regards,

Denis

_______________________________________________
Boost-users mailing list
Boost...@lists.boost.org
http://lists.boost.org/mailman/listinfo.cgi/boost-users

--
You received this message because you are subscribed to the Google Groups "BOOST Archives" group.
To post to this group, send email to boost...@googlegroups.com.
To unsubscribe from this group, send email to boost-list+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/boost-list?hl=en.

[Neil Groves]

On Mon, May 10, 2010 at 2:35 AM, Denis Taniguchi <tani...@tpn.usp.br> wrote:

Hi,

I was trying to compile the following simple piece of code:

#include <boost/range/algorithm.hpp>
#include <boost/range/adaptors.hpp>

#include <vector>

int main(int argc, char *argv[])
{
 std::vector<int> vec(10);
 boost::fill(vec | boost::adaptors::reversed, 1);

 return 0;
}

But I get the following erros with gcc (Ubuntu 4.3.3-5ubuntu4) 4.3.3:

Am I missing something here?

The reason this will not compile is that 'vec | boost::adaptors::reversed' returns an unnamed temporary which is then attempted to be passed to the fill function. On many versions of Visual C++ this actually will compile and work as you intended due to a language extension, but this is non-standard.

This can't easily be remedied since the Range Concept is non-copyable, and we obviously can't pass a const reference as the target to the fill algorithm. Making the Range Concept cheaply copyable would mean that the standard containers were no longer a model of the Range Concepts. I am currently investigating extending the sub_range into some new Range concepts that would integrate Alexandrescu's work into Boost.Range nicely. These sub-ranges would have to be copyable with constant-time complexity and therefore would enable the fill function to be written to take the sub-range by value. This would make your code compile. This work, however is very much in the experimental stage.

However this isn't a big problem because we typically solve the problem with the current Boost.Range like this:

#include <boost/range/adaptor/reversed.hpp>

#include <boost/range/irange.hpp>

#include <boost/range/algorithm_ext/push_back.hpp>

#include <vector>

int main(int argc, char *argv[])

{

std::vector<int> vec;

boost::push_back(vec, boost::irange(0, 10) | boost::adaptors::reversed);

return 0;

}

The adaptor is more typically applied to the right-hand side of the expression, and the population of a target container is normally better achieved by using one the algorithm_ext functions. In this case I am demonstrating the use of push_back.

This is better arrangement over the example code because the size does not have to be ascertained twice (once in the constructor of vec, and again in the actual source expression).

The other alternative arrangements that are equally safe to the new push_back use an back_insert_iterator which is less efficient.

Hence the new algorithms for transferring ranges to containers are safer, cleaner and more efficient than the alternative designs.

 

Best regards,

Denis

Regards,

Neil Groves 

[Mathias Gaunard]

Denis Taniguchi wrote:
> Hi,
>
> I was trying to compile the following simple piece of code:
>
> #include <boost/range/algorithm.hpp>
> #include <boost/range/adaptors.hpp>
>
> #include <vector>
>
> int main(int argc, char *argv[])
> {
> std::vector<int> vec(10);
> boost::fill(vec | boost::adaptors::reversed, 1);
>
> return 0;
> }

Workaround suggestions:

std::vector<int> vec(10);
BOOST_AUTO(vec_reversed, vec | boost::adaptors::reversed);
boost::fill(vec_reversed, 1);

std::vector<int> vec(10);
BOOST_FOREACH(int& i, vec | boost::adaptors::reversed)
i = 1;

The first one requires certain compilers.

[Mathias Gaunard]

Neil Groves wrote:

> This can't easily be remedied since the Range Concept is non-copyable,
> and we obviously can't pass a const reference as the target to the fill
> algorithm. Making the Range Concept cheaply copyable would mean that the
> standard containers were no longer a model of the Range Concepts. I am
> currently investigating extending the sub_range

It should be perfectly ok to make iterator_range and sub_range cheaply
copyable, since they just wrap iterators.
No need to extend the range concepts for that; iterator_range would
simply provide more than those concepts.

[Thorsten Ottosen]

Neil Groves skrev:

> On Mon, May 10, 2010 at 2:35 AM, Denis Taniguchi <tani...@tpn.usp.br
> <mailto:tani...@tpn.usp.br>> wrote:
>
> Hi,
>
> I was trying to compile the following simple piece of code:
>
> #include <boost/range/algorithm.hpp>
> #include <boost/range/adaptors.hpp>
>
> #include <vector>
>
> int main(int argc, char *argv[])
> {
> std::vector<int> vec(10);
> boost::fill(vec | boost::adaptors::reversed, 1);
>
> return 0;
> }
>
> But I get the following erros with gcc (Ubuntu 4.3.3-5ubuntu4) 4.3.3:
> Am I missing something here?

We shall ignore that there is no reason to use "reversed" here.

>
> The reason this will not compile is that 'vec |
> boost::adaptors::reversed' returns an unnamed temporary which is then
> attempted to be passed to the fill function. On many versions of Visual
> C++ this actually will compile and work as you intended due to a
> language extension, but this is non-standard.

Does make_reverse_range(rng) not use iterator_range rather than sub_range?

Also, I don't see any reason why a const T& overload of boost::fill
cannot exists as long as the const-iterator is actually a mutable
iterator (like with iterator_range<MutableRange>).

-Thorsten

[Thorsten Ottosen]

Thorsten Ottosen skrev:

> Also, I don't see any reason why a const T& overload of boost::fill
> cannot exists as long as the const-iterator is actually a mutable
> iterator (like with iterator_range<MutableRange>).

Also note that in a world with perfect forwarding, we shuold probably
specify fill() like

template< class Rng >
void fill( Rng&& rng, range_value<Rng>::type x );

[Neil Groves]

On Mon, May 10, 2010 at 2:24 PM, Mathias Gaunard <mathias...@ens-lyon.org> wrote:

Neil Groves wrote:

This can't easily be remedied since the Range Concept is non-copyable, and we obviously can't pass a const reference as the target to the fill algorithm. Making the Range Concept cheaply copyable would mean that the standard containers were no longer a model of the Range Concepts. I am currently investigating extending the sub_range

It should be perfectly ok to make iterator_range and sub_range cheaply copyable, since they just wrap iterators.
No need to extend the range concepts for that; iterator_range would simply provide more than those concepts.

The rationale for providing new concepts that extend the sub_range is that we can then implement new containers and new algorithms in terms of concepts similar to those that Alexandrescu describes. 

As you have suggested this is actually a very simple extension to sub-range and allows efficient interoperability between iterator ranges and Alexandrescu style ranges. I'm finding this to be very nice when implementing ranges that do not map well to iterators, such as an unbounded integer range.

I'm in the very early stages of exploring these ideas.

Regards,

Neil Groves

[Neil Groves]

On Mon, May 10, 2010 at 2:51 PM, Thorsten Ottosen <nes...@cs.aau.dk> wrote:

Thorsten Ottosen skrev:

Also, I don't see any reason why a const T& overload of boost::fill cannot exists as long as the const-iterator is actually a mutable iterator (like with iterator_range<MutableRange>).

Also note that in a world with perfect forwarding, we shuold probably specify fill() like

 template< class Rng >
 void fill( Rng&& rng, range_value<Rng>::type x );

Wouldn't this stop fill from working on ranges of abstract types?

I like the idea of taking advantage of perfect forwarding where available.

 

-Thorsten

Regards,

Neil Groves 

[Denis Taniguchi]

Looking forward to see it in next revisions of boost.
Thank you all for the replies, and I'll try to post more meaningful
examples next time.

Regards

>
> Regards,
> Neil Groves

[Thorsten Ottosen]

Neil Groves skrev:

>
>
> On Mon, May 10, 2010 at 2:51 PM, Thorsten Ottosen <nes...@cs.aau.dk
> <mailto:nes...@cs.aau.dk>> wrote:
>
> Thorsten Ottosen skrev:
>
>
> Also, I don't see any reason why a const T& overload of
> boost::fill cannot exists as long as the const-iterator is
> actually a mutable iterator (like with
> iterator_range<MutableRange>).
>
>
> Also note that in a world with perfect forwarding, we shuold
> probably specify fill() like
>
> template< class Rng >
> void fill( Rng&& rng, range_value<Rng>::type x );
>
>
> Wouldn't this stop fill from working on ranges of abstract types?

I don't know. Usually when deal with abstract types, I don't use fill()
for anything because those types are not copyable or assignable.

[Neil Groves]

   Also note that in a world with perfect forwarding, we shuold
   probably specify fill() like

    template< class Rng >
    void fill( Rng&& rng, range_value<Rng>::type x );


Wouldn't this stop fill from working on ranges of abstract types?

I don't know. Usually when deal with abstract types, I don't use fill()
for anything because those types are not copyable or assignable.

-Thorsten

I have infrequently have abstract types that are assignable. I don't think this alters your key point about the forwarding but I think we should probably have this instead:

template< class Range, class Value >

void fill( Rng&&  rng, const Value& x );

Where 'Value' is required to be convertible to one of the right-hand side arguments of an assignment to a range_value<Rng>::type.

Does this look sensible to you?

Regards,

Neil Groves

[Thorsten Ottosen]

Neil Groves skrev:

> I have infrequently have abstract types that are assignable. I don't
> think this alters your key point about the forwarding but I think we
> should probably have this instead:
>
> template< class Range, class Value >
> void fill( Rng&& rng, const Value& x );
>
> Where 'Value' is required to be convertible to one of the right-hand
> side arguments of an assignment to a range_value<Rng>::type.
>
> Does this look sensible to you?

I'm slightly against a template argument here as it means more code
being generated. I would rather see the conversion to happen before
the function is called. Would


const typename range_value<Range>::type&

not work?

[Steven Watanabe]

AMDG

Thorsten Ottosen wrote:
> Neil Groves skrev:
>> I have infrequently have abstract types that are assignable. I don't
>> think this alters your key point about the forwarding but I think we
>> should probably have this instead:
>>
>> template< class Range, class Value >
>> void fill( Rng&& rng, const Value& x );
>>
>> Where 'Value' is required to be convertible to one of the right-hand
>> side arguments of an assignment to a range_value<Rng>::type.
>>
>> Does this look sensible to you?
>
> I'm slightly against a template argument here as it means more code
> being generated. I would rather see the conversion to happen before
> the function is called. Would
>
>
> const typename range_value<Range>::type&
>
> not work?

The standard has

template<class ForwardIterator, class T>
void fill(ForwardIterator first, ForwardIterator last, const T& value);

I don't think we should do it differently.

In Christ,
Steven Watanabe