Discussion:
DR 900 - incomplete rationale, please reconsider fixing it
(too old to reply)
Daniel Frey
2016-03-03 21:31:10 UTC
Permalink
In the last years, several people came up with code like this:

namespace detail {
template< class C >
struct reverse_range
{
explicit reverse_range (C& _c) : c(_c) {}
auto begin () { using std::rbegin; return rbegin(c); }
auto end () { using std::rend; return rend(c); }
private:
C& c;
};
}


template< class C >
auto reverse (C& c)
{
return detail::reverse_range<C>{c};
}

in an attempt to allow:

// some function
std::vector<int> foo();

// correct usage
auto v = foo();
for( auto i : reverse(v) ) { std::cout << i << std::endl; }

// problematic usage
for( auto i : reverse(foo()) ) { std::cout << i << std::endl; }

The reason is that the temporary returned by foo() is destructed *before* the loop starts executing.

As far as I can tell DR 900 was supposed to ask about that, but the rationale for the rejection only considers the top-level temporary.

I think it is therefore justified to reconsider changing the range-based for loop to treat the init-expression like a function argument: All temporaries of the expression should only be destructed *after* the whole for-loop was executed. This also removes the only place where binding a temporary to a reference to extend its lifetime happens implicitly and thereby unseen to the user. (AFAIK)
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Jens Maurer
2016-03-04 05:20:12 UTC
Permalink
Post by Daniel Frey
As far as I can tell DR 900 was supposed to ask about that, but the rationale for the rejection only considers the top-level temporary.
I think it is therefore justified to reconsider changing the range-based for loop to treat the init-expression like a function argument: All temporaries of the expression should only be destructed *after* the whole for-loop was executed. This also removes the only place where binding a temporary to a reference to extend its lifetime happens implicitly and thereby unseen to the user. (AFAIK)
Core issue 900 will be re-openend to consider this case.

Thanks,
Jens
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
S.B.
2016-03-04 09:39:36 UTC
Permalink
See also EWG 120 <http://wg21.link/ewg120>.
Post by Daniel Frey
namespace detail {
template< class C >
struct reverse_range
{
explicit reverse_range (C& _c) : c(_c) {}
auto begin () { using std::rbegin; return rbegin(c); }
auto end () { using std::rend; return rend(c); }
C& c;
};
}
template< class C >
auto reverse (C& c)
{
return detail::reverse_range<C>{c};
}
// some function
std::vector<int> foo();
// correct usage
auto v = foo();
for( auto i : reverse(v) ) { std::cout << i << std::endl; }
// problematic usage
for( auto i : reverse(foo()) ) { std::cout << i << std::endl; }
The reason is that the temporary returned by foo() is destructed *before*
the loop starts executing.
As far as I can tell DR 900 was supposed to ask about that, but the
rationale for the rejection only considers the top-level temporary.
I think it is therefore justified to reconsider changing the range-based
for loop to treat the init-expression like a function argument: All
temporaries of the expression should only be destructed *after* the whole
for-loop was executed. This also removes the only place where binding a
temporary to a reference to extend its lifetime happens implicitly and
thereby unseen to the user. (AFAIK)
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-04 15:58:40 UTC
Permalink
See also EWG 120.
Interesting. Well, the obvious attempt to fix the definition of the range-based for-loop would be to replace

{
auto && __range = range-init;
for ( auto __begin = begin-expr,
__end = end-expr;
__begin != __end;
++__begin ) {
for-range-declaration = *__begin;
statement
}
}

with something like this:

[] (auto && __range) {
for ( auto __begin = begin-expr,
__end = end-expr;
__begin != __end;
++__begin ) {
for-range-declaration = *__begin;
statement
}
}( range-init );

but I think this could lead to problems when statement would contain a goto, couldn’t it?
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-04 16:14:24 UTC
Permalink
I fully believe that this is a problem which is the range adaptor's
responsibility to support. It is relatively trivial to store the underlying
container by value if it is a temporary or by reference if it is not a
temporary.

If we simply change the range-for loop, then
auto v = reverse(foo())
for (auto& elem : v)

would still not be supported.

