Avoid copy in getindex(::AbstractQ, ...)

[dlfivefifty]

It's possible (and reasonable) that a subtype of AbstractQ overloads copy to return itself. This change avoids an infinite recursion that would result in this situation.

[jishnub]

Probably should be backported to v1.8

[dkarrasch]

Should we have some test with a little dummy struct? For example (fails on master with the "desired" stack overflow):

struct MyIdentity{T} <: LinearAlgebra.AbstractQ{T} end
Base.size(::MyIdentity, dim::Integer) = dim in (1,2) ? 2 : 1
Base.copy(J::MyIdentity) = J
LinearAlgebra.lmul!(::MyIdentity{T}, M::Array{T}) where {T} = M
@test MyIdentity{Float64}()[1,:] == [1.0, 0.0]

[dkarrasch]

While developing the test proposal, I noticed another strange thing, a few lines above the changed code:

size(Q::AbstractQ, dim::Integer) = size(getfield(Q, :factors), dim == 2 ? 1 : dim)

But we don't expect (or do we?) any AbstractQ subtype to have a field factors, so this should perhaps be restricted to the subtypes provided by LinearAlgebra. Downstream packages will need to define their own size, which they likely do anyways. Perhaps, this should be

size(Q::Union{HessenbergQ, QRCompactWYQ, QRPackedQ}, dim::Integer) = size(getfield(Q, :factors), dim == 2 ? 1 : dim)

The missing type in this list, from the sysimg, is QRSparseQ, which already has

https://github.com/JuliaSparse/SuiteSparse.jl/blob/f63732c1c6adecb277d8f2981cc8c1883c321bcc/src/spqr.jl#L136

[dkarrasch]

Since this is meant for backport, I'll merge this as is and do the size change on master to avoid any confusion.