Fix findmin/findmax to return cartesian indices for BitMatrix

[nalimilan]

For consistency with other AbstractArray types. It's annoying we haven't spotted this earlier, as fixing this inconsistency is technically breaking.

The call to keys is optimized out by the compiler for vectors thanks to @inbounds.

BTW, I've also noticed that the findnext/prev(testf::Function, B::BitArray, start::Integer) methods (just above the diff) accept linear indices even for non-vectors, which is not the case for Array. What's more, there is currently no equivalent of these optimized methods taking CartesianIndex objects, meaning we use the slower generic fallback. That's lower priority, but I'll file an issue.

[mbauman]

Dang, that's unfortunate. I approve of the idea, but we should try to figure out if this is too big for a minor change.

[mbauman]

Ideally we could just compose with findfirst(B) (and findfirst(!, B) below), but I think the current implementations would be slower than what you have here. Perhaps another place for separate improvements.

[nalimilan]

Indeed. But that will require fixing findnext and findprev to accept cartesian indices (as I noted in the description).

[timholy]

Good catch. I do think this is too big for 1.0.x but obviously this is a critical fix for 1.1.0.

[StefanKarpinski]

macOS CI seems to be failing, anyone know why? @staticfloat?

[staticfloat]

It looks like Travis has changed their image to no longer include ccache. We need to either disable our use of that or get the Travis image to install it again.

EDIT: #30111

[StefanKarpinski]

F&#$ing Travis.

[StefanKarpinski]

@mbauman or @nalimilan, could you add NEWS for this minor change?

[mbauman]

NEWS is in add1246#diff-8312ad0561ef661716b48d09478362f3R27 — a similar change that also should get into 1.1.