Such a difference would be very surprising to me, and I think to others as
well.
See also EWG 120.
Interesting. Well, the obvious attempt to fix the definition of the
range-based for-loop would be to replace
{
auto && __range = *range-init*;
for ( auto __begin = *begin-expr*,
__end = *end-expr*;
__begin != __end;
++__begin ) {
*for-range-declaration* = *__begin;
*statement*
}
}
[] (auto && __range) {
for ( auto __begin = *begin-expr*,
__end = *end-expr*;
__begin != __end;
++__begin ) {
*for-range-declaration* = *__begin;
*statement*
}
}( *range-init* );
but I think this could lead to problems when statement would contain a
goto, couldn’t it?
--
---
You received this message because you are subscribed to the Google Groups
"ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an
Visit this group at
https://groups.google.com/a/isocpp.org/group/std-discussion/.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2016-03-04 16:35:57 UTC
Permalink
Post by Brent Friedman
I fully believe that this is a problem which is the range adaptor's
responsibility to support. It is relatively trivial to store the underlying
container by value if it is a temporary or by reference if it is not a
temporary.
How is that "relatively trivial"? It will either require that the
(template) range adaptor have enough storage space to store the value (thus
making them quite large even if they don't use that space), or they have to
allocate memory dynamically to store the value (thus making them allocate
memory).

Neither option is a good idea. This is a general problem in C++, and thus
far, we have no general solution to it. And with range adapters, this
general problem will only get *worse*.

If we simply change the range-for loop, then
Post by Brent Friedman
auto v = reverse(foo())
for (auto& elem : v)
would still not be supported.
Such a difference would be very surprising to me, and I think to others as
well.
All that means is that we need a general solution to the general problem.
We already know that.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-04 16:55:07 UTC
Permalink
[This is a reply to Brent Friedman, but I haven’t received your answer by email and I don’t have a Google+ account because the “privacy” policy is the exact opposite of privacy IMO.]

You wrote:

"I fully believe that this is a problem which is the range adaptor's responsibility to support. It is relatively trivial to store the underlying container by value if it is a temporary or by reference if it is not a temporary.”

I don’t think it is trivial (even if the code looks simple, it is inefficient). Also, while you are correct that the adaptors could be misused, I think that you are addressing a different problem. I was just providing one example, others do exist. The point I’m trying to address and which is also addressed by CWG 900, CWG 1498 (and EWG 120) is that the expression used in a range-based for-loop is expected my most people to be similar to a parameter passed to a function, where the function ends when the for-loop ends. Fixing this will prevent a bunch of real-world bugs as well as allow for some additional use-cases.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-05 15:08:30 UTC
Permalink
Post by Nicol Bolas
How is that "relatively trivial"? It will either require that the
(template) range adaptor have enough storage space to store the value (thus
making them quite large even if they don't use that space), or they have to
allocate memory dynamically to store the value (thus making them allocate
memory).
Post by Nicol Bolas
I don’t think it is trivial (even if the code looks simple, it is
inefficient).

Here's how I implemented it in our range library.
Note that this is written to work under VS2012 so there's a little bit of
hokiness in the implementation and some more modern features aren't used.

First, we have a class called holder which implements the lifetime
extension policy.
template<class T>
struct holder
{
typedef typename std::remove_reference<T>::type noref;
typedef typename std::conditional< std::is_rvalue_reference<T>::value,
noref, noref& >::type type;
};

type is either an lvalue reference or a value, based on if T is an rvalue
reference. This could probably be nailed down a little bit more by using
T&& in the is_rvalue_reference. The way we use it though, this has worked
fine.

The range adapter uses a holder<T>::type, which has been perfectly
forwarded from the call site all the way up into holder.

namespace detail
{
namespace reverse
{
template<class TAIL, class TAILIT>
struct tag
{
* typename holder<TAIL>::type Tail;*

typedef std::reverse_iterator<TAILIT> iterator_t;

template<class TAILX>
FTag(TAILX&& inRange)
:Tail(std::forward<TAILX>(inRange))
{}

iterator_t begin()
{
return iterator_t( std::end(Tail) );
}
iterator_t end()
{
return iterator_t( std::begin(Tail) );
}
};
}
}

template<class R>
inline auto reverse(R&& Range) ->
decltype(detail::reverse::FTag<decltype(std::forward<R>(Range)),
decltype(std::begin(Range))>(std::forward<R>(Range)))
{
return detail::reverse::tag<decltype(std::forward<R>(Range)),
decltype(std::begin(Range))>(std::forward<R>(Range));
}

So reverse( T& ) creates a different data structure than reverse( T&& ). No
dynamic memory allocation, and the structure is only as big as is necessary
for that particular invocation.

Am I missing something?
Post by Nicol Bolas
[This is a reply to Brent Friedman, but I haven’t received your answer by
email and I don’t have a Google+ account because the “privacy” policy is
the exact opposite of privacy IMO.]
"I fully believe that this is a problem which is the range adaptor's
responsibility to support. It is relatively trivial to store the underlying
container by value if it is a temporary or by reference if it is not a
temporary.”
I don’t think it is trivial (even if the code looks simple, it is
inefficient). Also, while you are correct that the adaptors could be
misused, I think that you are addressing a different problem. I was just
providing one example, others do exist. The point I’m trying to address and
which is also addressed by CWG 900, CWG 1498 (and EWG 120) is that the
expression used in a range-based for-loop is expected my most people to be
similar to a parameter passed to a function, where the function ends when
the for-loop ends. Fixing this will prevent a bunch of real-world bugs as
well as allow for some additional use-cases.
--
---
You received this message because you are subscribed to the Google Groups
"ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an
Visit this group at
https://groups.google.com/a/isocpp.org/group/std-discussion/.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-05 15:12:32 UTC
Permalink
Post by Brent Friedman
FTag(TAILX&& inRange)
detail::reverse::FTag<decltype(std::fo
The "FTag"s are supposed to read "tag". Sorry about that, was trying to
convert from our naming conventions to something more readable.
Post by Brent Friedman
Post by Nicol Bolas
How is that "relatively trivial"? It will either require that the
(template) range adaptor have enough storage space to store the value (thus
making them quite large even if they don't use that space), or they have to
allocate memory dynamically to store the value (thus making them allocate
memory).
Post by Nicol Bolas
I don’t think it is trivial (even if the code looks simple, it is
inefficient).
Here's how I implemented it in our range library.
Note that this is written to work under VS2012 so there's a little bit of
hokiness in the implementation and some more modern features aren't used.
First, we have a class called holder which implements the lifetime
extension policy.
template<class T>
struct holder
{
typedef typename std::remove_reference<T>::type noref;
typedef typename std::conditional< std::is_rvalue_reference<T>::value,
noref, noref& >::type type;
};
type is either an lvalue reference or a value, based on if T is an rvalue
reference. This could probably be nailed down a little bit more by using
T&& in the is_rvalue_reference. The way we use it though, this has worked
fine.
The range adapter uses a holder<T>::type, which has been perfectly
forwarded from the call site all the way up into holder.
namespace detail
{
namespace reverse
{
template<class TAIL, class TAILIT>
struct tag
{
* typename holder<TAIL>::type Tail;*
typedef std::reverse_iterator<TAILIT> iterator_t;
template<class TAILX>
FTag(TAILX&& inRange)
:Tail(std::forward<TAILX>(inRange))
{}
iterator_t begin()
{
return iterator_t( std::end(Tail) );
}
iterator_t end()
{
return iterator_t( std::begin(Tail) );
}
};
}
}
template<class R>
inline auto reverse(R&& Range) ->
decltype(detail::reverse::FTag<decltype(std::forward<R>(Range)),
decltype(std::begin(Range))>(std::forward<R>(Range)))
{
return detail::reverse::tag<decltype(std::forward<R>(Range)),
decltype(std::begin(Range))>(std::forward<R>(Range));
}
So reverse( T& ) creates a different data structure than reverse( T&& ).
No dynamic memory allocation, and the structure is only as big as is
necessary for that particular invocation.
Am I missing something?
Post by Nicol Bolas
[This is a reply to Brent Friedman, but I haven’t received your answer by
email and I don’t have a Google+ account because the “privacy” policy is
the exact opposite of privacy IMO.]
"I fully believe that this is a problem which is the range adaptor's
responsibility to support. It is relatively trivial to store the underlying
container by value if it is a temporary or by reference if it is not a
temporary.”
I don’t think it is trivial (even if the code looks simple, it is
inefficient). Also, while you are correct that the adaptors could be
misused, I think that you are addressing a different problem. I was just
providing one example, others do exist. The point I’m trying to address and
which is also addressed by CWG 900, CWG 1498 (and EWG 120) is that the
expression used in a range-based for-loop is expected my most people to be
similar to a parameter passed to a function, where the function ends when
the for-loop ends. Fixing this will prevent a bunch of real-world bugs as
well as allow for some additional use-cases.
--
---
You received this message because you are subscribed to the Google Groups
"ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an
Visit this group at
https://groups.google.com/a/isocpp.org/group/std-discussion/.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-05 16:08:04 UTC
Permalink
Post by Brent Friedman
Here's how I implemented it in our range library.
[...]
Am I missing something?
This is still inefficient when the type is expensive to move. Plus, as I said, it is only an example, other cases exist. It was meant to show the expectation people have. This discussion is not about how to write a range adapter, it is about whether or not a range-based for loop should treat nested temporaries in the expression like it is initializing a variable (which is does internally, but that is invisible to the user!) or more like a function parameter (when thinking that the whole range-based for loop is one big expression wrt it’s argument). Most people seem to expect the latter and are surprised to find out that the nested temporaries of the expression are deleted before the loop executes, not after.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-05 16:36:12 UTC
Permalink
@BrentFriedman: Please use Reply-All so I receive your answer and can
reply to it directly.

Sorry about this, gmail doesn't provide me with a reply-to-all option. I'll
try to remember to manually add you to my replies.
This is still inefficient when the type is expensive to move.
I agree this could be an issue, but if your type is expensive to move then
moving it or returning it from a function is a bit dubious in the first
place. No doubt, being able to avoid unnecessary moves is a good thing. The
rationale offered in CWG 900 and in your original post doesn't mention my
alternative. If the cost of move operations is our rationale for
implementing this change, then let's make that explicit. All that has been
mentioned up to now is the danger of destroyed subexpressions, which I have
demonstrated is resolvable in the library.
This discussion is not about how to write a range adapter, it is about
whether or not a range-based for loop should treat nested temporaries
in the expression like it is initializing a variable (which is does
internally, but that is invisible to the user!) or more like a function
parameter
(when thinking that the whole range-based for loop is one big expression
wrt it’s argument). Most people seem to expect the latter and are
surprised to find out that the nested temporaries of the expression are
deleted before the loop executes, not after.

The provided use cases in your original post and in CWG 900 refers to range
adapters. Is there another use case which cannot be resolved using this
library-based lifetime extension technique? Usually, library solutions to
proposed language features are not considered off topic.

I'm not saying we absolutely can't do it, but we need to evaluate
alternatives and do it for the right reasons.
@BrentFriedman: Please use Reply-All so I receive your answer and can
Post by Brent Friedman
Here's how I implemented it in our range library.
[...]
Am I missing something?
This is still inefficient when the type is expensive to move. Plus, as I
said, it is only an example, other cases exist. It was meant to show the
expectation people have. This discussion is not about how to write a range
adapter, it is about whether or not a range-based for loop should treat
nested temporaries in the expression like it is initializing a variable
(which is does internally, but that is invisible to the user!) or more like
a function parameter (when thinking that the whole range-based for loop is
one big expression wrt it’s argument). Most people seem to expect the
latter and are surprised to find out that the nested temporaries of the
expression are deleted before the loop executes, not after.
--
---
You received this message because you are subscribed to the Google Groups
"ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an
Visit this group at
https://groups.google.com/a/isocpp.org/group/std-discussion/.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-05 17:01:52 UTC
Permalink
Post by Daniel Frey
This is still inefficient when the type is expensive to move.
I agree this could be an issue, but if your type is expensive to move then moving it or returning it from a function is a bit dubious in the first place. No doubt, being able to avoid unnecessary moves is a good thing. The rationale offered in CWG 900 and in your original post doesn't mention my alternative. If the cost of move operations is our rationale for implementing this change, then let's make that explicit. All that has been mentioned up to now is the danger of destroyed subexpressions, which I have demonstrated is resolvable in the library.
RVO/NRVO allow pretty efficient returning of such types from functions, no move necessary. My use case is about an expensive-to-move type that is a matrix with compile-time size, e.g., matrix<double,4,4>. The type should contain the double values directly and work without heap-allocations (otherwise the move becomes cheap, but at a price). Now apply operators to it and you could pass-through rvalue-references in expressions like this:

using M = matrix<double,4,4>; // or imaging 100x100 :)

M a,b,c,d,e; // initialized somehow

M r = a + b + c + d + e;

With rvalue-reference pass-through, my library creates one temporary for a+b, then applies +=c to that temporary, then +=d, then +=e and returns the rvalue reference which is then moved into r; Only after the whole expression was evaluated, the temporary created by a+b is destroyed.

Now let’s say the library allows iteration over the matrix, returning a vector of each row as elements. One would use

for( auto& e : a+b+c+d+e ) { 
 }

Here it would be extremely hard to explain to the user why this is wrong. In fact, it’s the single pain-point of my Operators library (https://github.com/taocpp/operators) that people were concerned about. All other places in the language will work, only range-based for loops to silent reference binding.

I think a similar argument could also be made for std::string when adding multiple strings. And possibly other standard types. With the small-string-optimization it is not as cheap to move a string as you might which and I seem to remember that there were actually discussions about pass-through of rvalue-references in that case, not sure why it was dropped though.
The provided use cases in your original post and in CWG 900 refers to range adapters. Is there another use case which cannot be resolved using this library-based lifetime extension technique? Usually, library solutions to proposed language features are not considered off topic.
I agree that a pure library solution is usually preferable, but when the library solutions leads to inefficient code or massively increased compile-times (like when using expression templates to solve the above problem for matrix classes), I think that looking into small changes in the language is acceptable.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-05 20:02:36 UTC
Permalink
Usually, library solutions to proposed language features are not considered off topic.
This, BTW, makes me wonder why we have range-based for loops at all (yes, because they predate generalized lambdas). The “goto”-issue aside, the following (plus some omitted specializations for c.begin(), arrays...) provides almost the same functionality; plus it treats “c" (the expression) as a real parameter. Somehow, I think this is also something that people might have in mind when reasoning about the lifetime of the expression “c”. They might see the real range-based for loop as something similar to for_each
 YMMV. :)

#include <iostream>
#include <vector>
#include <algorithm>

template< typename C, typename F >
void for_each( C && c, F f )
{
using std::begin;
using std::end;
const auto e = end( c );
for( auto it = begin( c ); it != e; ++it ) {
f( *it );
}
}

int main()
{
std::vector< int > c = { 1, 2, 3, 4, 5, 6, 8, 10, 42, 100, 1000, 1234 };

// range-based
for( auto && e : c ) { std::cout << e << std::endl; };

// function template + lambda
for_each( c, []( auto && e ) { std::cout << e << std::endl; } );
}
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Nevin Liber
2016-03-05 20:56:28 UTC
Permalink
Post by Brent Friedman
Usually, library solutions to proposed language features are not
considered off topic.
This, BTW, makes me wonder why we have range-based for loops at all (yes,
because they predate generalized lambdas).
While I wasn't around for that decision, I don't think that was the
reason. Range-based for both has better syntax for a very common use case
and is easier to reason about than the full general for loop because you
cannot get to the iterator.

IMO, std::for_each is still easier to reason about because it is even more
restrictive: exceptions notwithstanding, you will visit every element in
the range in a for_each. The cost is that it is much more syntax heavy
than range-based for.
--
Nevin ":-)" Liber <mailto:***@eviloverlord.com> +1-847-691-1404
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-05 20:58:07 UTC
Permalink
Post by Daniel Frey
Somehow, I think this is also something that people might have in mind
when reasoning about the lifetime of the expression “c”. They might see the
real range-based for loop as something similar to for_each
 YMMV. :)

I don't think this problem is unique to the range for loop. It can also be
experienced with if statements.

consider:

vector<int> foo();

//let us assume that the reverse range is hypothetically convertible to
bool, for "not empty"
if ( auto range = reverse( foo() ) )
{
do_work( range ); //our vector<int> has been destroyed :(
}


On the other hand, an "if" algorithm could maintain the lifetime of foo()
until the end of the full expression.

call_if( reverse(foo()), &do_work);


I think if, while, do-while, switch, for, and range-for are all affected.
If there's an argument to be made here, I feel like all of these are
affected. range-for just happens to be a particularly interesting use case.
Post by Daniel Frey
Usually, library solutions to proposed language features are not
considered off topic.
This, BTW, makes me wonder why we have range-based for loops at all (yes,
because they predate generalized lambdas). The “goto”-issue aside, the
following (plus some omitted specializations for c.begin(), arrays...)
provides almost the same functionality; plus it treats “c" (the expression)
as a real parameter. Somehow, I think this is also something that people
might have in mind when reasoning about the lifetime of the expression “c”.
They might see the real range-based for loop as something similar to
for_each
 YMMV. :)
#include <iostream>
#include <vector>
#include <algorithm>
template< typename C, typename F >
void for_each( C && c, F f )
{
using std::begin;
using std::end;
const auto e = end( c );
for( auto it = begin( c ); it != e; ++it ) {
f( *it );
}
}
int main()
{
std::vector< int > c = { 1, 2, 3, 4, 5, 6, 8, 10, 42, 100, 1000, 1234 };
// range-based
for( auto && e : c ) { std::cout << e << std::endl; };
// function template + lambda
for_each( c, []( auto && e ) { std::cout << e << std::endl; } );
}
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-05 21:05:53 UTC
Permalink
Post by Daniel Frey
Somehow, I think this is also something that people might have in mind when reasoning about the lifetime of the expression “c”. They might see the real range-based for loop as something similar to for_each
 YMMV. :)
I don't think this problem is unique to the range for loop. It can also be experienced with if statements.
vector<int> foo();
//let us assume that the reverse range is hypothetically convertible to bool, for "not empty"
if ( auto range = reverse( foo() ) )
{
do_work( range ); //our vector<int> has been destroyed :(
}
On the other hand, an "if" algorithm could maintain the lifetime of foo() until the end of the full expression.
There is a key difference: In case of “if”, “while”, etc. you use the “auto x = 
” syntax which is a normal variable declaration. Even for “for( 
; 
; 
 )” this holds true. The user can explicitly see (and to some extend control) what is going on. Only the range-based for loop is different as the variable declaration happens implicitly. It’s not like you are writing “for( auto&& e : auto && range = reverse( foo() ) ) { 
 }”.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Brent Friedman
2016-03-05 21:21:59 UTC
Permalink
Post by Daniel Frey
There is a key difference: In case of “if”, “while”, etc. you use the
“auto x = 
” syntax which is a normal variable declaration. Even for “for(

; 
; 
 )” this holds true. The user can explicitly see (and to some extend
control) what is going on. Only the range-based for loop is different as
the variable declaration happens implicitly. It’s not like you are writing
“for( auto&& e : auto && range = reverse( foo() ) ) { 
 }”.

Maybe others feel differently, but the extra auto used with if doesn't
personally impact my mental model. I'm just declaring some data that should
exist for the duration of the scope.
Post by Daniel Frey
Post by Brent Friedman
Post by Daniel Frey
Somehow, I think this is also something that people might have in mind
when reasoning about the lifetime of the expression “c”. They might see the
real range-based for loop as something similar to for_each
 YMMV. :)
