<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 13, 2015 at 11:55 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><span class=""><br><div class="gmail_quote">On Fri, Feb 13, 2015 at 11:30 AM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Returning a const reference to a set is not wrong at all, and it involves less moving parts, no?<div><br></div><div>The other reason why I prefer a set over range here is because I don't want to see more uses of range in LLD at this moment. I believe range is designed after N3350, which didn't make it to the standard yet. It doesn't feel great that this generic library lives under LLD. If we really need this one, we should move the file to LLVM support directory. Otherwise, I don't like to build a local rule to use range in LLD. After all other LLVM projects are not using this range.</div></div></blockquote></div><br></span>FWIW, LLVM projects are using ranges pretty heavily now, and we added llvm::iterator_range to LLVM's ADT library. So I'm pretty happy with increased usage of ranges in LLD.</div></div></blockquote><div><br></div><div>Thanks, Chandler. Maybe we should replace all uses of lld::range with llvm::iterator_range? Looks like there's no reason to not do.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br></div><div class="gmail_extra">However ranges != sets.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Honestly, std::set is the wrong data type here no matter what.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Do you need to preserve order? Use a sorted vector, and return a range (or just an arrayref), and *document* that it is a sorted order range. That is a really useful concept, and also allows fast lookups. It will be much faster than std::set.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Do you not need the order, just need to test whether things are members? Use a SmallPtrSet or a DenseSet or just use a sorted vector as above. All of them will be faster than std::set.</div></div>
</blockquote></div><br></div></div>