[llvm] r225592 - [ADT] Remove the unused default constructor for iterator_range.

[David Blaikie]

On Sat, Jan 10, 2015 at 5:52 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

>
> On Sat, Jan 10, 2015 at 5:16 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> Author: chandlerc
>> Date: Sat Jan 10 19:16:26 2015
>> New Revision: 225592
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=225592&view=rev
>> Log:
>> [ADT] Remove the unused default constructor for iterator_range.
>>
>> This default constructor is a bit weird. It left the range in an invalid
>> state. That might be reasonable so that you can construct a local
>> iterator range and assign to it based on some logic to compute the range
>> you want. If folks would like to support that use case, I can add it
>> back, but in 238-odd usages none have actually wanted to do this. ;]
>>
>
> Actually, there were 3 usages in Clang.
>
> 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.
>

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?)


> 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.
>

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)

- David


>
> -Chandler
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150112/00bee6e7/attachment.html>