It seems convenient to me to be able to use make_early_inc_range as a
direct replacement for make_range, so you can change:
foad on Apr 30 2021, 5:30 AM.Authored by
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.
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.
A few extra words: Generally I'd hope we move towards fewer uses of make_range (& mostly in the implementation of container-like things, rather than in random code handling iterators, like in the two places touched in this patch) and more towards having ranges around for a while to begin with.
For instance, the second piece of code, in LoopInterchange, might be nice if we had a "drop last" operation that operated on a range and this code became:
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.