Make reshape and view on Memory produce Arrays and delete wrap

[LilithHafner]

Make reshape and (one-based) view on Memory produce Arrays and delete wrap

Implements #53552 (comment)

Fixes #53552

[N5N3]

The reshape change seems ok as reshape of Array returns Array.
But I'm afraid view is another story, as it seems incompatible with offset inds.

[LilithHafner]

I dropped the @inbounds and restricted view to Base.OneTo and UnitRange inds. If there were an AbstractOneBasedUintRange type or an isonebased trait on abstract array types I would have used that instead.

Should we support slices?

view(m::GenericMemory{M, T}, inds::Colon) where {M, T} = view(m, eachindex(m))

[vtjnash]

But I'm afraid view is another story, as it seems incompatible with offset inds.

I think reshape(view(a, n:m), (x, y)) should be possible for the compiler to be exactly as optimized?

[LilithHafner]

I think @N5N3 was referring to offset indices like OffsetArrays.IdOffsetRange(values=3:6, indices=53:56), and they were correct that the proposed implementation was buggy for those offset inds. Fixed in 91542bc.

[LilithHafner]

IIUC it is possible for resizing a vector produced here to corrupt the underlying memory. If I'm correct, that should be documented.

[LilithHafner]

Seemingly unrelated CI failure, but master is green. Updating branch.

[LilithHafner]

The failure, all(T -> T <: AbstractString, Base.return_types(string)), was real. IDK if that's a valid or useful thing to test, but I fixed it by telling the compiler that StringVector will indeed return a Vector{UInt8}.

[oscardssmith]

I think it might be an issue if the compiler isn't able to infer the type of taking a view of a Memory.

[odow]

I think this change broke JSON3:

(base) oscar@Oscars-MBP JSONSchema % julia +nightly
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.12.0-DEV.313 (2024-04-07)
 _/ |\__'_|_|_|\__'_|  |  Commit c5a3b653353 (0 days old master)
|__/                   |

julia> Base.StringVector(UInt64(2))
ERROR: TypeError: in new, expected Tuple{Int64}, got a value of type Tuple{UInt64}
Stacktrace:
 [1] view
   @ ./genericmemory.jl:312 [inlined]
 [2] StringVector(n::UInt64)
   @ Base ./iobuffer.jl:45
 [3] top-level scope
   @ REPL[1]:1

julia> exit()
(base) oscar@Oscars-MBP JSONSchema % julia         
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.10.2 (2024-03-01)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> Base.StringVector(UInt64(2))
2-element Vector{UInt8}:
 0x50
 0x47

Example: https://github.com/quinnj/JSON3.jl/blob/6429adafd02a7ad07fa80e9a0ad21b098635244c/src/write.jl#L32

MathOptInterface failing on nightly: https://github.com/jump-dev/MathOptInterface.jl/actions/runs/8591539106/job/23540421403?pr=2464

[LilithHafner]

Thank you for reporting. I can confirm that this is a bug and this PR introduced it.

[LilithHafner]

PR fixing this: #53991

[longemen3000]

there is also a bug on CSV.jl that seems to be activated with this PR, but i suppose that is for the same reasons as the last bug (CSV really wants a Vector{UInt8})

[odow]

there is also a bug on CSV.jl that seems to be activated with this PR

Can you elaborate? Reproducible example and stack trace?

[longemen3000]

Can you elaborate? Reproducible example and stack trace?

The main problem is that CSV.jl depended on Base.wrap, that was removed in this PR. but the dependency was just to fix another bug, so i already made a PR fixing the breakage (JuliaData/CSV.jl#1133)