[llvm] r213474 - [C++11] Add predecessors(BasicBlock *) / successors(BasicBlock *) iterator ranges.

Chandler Carruth chandlerc at google.com
Mon Jul 21 13:51:15 PDT 2014


On Mon, Jul 21, 2014 at 10:31 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> BTW, I don't see the value in these mass refactorings to use C++11
> range-based for loops.  While adding the API is obviously good, changing
> everything to use it means that these mistakes are going to creep in,
> and it's hard enough to review these that they won't get many eyes.
>
> I think it would be better if the next person to touch the code cleaned
> it up (where appropriate).  If they're touching the code anyway, they'll
> have the time to understand the logic and be careful about it.
>

FWIW, I *strongly* agree.

There was one place where I updated the users of the API and that was
because *all of them had to change anyways*. And it was miserable, horrid,
and required a ton of testing and review to shake out the bugs.

Please, don't "fix" code in ways that can introduce subtle bugs unless
you're working on that code anyways, and do so in small chunks that are
easier to test, revert, and isolate when bugs sneak in.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140721/6039e3c4/attachment.html>


More information about the llvm-commits mailing list