<div dir="ltr">(+Chandler because I've talked to him a few times about range adapters and he's expressed hesitancy, I believe hoping to wait for some direction to come out of the C++ standards committee, but that was quite a while back - not sure what the state is now)<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 7, 2015 at 3:02 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all<br>
<br>
This patch adds a wrapper to iterators to allow us to use foreach loops, but also get the index of elements.  This allows a greater number of loops to use foreach, or avoid a foreach with a separate index.<br></blockquote><div><br>Usual caveat for range adapters (not a problem for /iterator/ adapters) is lifetime:<br><br>container func();<br><br>for (auto x : func()) // this is fine<br>  f(x);<br><br>for (auto x : wrap(func()) // this will invoke UB</div><div>  f(x)<br><br>the problem being that range-for uses reference-extension to extend the lifetime of the container, but if it's wrapped then only the lifetime of the wrapper will be extended. <br><br>This is the main thing holding up standardization of range adapters in C++, as I understand it (though I haven't followed closely).<br><br>Options include<br><br>1) disallow: add an overload of IndexedIterator that takes the container by rvalue reference, and = delete it<br>2) do nothing (the range adapter would still work for temporaries when passed to another function that does the work, rather than to a range-for loop or other reference-extension context: do_things(reverse(createSequence())); for example)<br>3) get really fancy and have range adapter that takes rvalue reference objects and moves them into the adapter itself to extend their lifetime - there's all sorts of interesting gotchas here, especially once you get around to composability of range adapters... <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I’ve updated a loop in AArch64A57FPLoadBalancing to show this.  The gist of the change is that the loop goes from<br>
<br>
  unsigned Idx = 0;<br>
  for (auto &MI : MBB)<br>
    scanInstruction(&MI, Idx++, ActiveChains, AllChains);<br>
<br>
to<br>
<br>
  for (auto MI : IndexedIterator(MBB))<br>
    scanInstruction(&*MI, MI.index(), ActiveChains, AllChains);<br>
<br>
So accesses to the iterator need a *, and there’s a new index() method.  I considered adding a .value() to the iterator instead of *.  I’m not strongly attached to one over the other (or both).<br></blockquote><div><br>Yeah, I'd probably make it a trivial struct with a "index" and "value" member (the value member would be a const ref - though this makes the "auto" versus "auto&" in the range-for a bit misleading... <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Comments welcome.  I personally like the idea wrapping iterators, but i understand if anyone is fundamentally opposed.  I’d really like to add another wrapper later which keeps track of the next pointer for you as then the current element can be safely deleted.<br></blockquote><div><br>Yeah, there's a bunch of range adapters that would be handy. Lang & I were talking about the desire for a "reverse" adapter. <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers,<br>
Pete<br>
<br>
<br><br>
<br>
<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>