Post by Brent Friedman
I don't think this problem is unique to the range for loop. It can also
be experienced with if statements.
Post by Brent Friedman
vector<int> foo();
//let us assume that the reverse range is hypothetically convertible to
bool, for "not empty"
Post by Brent Friedman
if ( auto range = reverse( foo() ) )
{
do_work( range ); //our vector<int> has been destroyed :(
}
On the other hand, an "if" algorithm could maintain the lifetime of
foo() until the end of the full expression.
There is a key difference: In case of “if”, “while”, etc. you use the
“auto x = 
” syntax which is a normal variable declaration. Even for “for(

; 
; 
 )” this holds true. The user can explicitly see (and to some extend
control) what is going on. Only the range-based for loop is different as
the variable declaration happens implicitly. It’s not like you are writing
“for( auto&& e : auto && range = reverse( foo() ) ) { 
 }”.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2016-03-06 00:31:13 UTC
Permalink
Post by Daniel Frey
Post by Brent Friedman
Post by Daniel Frey
Somehow, I think this is also something that people might have in mind
when reasoning about the lifetime of the expression “c”. They might see the
real range-based for loop as something similar to for_each
 YMMV. :)
Post by Brent Friedman
I don't think this problem is unique to the range for loop. It can also
be experienced with if statements.
Post by Brent Friedman
vector<int> foo();
//let us assume that the reverse range is hypothetically convertible to
bool, for "not empty"
Post by Brent Friedman
if ( auto range = reverse( foo() ) )
{
do_work( range ); //our vector<int> has been destroyed :(
}
On the other hand, an "if" algorithm could maintain the lifetime of
foo() until the end of the full expression.
There is a key difference: In case of “if”, “while”, etc. you use the
“auto x = 
” syntax which is a normal variable declaration. Even for “for(

; 
; 
 )” this holds true. The user can explicitly see (and to some extend
control) what is going on. Only the range-based for loop is different as
the variable declaration happens implicitly. It’s not like you are writing
“for( auto&& e : auto && range = reverse( foo() ) ) { 
 }”.
