[Multiarray] & [Range] MSVC/gcc warning: returning address of local variable or temporary

The following code generates a worrying warning under MSVC: #include <boost/range/iterator_range.hpp> #include <boost/multi_array.hpp> typedef boost::multi_array< double, 1 > my_array; typedef my_array::const_iterator it; void test(const my_array & a) { boost::iterator_range< it > it_range(a.begin(), a.begin() + 1); it_range[0]; } C:\Dev\ThirdParty\boost\boost_1_37_0\boost/range/iterator_range.hpp(356) : warning C4172: returning address of local variable or temporary C:\Dev\ThirdParty\boost\boost_1_37_0\boost/range/iterator_range.hpp(354) : while compiling class template member function 'const double &boost::iterator_range<IteratorT>::operator [](__w64 int) const' with [ IteratorT=it ] iterator_range.cpp(8) : see reference to class template instantiation 'boost::iterator_range<IteratorT>' being compiled with [ IteratorT=it ] I'm using msvc 8.0 express and boost 1.37.0 release. I've seen similar warnings from gcc 4.3 on similar code. Is this a problem with boost.Multiarray, boost.range or the compiler? My guess is that Multiarray has defined its iterators incorrectly. Can anyone confirm this/provide a fix? Thanks, John.

AMDG John Reid wrote:
The following code generates a worrying warning under MSVC:
<snip>
I'm using msvc 8.0 express and boost 1.37.0 release. I've seen similar warnings from gcc 4.3 on similar code. Is this a problem with boost.Multiarray, boost.range or the compiler? My guess is that Multiarray has defined its iterators incorrectly. Can anyone confirm this/provide a fix?
This is the result of a bad interaction between iterator_range and iterator_facade. iterator_facade makes the result of operator[] an rvalue http://www.boost.org/libs/iterator/doc/iterator_facade.html#operator iterator_range assumes that operator[] for an iterator returns the type reference. http://www.boost.org/libs/range/doc/utility_class.html#iter_range In Christ, Steven Watanabe

on Tue Nov 11 2008, Steven Watanabe <watanabesj-AT-gmail.com> wrote:
AMDG
John Reid wrote:
The following code generates a worrying warning under MSVC:
<snip>
I'm using msvc 8.0 express and boost 1.37.0 release. I've seen similar warnings from gcc 4.3 on similar code. Is this a problem with boost.Multiarray, boost.range or the compiler? My guess is that Multiarray has defined its iterators incorrectly. Can anyone confirm this/provide a fix?
This is the result of a bad interaction between iterator_range and iterator_facade.
iterator_facade makes the result of operator[] an rvalue http://www.boost.org/libs/iterator/doc/iterator_facade.html#operator
iterator_range assumes that operator[] for an iterator returns the type reference. http://www.boost.org/libs/range/doc/utility_class.html#iter_range
FWIW, iterator_range is in the wrong here. That is not a safe assumption. It can, however, be checked for fairly reliably at compile-time. -- Dave Abrahams BoostPro Computing http://www.boostpro.com

David Abrahams skrev:
on Tue Nov 11 2008, Steven Watanabe <watanabesj-AT-gmail.com> wrote:
AMDG
John Reid wrote:
The following code generates a worrying warning under MSVC:
<snip>
I'm using msvc 8.0 express and boost 1.37.0 release. I've seen similar warnings from gcc 4.3 on similar code. Is this a problem with boost.Multiarray, boost.range or the compiler? My guess is that Multiarray has defined its iterators incorrectly. Can anyone confirm this/provide a fix? This is the result of a bad interaction between iterator_range and iterator_facade.
iterator_facade makes the result of operator[] an rvalue http://www.boost.org/libs/iterator/doc/iterator_facade.html#operator
iterator_range assumes that operator[] for an iterator returns the type reference. http://www.boost.org/libs/range/doc/utility_class.html#iter_range
FWIW, iterator_range is in the wrong here. That is not a safe assumption. It can, however, be checked for fairly reliably at compile-time.
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding? If so, how? -Thorsten

on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type. Maybe if it isn't, you could turn off whatever you're doing with enable_if or something. Why is iterator_range using the iterator's operator[], anyway? -- Dave Abrahams BoostPro Computing http://www.boostpro.com

