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.

[Chandler Carruth]

FWIW, I think I actually prefer defining the range in terms of the
iterators rather than vice-versa... At least, until the non-range variants
go away.

On Fri Mar 07 2014 at 4:37:09 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Mar 7, 2014 at 4:18 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> > On Fri, Mar 7, 2014 at 6:35 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> >> On Fri, Mar 7, 2014 at 2:43 PM, Aaron Ballman <aaron at aaronballman.com>
> >> wrote:
> >>>
> >>> 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),
> >>
> >>
> >> I thought the problem in the attributes case was that an accidental
> copy of
> >> the collection of attributes was being made? Or does this still break
> with
> >> that fixed?
> >
> > It was still breaking with that fixed, once upon a time. Now, not as
> confident.
> >
> >>
> >>> 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).
> >>
> >>
> >> Does this happen with multiple compilers, or is it an MSVC-specific
> >> miscompile?
> >
> > I'm starting to doubt my sanity. I just locally reverted this commit,
> > applied my enums patch, and everything passes whereas it was
> > consistently failing before.
> >
> > Let's pretend I've got a goblin (I should probably reboot this
> > machine, it's been quite a while and I've been taxing it lately). Do
> > you (or David) see any problems with the code as it was written prior
> > to this commit? I originally thought the code was fine, and only
> > rolled it back to the state it's currently in because of local worries
> > that don't seem so valid now. I'd like to go with a second set of eyes
> > to reduce any further churn.
> >
> > Assuming that you and/or David don't see anything wrong with the code
> > I reverted, I'll re-revert this as being a valid pattern and chalk
> > this up to solar flares or some such.  :-/
>
> Yeah, I certainly can't see any fault with the implementation as it
> was before you reverted it - hence my surprise at the revert.
>
> - David
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140308/63be441c/attachment.html>