I just don't see why it matters. Whether you're seeing an explicit variable
declaration or not, you have an expression who's components need to survive
that expression.

Users who know about the pitfalls of subexpressions will know to be
careful, whether they're using `auto x = ...` or `for(auto x : ...)`. And
users who don't know about the pitfalls will... fall into those pits.
Again, regardless of which form they take.

That's why I say that we need a *generalized* solution to the problem, not
merely to fix it in one place. Indeed, fixing it in one place creates more
confusion than it solves. If someone sees:

for(auto x: some_expression)

They will naturally think that this is equally valid:

auto rng = some_expression;

And it isn't.

Either solve it everywhere, or solve it nowhere. Don't solve it for one
specific case.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-06 07:20:20 UTC
Permalink
I just don't see why it matters. Whether you're seeing an explicit variable declaration or not, you have an expression who's components need to survive that expression.
With my use-case it makes all the difference. It’s not just about being visible, but also about being controllable! If you write

if( auto x = a+b+c+d+e ) { 
 }

Then you create an instance “x”, not just a bound reference. That is OK and works just fine. If you’d write

if( const auto& x = a+b+c+d+e ) { 
 }

you would end up with a dangling reference. This is why the range-based for loop is not the same as the other cases, it creates an implicit reference that I neither see, nor can I control it.
for(auto x: some_expression)
auto rng = some_expression;
And it isn’t.
It is, in my case. “auto x=a+b+c+d+e” is fine, only “const auto& x=a+b+c+d+e” or (even worse) “auto&& x=a+b+c+d+e” is not. But if “auto x=a+b+c+d+e” is valid, then why should “for(const auto& v : a+b+c+d+e)” not be valid?
Either solve it everywhere, or solve it nowhere. Don't solve it for one specific case.
I wouldn’t mind to consider extending the lifetime of nested temporaries for “if( auto x = expression ) { 
 }” either, but I’m not sure that this is needed. I still think that the range-based for loop is a special case and should be considered separately.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2016-03-06 14:25:21 UTC
