Fix panic message when RangeFrom index is out of bounds

[LukasKalbertodt]

Before, the Range method was called with end = slice.len(). Unfortunately, because Range::index first checks the order of the indices (start has to be smaller than end), an out of bounds index leads to core::slice::slice_index_order_fail being called. This prints the message 'slice index starts at 27 but ends at 10', which is worse than 'index 27 out of range for slice of length 10'. This is not only useful to normal users reading panic messages, but also for people inspecting assembly and being confused by slice_index_order_fail calls.

You can see the produced assembly here and try on Playground here. (By the way. this is only about which panic function is called; I'm pretty sure it does not improve anything about performance).

[rust-highfive]

r? @hanna-kruppe

(rust_highfive has picked a reviewer for you, use r? to override)

[hanna-kruppe]

So, this is definitely an improvement and I'd be happy to merge it. But I wonder if a different (entirely new) panic message would be better than reusing the one for a single index being OOB? Something like:

slice index starts at 27 but len is 10

[LukasKalbertodt]

That's a good point. I guess I could then also improve the error message for RangeTo. I just saw CI failed, too, so I will adjust this and push shortly.

[LukasKalbertodt]

I thought a bit about good panic messages, but this isn't trivial :/
I think the best would be to have different panic messages for each range type. Something like:

end index of 3..27 is out of range for slice of length 10

That way, users easily see that the panic was caused by a range index. But... (a) that adds a couple of new panic function, and (b) passing two indices (in case of Range) to the panic function bloats the actual function (as it usually requires at least one additional instruction to prepare the argument).

Alternatively, one could add two new panic messages (for start index of range >= len, and for end index of range >= len). But it's not as clear to the user, I think.

Opinions?

[hanna-kruppe]

Code size increases aren't great, and the separate "range start > len" and "range end > len" messages would be a considerable improvement over the status quo already, so I think I prefer that.

[LukasKalbertodt]

Sounds good to me. I just pushed an update. Now we should have:

range start index 27 out of range for slice of length 10

(for RangeFrom)

range end index 27 out of range for slice of length 10

(for Range and RangeTo)

I didn't use your suggestion as these ones are more similar to the existing index message. But if you prefer something else, I can still change of course.

[hanna-kruppe]

@bors r+

[bors]

📌 Commit ea74e7d has been approved by hanna-kruppe

[bors]

⌛ Testing commit ea74e7d with merge 4006dc7d272433d55e4fa4c2d8da2b709bd7e2fd...

[bors]

💔 Test failed - checks-actions

[LukasKalbertodt]

Fixed and pushed again. And I ran the full x.py test locally now, so I guess this should be fine :P

[hanna-kruppe]

Whoops, missed this! @bors r+

[bors]

📌 Commit 0d64b01 has been approved by hanna-kruppe

[bors]

⌛ Testing commit 0d64b01 with merge fe08fb7...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: hanna-kruppe
Pushing fe08fb7 to master...

[LukasKalbertodt]

@hanna-kruppe No problem, that's still notably faster than my average review delay :P Thanks!