Is BTreeMap::range panicking on certain empty ranges helpful?

According to the documentation BTreeMap::range panics when the provided range's start value is past the end.

Not only is that happening only on half the paths at best, it's also counter to how I understand the various range types' interface: there is an is_empty method and it is well defined that a range is empty, among others things, when the start is past the end. That's acknowledged by examples.

Somewhere in the BTreeMap source we read:

As a diagnostic service, panics if the range specifies impossible bounds.

I question whether it's a service to anyone to have such inconsistent behavior and think that it is largely context dependent whether "impossible" is something that is invalid or not. In my opinion it is not on the standard library to have such unnecessary policies built into a container's API. Coming from the range interface the behavior I expect is clear: the range is empty so give me back an empty iterator.
Specifically, I want to invoke it with an "invalid" range at times and I don't want to special case all my logic just because BTreeMap is a snowflake that panics "as a service" when a range is empty in a certain way.

What do others think?

You're probably right, but it can't be changed now, so :woman_shrugging:

Is that because of backwards compatibility guarantees include panic behavior and folks may rely on getting a panic? I'd have hoped that "loosening" the behavior would still be fine, to be honest (and I vaguely recall reading about a similar precedent) and given how inconsistent this check is I don't really think anybody could rely on it (not happening when there is a root node vs. there is none -- something that isn't exactly reliably visible to clients).

The rather cynical wording of that source code comment reflects that its author agrees with you. The PR introducing this mentions "not in order", "nonsensical range" but the examples in this is_empty method illustrate they are just empty ranges.

Granted, there was a crash, and panicking was a reasonable solution. At the point in time when I added the source code comment, so much was refactored that the check had become a waste of time.

But since it was also documented, now it's a part of the method's contract? I don't know. I only know about one panic being revoked, and it was removed from BTreeMap, but it was never promised to happen (in particular, a panic if the Ord implementation behaves inconsistently).

1 Like

Should be noted that BTreeMap is not a special snowflake in that respect. Slices (and consequently Vec, String etc) also panic if indexed with a..b where b < a. And this is not well documented by the respective impls :frowning:

BTW, is there a reason why BTreeMap has a named method rather than just an Index impl that takes ranges, like the index-based sequences do?

BTW #2: the panic messages for str and &[_] could be unified. The former is also a bit unintuitive right now, at least to me.

panicked at 'begin <= end (1 <= 0) when slicing `foo`', src/main.rs:5:11
panicked at 'slice index starts at 1 but ends at 0', src/main.rs:5:11

Yes, because the Index trait only works if you return a reference.

3 Likes

This is, arguably, yet another point where the lack of distinction between "ranges as intervals" and "ranges as iterators" bites us.

Given let r = 10..2;, r.is_empty() returns true because r.count() returns 0 -- and similarly for all the other Iterator methods.

But I really do wish it had a guarantee that a..b always had a <= b -- which wouldn't be something you could rely on for arbitrary Range<T> because PartialOrd is safe, but you could for things like Range<usize>.

2 Likes