Permalink
Post by Nicol Bolas
I just don't see why it matters. Whether you're seeing an explicit
variable declaration or not, you have an expression who's components need
to survive that expression.
With my use-case it makes all the difference. It’s not just about being
visible, but also about being controllable! If you write
if( auto x = a+b+c+d+e ) { 
 }
Then you create an instance “x”, not just a bound reference. That is OK
and works just fine. If you’d write
if( const auto& x = a+b+c+d+e ) { 
 }
you would end up with a dangling reference.
... where?

If `x` stores references to any of the intermediaries that the expression
generates, you have a dangling reference in *both cases*. And if `x` does
not store references to intermediaries of the expression... then both
pieces of code are *perfectly fine*. In the second case, `x` will be a
reference who's lifetime is extended.

There are either dangling references in both of them or dangling references
in *none* of them. There is no instances where the latter will break that
the former will not.

I think you have become someone confused on how the dangling reference
problem happens. In your OP, you gave this example:

for( auto i : reverse(foo()) )

This does not fail because range-based `for` happens to use `auto&&`
syntax. It fails because `reverse` returns an object that stores a
reference to a temporary. The reference that range-based `for` stores is
not dangling; the dangling reference is what gets stored *within* the
object that range-based `for` stores.

auto x = reverse(foo());

