<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 Jun 26, 2015, at 10:33 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">As to the code in the patch itself:<br class=""><br class="">Do you need to implement op* and op->, or does the adapter have convevience implementations of one in terms of the other? (be nice if it did, but I could believe/imagine that it might not)<br class=""></div></div></blockquote>You’re right, they do define one. I needed my own operator* :</div><div><br class=""></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(187, 44, 162);" class="">const<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">SDValue</span><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> &</span>operator<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">*() </span>const<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> { </span>return<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #703daa" class="">I</span><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">-></span><span style="font-variant-ligatures: no-common-ligatures; color: #31595d" class="">get</span><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">(); }</span></div></div><div><br class=""></div><div>but ultimately the iterator_facade_base which we inherit from provides -> in terms of *.</div><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><br class="">Is that naming convention consistent with other names we use when making iterator/range pairs? Plurality in the range accessor, singularity in the begin/end? Should we just avoid providing begin/end and only provide the range accessor (people can always use op_values().begin() if they really need it - I don't mind the syntax being a bit heavier for that case to discourage raw iterator usage a little bit)</div></div></blockquote>I just copied the naming from the iterator immediately prior to this one in SelectionDAGNodes.h (so honestly that means I didn’t even take the time to notice about the naming. Probably should have). It had op_begin(), op_end() and ops(). I took a look at Value.h for what is one of the most commonly used iterators. In there we have use_begin(), use_end(), and uses(). So I think my naming is fairly consistent, but I can see your point here.</div><div><br class=""></div><div>I like the idea of just removing begin()/end(). I can’t think of a reason to iterate over a subset of the operands to a SDNode, so I’ve removed that.</div><div><br class=""></div><div>What do you think?</div><div><br class=""></div><div>Cheers,</div><div>Pete</div><div><br class=""></div><div></div></body></html>