D36089: [ELF] - Replace parallelForEach with ranged form.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 9 21:08:44 PDT 2017


Given that C++ seems to be getting more range support and so does llvm,
I think it is a reasonable direction even if the use of && is not
intuitive at first.

Cheers,
Rafael

Rui Ueyama <ruiu at google.com> writes:

> Passing begin and end is consistent with STL, so the original interface
> made sense. I don't think adding `const` improves the code because it
> doesn't make it consistent with STL and, even worse, it makes the code
> inconsistent with the LLVM libraries.
>
> My point is that, if you can keep code simple by writing a few more lines
> of code, it is worth doing, and removing a few lines of code by depending
> on a subtle behavior is not usually worth it. (So, in that sense, asking
> whether adding const improves the code or not is irrelevant, as we don't
> want to even think about it unless we have to think about it.)
>
> On Fri, Aug 4, 2017 at 7:01 PM, George Rimar <grimar at accesssoftek.com>
> wrote:
>
>> >ruiu added a comment.
>> >
>> >I wouldn't request you revert this, but if I were you, I wouldn't have
>> commit this change, because as we discussed, this change contains subtle
>> code that seems only language >experts can understand. I don't think it is
>> overall beneficial to save a few lines of code by writing very "smart" code
>> here.
>>
>> We still can save few lines if use "const R& Range" instead of "R
>> &&Range". It would work for LLD.
>> I would be fine with const version too, because personally for me ranged
>> vesrions of algorithms is much easier to read as I do not need to read
>> where is the begin() and where is end().
>>
>> So If you prefer 'const' I can change it in that way. I think in this
>> discussion was already that we can use
>> any way in LLD and change to && version later if we will need it, looks
>> nobody be against using 'const' atm.
>>
>> Though "R &&Range" is still consistent with the rest LLVM code.
>>
>> George.


More information about the llvm-commits mailing list