Is just as broken as the range-based for example.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-06 21:59:07 UTC
Permalink
Post by Daniel Frey
With my use-case it makes all the difference. It’s not just about being visible, but also about being controllable! If you write
if( auto x = a+b+c+d+e ) { 
 }
Then you create an instance “x”, not just a bound reference. That is OK and works just fine. If you’d write
if( const auto& x = a+b+c+d+e ) { 
 }
you would end up with a dangling reference.
... where?
If `x` stores references to any of the intermediaries that the expression generates, you have a dangling reference in both cases. And if `x` does not store references to intermediaries of the expression... then both pieces of code are perfectly fine. In the second case, `x` will be a reference who's lifetime is extended.
There are either dangling references in both of them or dangling references in none of them. There is no instances where the latter will break that the former will not.
No. The first case is OK. If a+b+c+d+e returns T&&, auto deduces the type for x to T, not T&&. The returned reference (from the nested temporary generated by a+b) is then moved into the instance x.
I don’t think I’m confused :)
Post by Daniel Frey
for( auto i : reverse(foo()) )
This does not fail because range-based `for` happens to use `auto&&` syntax. It fails because `reverse` returns an object that stores a reference to a temporary. The reference that range-based `for` stores is not dangling; the dangling reference is what gets stored within the object that range-based `for` stores.
auto x = reverse(foo());
Is just as broken as the range-based for example.
It is a question of where the full expression ends that a temporary is part of. Until the end of a full expression, a temporary automatically lives on. The original example of a range adapter was just used because it is very common. It just shows that people consider the while loop to be the “full expression” wrt the expression given after the colon of the range-based for loop. And this can be useful. Range-adaptors might be useful as given in the original example if the lifetime of the expression is changes. It would be similar to iterators, where you have some invalidation rules. Hence the “for( auto i : reverse(foo()) )” would be fine while “auto x = reverse(foo())” is not. Unless you want to tell me that iterators and basically the whole STL is broken because I can write “auto it = foo().begin();” :)

