[PATCH] CodeGenPrep: rewrite a few loops in C++11 style

[Duncan P. N. Exon Smith]

> On 2015-Jan-12, at 22:19, Nick Lewycky <nicholas at mxc.ca> wrote:
> 
> Duncan P. N. Exon Smith wrote:
>> 
>>> On 2015-Jan-07, at 14:41, Ramkumar Ramachandra<artagnon at gmail.com>  wrote:
>>> 
>>> It depends on what you want to construct from the iteration. You could construct a BasicBlock, like you said; but this is perfectly valid too: you can try it out, and see that tests pass.
>>> 
>> 
>> This works because of an implementation quirk of `ilist_iterator`,
>> which can be constructed from a reference of the node type.
> 
> That's really not an implementation quirk, that's part of the intended interface and there is code which does this on purpose (admittedly rare). It's one of the nice things you can do with intrusive lists, you can always go from one node to the prev/next node. This is how we expose that.

Right, that's how intrusive lists work; but IMO implicitly
converting to iterators is a dangerous API leak (made all the more
dangerous by range-based `for`s, where code like this "looks" like
it's doing something it isn't: `bypassSlowDivision()` is expecting
to advance the loop iterator, but it's not).

I think this should be an `explicit` constructor, like in the
attached patch (which changes the API and updates a few (of the
many) call sites).

-------------- next part --------------
A non-text attachment was scrubbed...
Name: explicit-ilist-iterator-conversions.patch
Type: application/octet-stream
Size: 4513 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150113/866d699b/attachment.obj>
-------------- next part --------------


> 
>> While technically valid, this code is misleading, and I don't know
>> that we want to propagate dependencies on that conversion.  I think
>> this should instead be, e.g.:
>> 
>>     for (BasicBlock&BB : F)
>>       EverMadeChange |= bypassSlowDivision(F,&BB, BypassWidths);
>> 
>> Thoughts from anyone else?
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>