julia
julia copied to clipboard
JuliaLang
Rename Memory wrapper from `view` to `unsafe_vector`
Fixes #54156 Changes part of #53896
The main difference between view and unsafe_vector is that the result of view has a fixed size, and creates a copy when passed to String.
The String(view( pattern is fairly common: https://juliahub.com/ui/Search?q=String(view&type=code and as more packages start using Memory it may be hard to tell if view is being used safely or not.
I'm not sure about the name unsafe_vector, but the docstring says
It is only safe to resize
vifmis subseqently not used.
So I added an unsafe_ prefix.
Ref https://github.com/JuliaLang/julia/issues/53552#issuecomment-2024288283 for a discussion on why view on a Memory should produce an Array.
If this gets approved, it might be worth adding a "See also:" reference to unsafe_vector in the view docstring.
Nit: nothing about this is unsafe, so we could call it wrap as the counter part to unsafe_wrap
wasn't it originally wrap then reverted?
https://github.com/JuliaLang/julia/pull/53896
I might be conflating issues together, didn't follow super closely
Isn't Vector allowed to shift the data in its mem when resized? For example, in the future as an optimization, could doing popfirst! then push! cause the data to be shifted over by one in mem?
This would surely break CSV.jl, as the removal of wrap did before. I don't have a strong opinion on this, but I'm commenting to know when/if this PR is merged to add the corresponding fix
Triage named it view in https://github.com/JuliaLang/julia/issues/53552.
I don't really understand what the String(_) constructor's contract is supposed to be, especially on the issue of memory ownership and mutation.
"Base.wrap" doesn't seem like it means anything in particular, it's too uninformative. At least that was my view and I think triage agreed.
Closing as discussed in https://github.com/JuliaLang/julia/pull/54372#issuecomment-2102885169