Anyways, range-adaptors are not the use-case I have in mind. More like matrix classes or, this might be easier for now, consider this example: std::string.

Imagine the following set of operators:

string& string::operator+=( const string& s );

string operator+( const string& lhs, const string& rhs ) { string nrv{lhs}; nrv+=rhs; return nrv; }
string&& operator+( string&& lhs, const string& rhs ) { lhs+=rhs; return move(lhs); }

Now call it like (omitting const char* to string):

string s = “Hello”;
auto s2 = s + “,” + “ “ + “world” + “!”;

The above yields a perfectly fine string. Creating a copy of s as a temporary on the first +, then +=“,”, then +=“ “, then +=“world”, then +=“!”. Finally, move the temporary to s2.

The same is OK for function calls:

void f(string s);
f(s + “,” + “ “ + “world” + “!”);

Again, while f is evaluated, the temporary still exists. You only have a problem if you explicitly bind the result to a reference:

const auto& s3 = s + “,” + “ “ + “world” + “!”; // dangling reference

or, and this is the whole point, if you are using it in a range-based for loop - the only place in the language where you can not see or control the implicit binding of the reference and which appears to be contrary to what people expect:

for( auto c : s + “,” + “ “ + “world” + “!” ) { 
handle character c
 }

I don’t mind that the s3 above is broken, all I care about is the line above: This should be fixed to be more like the function call and its parameters.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Nicol Bolas
2016-03-07 02:49:55 UTC
Permalink
Post by Nicol Bolas
Post by Daniel Frey
for( auto i : reverse(foo()) )
This does not fail because range-based `for` happens to use `auto&&`
syntax. It fails because `reverse` returns an object that stores a
reference to a temporary. The reference that range-based `for` stores is
not dangling; the dangling reference is what gets stored within the object
that range-based `for` stores.
Post by Daniel Frey
auto x = reverse(foo());
Is just as broken as the range-based for example.
It is a question of where the full expression ends that a temporary is
part of. Until the end of a full expression, a temporary automatically
lives on. The original example of a range adapter was just used because it
is very common. It just shows that people consider the while loop to be the
“full expression” wrt the expression given after the colon of the
range-based for loop. And this can be useful. Range-adaptors might be
useful as given in the original example if the lifetime of the expression
is changes. It would be similar to iterators, where you have some
invalidation rules. Hence the “for( auto i : reverse(foo()) )” would be
fine while “auto x = reverse(foo())” is not.
And that's why I'm against it. It would be incredibly surprising to most
users for one of those to be fine and the other not. If you know that `auto
x = reverse(foo())` is problematic, then you also know that you can't use
it in a range-based for loop. Or in a while loop. Or whever else. It's
*always* problematic.

