r203299 - In my tests, I'm finding that declaring iterators in terms of ranges can sometimes have dangerous side-effects where the range temporary is destroyed, taking the underlying iterators out with it.

[Aaron Ballman]

On Fri, Mar 7, 2014 at 5:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Mar 7, 2014 at 2:17 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> Author: aaronballman
>> Date: Fri Mar  7 16:17:20 2014
>> New Revision: 203299
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=203299&view=rev
>> Log:
>> In my tests, I'm finding that declaring iterators in terms of ranges can sometimes have dangerous side-effects where the range temporary is destroyed, taking the underlying iterators out with it.
>
> Um... what? do you have a concrete example because that doesn't sound
> right at all, at least not for iterator_range<T> which just holds
> copies of the iterator - are some of our iterators non-copyable (or
> not safely/sanely copyable in some way)? All the begin/end's return by
> value, so I don't see how they could be related to the copy that
> happens to be in the range temporary...

I've found this to be the case anywhere the iterator doesn't wind up
being a simple pointer. attributes (which I had to roll back), enums
(which I've yet to commit while trying to figure this out). In all
cases, the commonality is that it compiles fine, but exhibits
use-after-free behavior on test cases in some circumstances.

Regarding the attribute patch (which was committed this morning as
r203236 and reverted as r203237), Takumi found that these specific
changes were causing test cases to fail:
https://gist.github.com/chapuni/9412576

So I agree, I find is really surprising that it behaves this way, but
prefer to err on the side of caution at least while doing this initial
push. Once things are pretty well range-ified, then I intend to
revisit the problem (unless someone beats me to it).

~Aaron