[PATCH] D101621: [STLExtras] Add a two argument form of make_early_inc_range

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 1 08:59:30 PDT 2021


dblaikie added a comment.

In D101621#2731274 <https://reviews.llvm.org/D101621#2731274>, @foad wrote:

> In D101621#2730920 <https://reviews.llvm.org/D101621#2730920>, @dblaikie wrote:
>
>> In D101621#2728784 <https://reviews.llvm.org/D101621#2728784>, @foad wrote:
>>
>>> This is an RFC. I'm not really sure if it's worth doing, it just seemed natural to me that make_range and make_early_inc_range could be used in the same way.
>>
>> Agreed they could be used in this way (existence proofs are the callsites you've updated in this patch) - but I'm not sure they need a wrapper that does both together. I rather like the orthogonality/composability of things. But I wouldn't insist this never be done - just vague preference to leaving them as separate things that callers can compose.
>
> I'd be fine with that //except// that the name make_<something>_range strongly suggests to me that it should behave in the same kind of way as make_range, just making some specific kind of range instead.

Oh, sure - though I think the lesson to take from that is that the abstraction is correct (because composability/orthogonality is good) and the name is less than ideal. Totally open to naming suggestions/renaming this thing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101621/new/

https://reviews.llvm.org/D101621



More information about the llvm-commits mailing list