Whereas if you *don't* know that `reverse(foo())` can be problematic, and
you see `for(auto i: reverse(foo()))` working just fine... you may get it
in your head that `auto x = reverse(foo())` will work too.

That would be a big inconsistency in the language.

Unless you want to tell me that iterators and basically the whole STL is
Post by Nicol Bolas
broken because I can write “auto it = foo().begin();” :)
... yes. But it's not iterators or STL. It is *C++ itself* that's broken,
because it is what gives us the dangling reference problem with no general
solution.
Post by Nicol Bolas
Anyways, range-adaptors are not the use-case I have in mind. More like
std::string.
string& string::operator+=( const string& s );
string operator+( const string& lhs, const string& rhs ) { string
nrv{lhs}; nrv+=rhs; return nrv; }
string&& operator+( string&& lhs, const string& rhs ) { lhs+=rhs; return move(lhs); }
You are merely proving my point for me. Why should this work:

const auto &s = str1 + str2;

and this not work:

const auto &s = "foo" + str2;

There is absolutely *no reason at all* for anyone unfamiliar with your
particular `operator+` gymnastics to believe that the second case is
actually a dangling reference. By the look of it, it ought to just work.

Indeed, the fact that your code would break #2 is precisely why *we don't
write that!* If we could write such code *without* breaking everyone who
does #2, then you'd see it being done more often. But such overloads break
perfectly valid and reasonable code.

Your "solution" solves this problem for exactly and only one case. There
are far too many other cases to just pretend that it's OK.

You only have a problem if you explicitly bind the result to a reference:
Oh, I get it. You are effectively arguing that explicitly binding things to
a reference is bad coding, and therefore everyone should stop doing it,
because otherwise your chicanery will be exposed for what it is. You're
basically wanting to change the one place that you *can't* stop someone
from binding to a reference, so that it will be safe for your code that
doesn't handle being bound to references.

Sorry, but I prefer actually solving the problem to wallpapering over it.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Daniel Frey
2016-03-07 16:01:08 UTC
Permalink
Oh, I get it. You are effectively arguing that explicitly binding things to a reference is bad coding, and therefore everyone should stop doing it, because otherwise your chicanery will be exposed for what it is. [...]
Sorry, but I prefer actually solving the problem to wallpapering over it.
I’m not going to continue the discussion in this tone.
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at https://groups.google.com/a/isocpp.org/group/std-discussion/.
Loading...