AMDG David Abrahams wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type.
How do you distinguish const T& from T? If T is large, it is not necessarily a good idea to make extraneous copies if they can be avoided. Unlike iterator_facade, iterator_range doesn't make it easy to replace its version of operator[]... In Christ, Steven Watanabe

Steven Watanabe skrev:
AMDG
David Abrahams wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type.
How do you distinguish const T& from T? If T is large, it is not necessarily a good idea to make extraneous copies if they can be avoided. Unlike iterator_facade, iterator_range doesn't make it easy to replace its version of operator[]...
I provided operator() for use when pass by value must be used. -Thorsten

David Abrahams skrev:
on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type. Maybe if it isn't, you could turn off whatever you're doing with enable_if or something. Why is iterator_range using the iterator's operator[], anyway?
It seems very natural to be able to index a random access range, doesn't it. And how else would you implement that? Anyway, I guess I could do that check. Is it illegal for the iterators reference type to be a non-reference? -Thorsten

on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
David Abrahams skrev:
on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type. Maybe if it isn't, you could turn off whatever you're doing with enable_if or something. Why is iterator_range using the iterator's operator[], anyway?
It seems very natural to be able to index a random access range, doesn't it.
No comment.
And how else would you implement that?
The exact same way we implement it in iterator_facade. http://www.boost.org/doc/libs/1_37_0/libs/iterator/doc/iterator_facade.html#...
Anyway, I guess I could do that check. Is it illegal for the iterators reference type to be a non-reference?
No, but in C++03 that makes them input iterators or output iterators, but not forward iterators. -- Dave Abrahams BoostPro Computing http://www.boostpro.com

On Thu, Nov 13, 2008 at 2:25 AM, David Abrahams <dave@boostpro.com> wrote:
on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
David Abrahams skrev:
on Wed Nov 12 2008, Thorsten Ottosen <thorsten.ottosen-AT-dezide.com> wrote:
Do you mean that I can detect the return type of operator[] at compile-time and do correct forwarding?
No, I mean you can detect when the return type of operator[] is the same as the reference type. Maybe if it isn't, you could turn off whatever you're doing with enable_if or something. Why is iterator_range using the iterator's operator[], anyway?
It seems very natural to be able to index a random access range, doesn't it.
No comment.
And how else would you implement that?
The exact same way we implement it in iterator_facade.
http://www.boost.org/doc/libs/1_37_0/libs/iterator/doc/iterator_facade.html#...
I have the same issue in RangeEx. I would like to implement your suggestion. Your code looks so fit for purpose that perhaps we should take operator_brackets_proxy et al. out of the detail namespace and make it common to iterator_facade and to iterator_range? I'm concerned I might be being a little slow and that you are suggesting the use of already available public boost APIs.
Anyway, I guess I could do that check. Is it illegal for the iterators reference type to be a non-reference?
No, but in C++03 that makes them input iterators or output iterators, but not forward iterators.
I would appreciate a further guidance. Thank you in anticipation for your patience. Neil Groves

on Thu Nov 13 2008, "Neil Groves" <neil-AT-grovescomputing.com> wrote:
And how else would you implement that?
The exact same way we implement it in iterator_facade.
http://www.boost.org/doc/libs/1_37_0/libs/iterator/doc/iterator_facade.html#operator> I have the same issue in RangeEx. I would like to implement your suggestion. Your code looks so fit for purpose that perhaps we should take operator_brackets_proxy et al. out of the detail namespace and make it common to iterator_facade and to iterator_range?
Sounds fine to me.
Is it illegal for the iterators reference type to be a non-reference?
No, but in C++03 that makes them input iterators or output iterators, but not forward iterators.
I would appreciate a further guidance. Thank you in anticipation for your patience.
*p returning T& is in the forward iterator requirements table. -- Dave Abrahams BoostPro Computing http://www.boostpro.com

Neil Groves wrote:
I have the same issue in RangeEx. I would like to implement your suggestion. Your code looks so fit for purpose that perhaps we should take operator_brackets_proxy et al. out of the detail namespace and make it common to iterator_facade and to iterator_range? I'm concerned I might be being a little slow and that you are suggesting the use of already available public boost APIs.
While the proxy technique works fine, wouldn't using decltype or Boost.Typeof just solve the issue also, with less opportunities for abstraction overhead?
participants (6)
-
David Abrahams
-
John Reid
-
Mathias Gaunard
-
Neil Groves
-
Steven Watanabe
-
Thorsten Ottosen