<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 22, 2015, at 3:07 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Wed, Jul 22, 2015 at 12:11 PM, Duncan P. N. Exon Smith<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><span class=""><br class="">> On 2015-Jul-21, at 21:20, Pete Cooper <<a href="mailto:peter_cooper@apple.com" class="">peter_cooper@apple.com</a>> wrote:<br class="">><br class="">> Thanks for all the feedback. This is a patch which addresses all of it.<br class="">><br class=""></span>> <phinode.diff><br class="">><br class=""><br class="">> +<br class="">> + const PHINodeEdge operator*() const {<br class=""><br class="">No reason for the const in `const PHINodeEdge` here.<br class=""></blockquote><div class=""><br class=""></div><div class="">to support operator-> you have to return a pointer, which means you need the PHINodeEdge storage inside the iterator to point to (& then you can just return a const ref from op*)</div></div></div></blockquote>Ah, good point. I’m working on a version now with a mutable value we can return a reference to.<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">> + return { *Values, *Blocks };<br class="">> + }<br class="">> + };<br class="">> +<br class=""><br class="">Otherwise LGTM. Might want to pass it through clang-format; I noticed<br class="">some minor whitespace oddities.<br class=""><br class="">> *snip*<br class=""><span class="">>> On Jul 21, 2015, at 8:21 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class="">>><br class="">>><br class="">>><br class="">>> If you drop the requirements from `forward_iterator` to<br class="">>> `input_iterator`, then you're allowed to return a `PHINodeEdge` by-value<br class="">>> here instead of by-reference (unfortunately this makes it illegal to use<br class="">>> a bunch of STL algorithms; STL iterator traits are completely broken<br class="">>> IMO).<br class=""></span>> I’m fine with this. David, Chandler, please let me know how you feel about this.<br class="">><br class="">> Also, i forgot to say that I considered doing a zip iterator and inheriting this from it. This is something I think Chandler or David mentioned a few months ago. If there’s been any progress in the C++ committee on that then i’m happy to try implement something better. If not, then i don’t think what I have here should be difficult to change in future.<br class=""><br class="">I guess a generalized version would return a<br class="">`std::tuple<Value *const &, BasicBlock *const &>` or some such. Not<br class="">sure how to actually make zip iterators work well without something<br class="">like N1550 though.</blockquote><div class=""><br class="">What's N1550 offer to make zip work?<br class=""><br class="">I wouldn't mind a slightly half-hearted version that works for basic/common cases… </div></div></div></blockquote>I’m working on it now. Got most of it done, but am still working through the details of just how operator* should be implemented across a std::tuple<iterator1, iterator2, …>.</div><div><br class=""></div><div>Pete</div><br class=""></body></html>