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

Fri Apr 30 17:19:37 PDT 2021

dblaikie added a comment.

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.

In D101621#2730889 <https://reviews.llvm.org/D101621#2730889>, @dexonsmith wrote:

> Might be worth adding a small unit test, besides the examples in code, although I'm not sure what we usually do for simple wrappers like this...

Yeah, I think we're pretty hit-or-miss on testing such small things. I'm about where you are on this (if this patch is committed) - might be nice to have a unit test, but not the end of the world in any case.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101621