Order the arguments of pointer_in_range

[Quuxplusone]

Semantically ordered arguments should be lexically ordered, too.
See https://quuxplusone.github.io/blog/2021/05/05/its-clamping-time/

I feel very strongly that the arguments to pointer_in_range — like the arguments to rotate and so on (with the notable, regrettable, exception of std::clamp) — should be given in "semantic order." This means that pointer_in_range(a, b, c) should be true iff a <= b < c.

With the current definition, the programmer has to do a lot of extra brainwork (i.e. consult the documentation) to figure out that pointer_in_range(a, b, c) is true iff a >= b && a < c. (As opposed, to, say, if c >= a && c < b.) Putting the "middle" argument in the "middle" is simply the most natural and self-explanatory choice.

The changes in the test file should speak for themselves: we see that it's only the "middle" argument that's varying, and that the expected return value is false only when the "middle" value incorrectly falls outside the "lower" and "higher" bounds.

I'm sending an email to this effect also to the LEWG reflector, strongly recommending that the same change be made in P3234R0.

Obviously this will be much harder to do after Boost 1.86 ships, so time is of the essence now.

[Lastique]

I feel very strongly that the arguments to pointer_in_range — like the arguments to rotate and so on (with the notable, regrettable, exception of std::clamp) — should be given in "semantic order." This means that pointer_in_range(a, b, c) should be true iff a <= b < c.

I strongly disagree. It is established convention that iterators denoting a range are passed in pairs of arguments, begin and end. Breaking that pair with another argument will be a constant source of confusion and mistakes.

pointer_in_range(a, b, c) meaning a >= b && a < c makes sense if you read it as "pointer a in range [b; c)". This, of course is just an interpretation that helps remembering the order of the arguments, but this is also sort of a convention in itself. std::clamp(a, b, c) is similarly "clamp a between b and c".

[Lastique]

BTW, std::rotate is terrible. I can never remember what it does.

[Quuxplusone]

It is established convention that iterators denoting a range are passed in pairs of arguments, begin and end. Breaking that pair with another argument will be a constant source of confusion and mistakes.

I do agree that if the function were actually pointer_in_range(p, rg), taking a range, that that would be the right signature. But whenever a function takes three arguments that are supposed to be ordered a, b, c, it's really really good to put them lexically in that order.

FWIW, std::rotate takes the range to rotate and the "new beginning" of the range: it "pulls" the new beginning leftward so that that element is now at the beginning of the range. Since the "new beginning" argument is always semantically somewhere between first and last, it is passed lexically between first and last. std::rotate(a, b, c) is defined iff a <= b <= c. So, yeah, it's reasonable to get confused whether std::rotate is "pulling" the elements leftward or "pushing" them rightward — is the middle argument the new beginning or the new end? — but the one thing you can't possibly be confused about is what order the arguments are supposed to be in.

[Lastique]

it's really really good to put them lexically in that order.

Sorry, I don't see why, aside from personal preference. IMO, the least confusing interface is "good", and the least confusing most of the time means the most consistent with other similar interfaces. "a <= b < c" doesn't exist in C++, so trying to introduce it now, and not even in that literal form but a remote shadow of it in the form of the order of arguments, does not make the interface consistent or any less error-prone at all. It just makes that interface an odd ball to stumble upon.

So, yeah, it's reasonable to get confused whether std::rotate is "pulling" the elements leftward or "pushing" them rightward — is the middle argument the new beginning or the new end? — but the one thing you can't possibly be confused about is what order the arguments are supposed to be in.

Given that most if not all std interfaces accepting pairs of iterators accept them as consecutive arguments, the pivot iterator in the middle is confusing, at least at first, until you remember that std::rotate is "special".

[pdimov]

There are at least two reasons to prefer the current argument order. First, in the sentence "the pointer p is in the range [b, e)" the order is p, b, e. (This is consistent with is_base_of.)

Second, the mathematical notation for "p is a member of [b, e)" is p ∈ [b, e).

This is not the same as std::rotate, which takes two consecutive ranges (which also necessarily form a valid range when concatenated.)

[glenfe]

Though the current reflector discussion on LEWG about P3234R0 has focused on span range, I'll update the rationale in R1 to discuss this too.

[libbooze]

Just stumbled onto this by accident, so apologies for discussing this 7 months after last comment but I think it would be great if we would have overload that enables you to write:

if (!pointer_in_range(ptr, {data_, data_ + size_}))

I think this is a new convention in std:: so I think WG21 would reject it since it would be very different from rest of std::, but maybe in boost we can have that?
In terms of optimization I am not certain compilers would be able to remove the overhead(although it looks fine to me), but for sure to me it is much nicer to read.

[glenfe]

An overload with span in Core seems OK. I might end up adding that anyway since it looks like the desire is there for P3234R1 to use that interface too.