<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 13, 2015 at 1:45 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.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"><br><div class="gmail_quote"><span class="">On Mon, Jan 12, 2015 at 11:47 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Sat, Jan 10, 2015 at 5:52 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank"><span>chandlerc</span>@gmail.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><br><div class="gmail_quote">On Sat, Jan 10, 2015 at 5:16 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank"><span>chandlerc</span>@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="overflow:hidden">Author: <span>chandlerc</span><br>
Date: Sat Jan 10 19:16:26 2015<br>
New Revision: 225592<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225592&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225592&view=rev</a><br>
Log:<br>
[<span>ADT</span>] Remove the unused default constructor for iterator_range.<br>
<br>
This default constructor is a bit weird. It left the range in an invalid<br>
state. That might be reasonable so that you can construct a local<br>
iterator range and assign to it based on some logic to compute the range<br>
you want. If folks would like to support that use case, I can add it<br>
back, but in 238-odd usages none have actually wanted to do this. ;]</div></blockquote></div><br></span>Actually, there were 3 usages in Clang.</div><div class="gmail_extra"><br></div><div class="gmail_extra">We could value initialize the iterators in the default constructor, but that would preclude using iterator_range with InputIterators (restricting it to ForwardIterators and stronger). Or we could only provide the default constructor when the iterators provide a default constructor, but that's lots of template goop for what is otherwise super simple. </div></div></blockquote></span><div><br>You wouldn't actually need template goop, would you - it'd just fail to compile if the iterator doesn't have a default ctor. It's not like we need to SFINAE it away, do we? (Well, I guess if someone ends up using is_default_constructible on the range, it would fail - but I don't know that it's too important to account for that until it actually comes up, but maybe I'm understating the pain that would cause?)<br></div></blockquote><div><br></div></span><div>I wasn't sure it would work out this way, but if so, cool. Personally, I'd still rather add the constructor with conscious intent (and documentation) of forming an empty range much as value initialized forward iterators are required to.</div></div></div></div></blockquote><div><br>Yeah, the original implementation wouldn't've worked for pointer ranges (would need an explicit begin(T()), end(T()) in the init list, etc) so it does seem a bit more accidental than deliberate.<br> </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"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div> </div><span><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">And I've only found 3 usages that were easily "fixed" to just pass the iterators explicitly. But I wanted to mention the option here in case others feel strongly we should make empty-range construction simple in this way.</div></div></blockquote></span><div><br>I haven't looked closely at the clang changes, so I'm not sure - but I can imagine it might be nice to easily create a null range (as we do for ArrayRef quite frequently). Not too fussed, though. Maybe leaving some of this as a comment in iterator_range so someone knows why it was omitted/what the gotchas to adding one are if they think they need one. (usually source history would suffice, but with no lines to blame, etc, it's harder to do that)</div></blockquote></span></div><br>Perhaps. Honestly, I've no particularly strong feelings about this. Maybe if you and others come to a particular design perspective we can just settle with that resolution? </div></div></blockquote><div><br>I don't see anyone else putting their hand up to express opinions on this, so maybe just let it stand as-is. Though I suspect a comment might still be in order, just to allude to the gotchas (I guess we don't usually quote revisions in comments? I'd kind of like to be able to point to this conversation - but I suppose the commit message can refer to this commit and the comment will be blamed on that commit message, ec... so there's a path)<br> </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">I'm pretty happy to make this look however folks think is best. I went with the current approach because it is strictly less buggy and more minimal / conservative, not because I think this is definitely the right path forward. =]</div></div>
</blockquote></div><br></div></div>