<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Mar 8, 2014 at 12:11 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: aaronballman<br>
Date: Sat Mar 8 14:11:24 2014<br>
New Revision: 203354<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=203354&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=203354&view=rev</a><br>
Log:<br>
Adding range-based STL-like helper APIs. llvm::distance() is the range version of std::distance. llvm::copy is the range version of std::copy.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/iterator_range.h<br></blockquote><div><br></div><div>I don't think these belong in this header at all.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified: llvm/trunk/include/llvm/ADT/iterator_range.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/iterator_range.h?rev=203354&r1=203353&r2=203354&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/iterator_range.h?rev=203354&r1=203353&r2=203354&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/iterator_range.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/iterator_range.h Sat Mar 8 14:11:24 2014<br>
@@ -23,6 +23,11 @@<br>
<br>
namespace llvm {<br>
<br>
+template <typename Range><br>
+struct range_traits {<br>
+ typedef typename Range::difference_type difference_type;<br>
+};<br>
+<br></blockquote><div><br></div><div>There is no documentation for this. We have no pattern or reference from the standard about how it should work. I don't even know where you're really going here or why we need this at all. It feels like just boilerplate to me.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
/// \brief A range adaptor for a pair of iterators.<br>
///<br>
/// This just wraps two iterators into a range-compatible interface. Nothing<br>
@@ -32,6 +37,10 @@ class iterator_range {<br>
IteratorT begin_iterator, end_iterator;<br>
<br>
public:<br>
+ // FIXME: We should be using iterator_traits to determine the<br>
+ // difference_type, but most of our iterators do not expose anything like it.<br>
+ typedef int difference_type;<br></blockquote><div><br></div><div>I don't think we should expose this until those iterators are fixed and we can use something sensible. "int" doesn't really seem like a sensible choice for most of our iterators.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
iterator_range() {}<br>
iterator_range(IteratorT begin_iterator, IteratorT end_iterator)<br>
: begin_iterator(std::move(begin_iterator)),<br>
@@ -41,6 +50,19 @@ public:<br>
IteratorT end() const { return end_iterator; }<br>
};<br>
<br>
+/// \brief Determine the distance between the end() and begin() iterators of<br>
+/// a range. Analogous to std::distance().<br>
+template <class Range><br>
+typename range_traits<Range>::difference_type distance(Range R) {<br>
+ return std::distance(R.begin(), R.end());<br>
+}<br></blockquote><div><br></div><div>Is this really worth providing at this point?</div><div><br></div><div>If it is, why not define it with the return type computed from std::distance's return type? I don't see a need for traits yet.</div>
<div><br></div><div>It doesn't belong in this header file either way.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+/// \brief Copies members of a range into the output iterator provided.<br>
+/// Analogous to std::copy.<br>
+template <class Range, class OutputIterator><br>
+OutputIterator copy(Range In, OutputIterator Result) {<br>
+ return std::copy(In.begin(), In.end(), Result);<br>
+}<br></blockquote><div><br></div><div>Same problems. This provides no actual advantage over just using the iterator form. I would just use the iterator form. If we do provide it, it shouldn't be in this header.</div>
<div><br></div><div><br></div><div>Also, as a meta style point: please prefer "typename" to "class" everywhere. Please prefer "FooT" to "Foo" for parameters everywhere.</div></div></div>
</div>